Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions core/state/journal.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ type (
account *common.Address
prev *stateObject
prevdestruct bool
prevAccount []byte
prevStorage map[common.Hash][]byte
}
suicideChange struct {
account *common.Address
Expand Down Expand Up @@ -160,6 +162,12 @@ func (ch resetObjectChange) revert(s *StateDB) {
if !ch.prevdestruct {
delete(s.stateObjectsDestruct, ch.prev.address)
}
if len(ch.prevAccount) > 0 {
s.snapAccounts[ch.prev.addrHash] = ch.prevAccount
}
if len(ch.prevStorage) > 0 {
s.snapStorage[ch.prev.addrHash] = ch.prevStorage
}
}

func (ch resetObjectChange) dirtied() *common.Address {
Expand Down
27 changes: 26 additions & 1 deletion core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,11 +627,36 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)
if prev == nil {
s.journal.append(createObjectChange{account: &addr})
} else {
// The original account should be marked as destructed and all cached
// account and storage data should be cleared as well. Note, it must
// be done here, otherwise the destruction event of original one will
// be lost.
_, prevdestruct := s.stateObjectsDestruct[prev.address]
if !prevdestruct {
s.stateObjectsDestruct[prev.address] = struct{}{}
}
s.journal.append(resetObjectChange{account: &addr, prev: prev, prevdestruct: prevdestruct})
var (
account []byte
storage map[common.Hash][]byte
)
// There may be some cached account/storage data already since IntermediateRoot
// will be called for each transaction before byzantium fork which will always
// cache the latest account/storage data.
if s.snapAccounts != nil {
account = s.snapAccounts[prev.addrHash]
delete(s.snapAccounts, prev.addrHash)
}
if s.snapStorage != nil {
storage = s.snapStorage[prev.addrHash]
delete(s.snapStorage, prev.addrHash)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These storages are otherwise only manipulated at transaction boundaries: finalize and commit. It does not seem correct to me to modify them on the fly like this, in the middle of a transaction.

What I most of all want to ensure that we avoid: changing the evm semantics in a way which fixes a theoretical corner-case but causes a break in something which can be triggered in practice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

snapAccount, snapStorage are essentially equivalent with stateObjectsDestruct.

  • snapAccount, snapStorage cache the state data
  • stateObjectsDestruct caches the destruction event

Because in the case of ResetAccount, the original one is replaced with the new one,
it's the only correct place to manipulate these three maps, otherwise the destruction
event will be lost.

We can run a full sync to ensure nothing is broken.

s.journal.append(resetObjectChange{
account: &addr,
prev: prev,
prevdestruct: prevdestruct,
prevAccount: account,
prevStorage: storage,
})
}
s.setStateObject(newobj)
if prev != nil && !prev.deleted {
Expand Down
37 changes: 37 additions & 0 deletions core/state/statedb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import (

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/core/state/snapshot"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/trie"
)

// Tests that updating a state trie does not leak any database writes prior to
Expand Down Expand Up @@ -998,3 +1001,37 @@ func TestStateDBTransientStorage(t *testing.T) {
t.Fatalf("transient storage mismatch: have %x, want %x", got, value)
}
}

func TestResetObject(t *testing.T) {
var (
disk = rawdb.NewMemoryDatabase()
tdb = trie.NewDatabase(disk)
db = NewDatabaseWithNodeDB(disk, tdb)
snaps, _ = snapshot.New(snapshot.Config{CacheSize: 10}, disk, tdb, types.EmptyRootHash)
state, _ = New(types.EmptyRootHash, db, snaps)
addr = common.HexToAddress("0x1")
slotA = common.HexToHash("0x1")
slotB = common.HexToHash("0x2")
)
// Initialize account with balance and storage in first transaction.
state.SetBalance(addr, big.NewInt(1))
state.SetState(addr, slotA, common.BytesToHash([]byte{0x1}))
state.IntermediateRoot(true)

// Reset account and mutate balance and storages
state.CreateAccount(addr)
state.SetBalance(addr, big.NewInt(2))
state.SetState(addr, slotB, common.BytesToHash([]byte{0x2}))
root, _ := state.Commit(true)

// Ensure the original account is wiped properly
snap := snaps.Snapshot(root)
slot, _ := snap.Storage(crypto.Keccak256Hash(addr.Bytes()), crypto.Keccak256Hash(slotA.Bytes()))
if len(slot) != 0 {
t.Fatalf("Unexpected storage slot")
}
slot, _ = snap.Storage(crypto.Keccak256Hash(addr.Bytes()), crypto.Keccak256Hash(slotB.Bytes()))
if !bytes.Equal(slot, []byte{0x2}) {
t.Fatalf("Unexpected storage slot value %v", slot)
}
}