Skip to content

Commit

Permalink
Revert "core, trie: track state change set with account address (ethe…
Browse files Browse the repository at this point in the history 
…reum#27815)"

This reverts commit 426aec5.
  • Loading branch information
@devopsbo3
devopsbo3 committed Nov 10, 2023
1 parent f27d361 commit 562be5a
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 60 deletions.
4 changes: 2 additions & 2 deletions core/state/journal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,10 @@ func (ch resetObjectChange) revert(s *StateDB) {
s.storages[ch.prev.addrHash] = ch.prevStorage
}
if ch.prevAccountOriginExist {
s.accountsOrigin[ch.prev.address] = ch.prevAccountOrigin
s.accountsOrigin[ch.prev.addrHash] = ch.prevAccountOrigin
}
if ch.prevStorageOrigin != nil {
s.storagesOrigin[ch.prev.address] = ch.prevStorageOrigin
s.storagesOrigin[ch.prev.addrHash] = ch.prevStorageOrigin
}
}

Expand Down
4 changes: 2 additions & 2 deletions core/state/state_object.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,9 @@ func (s *stateObject) updateTrie() (Trie, error) {

// Cache the original value of mutated storage slots
if origin == nil {
if origin = s.db.storagesOrigin[s.address]; origin == nil {
if origin = s.db.storagesOrigin[s.addrHash]; origin == nil {
origin = make(map[common.Hash][]byte)
s.db.storagesOrigin[s.address] = origin
s.db.storagesOrigin[s.addrHash] = origin
}
}
// Track the original value of slot only if it's mutated first time
Expand Down
80 changes: 40 additions & 40 deletions core/state/statedb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ type StateDB struct {

// These maps hold the state changes (including the corresponding
// original value) that occurred in this **block**.
accounts map[common.Hash][]byte // The mutated accounts in 'slim RLP' encoding
storages map[common.Hash]map[common.Hash][]byte // The mutated slots in prefix-zero trimmed rlp format
accountsOrigin map[common.Address][]byte // The original value of mutated accounts in 'slim RLP' encoding
storagesOrigin map[common.Address]map[common.Hash][]byte // The original value of mutated slots in prefix-zero trimmed rlp format
accounts map[common.Hash][]byte // The mutated accounts in 'slim RLP' encoding
storages map[common.Hash]map[common.Hash][]byte // The mutated slots in prefix-zero trimmed rlp format
accountsOrigin map[common.Hash][]byte // The original value of mutated accounts in 'slim RLP' encoding
storagesOrigin map[common.Hash]map[common.Hash][]byte // The original value of mutated slots in prefix-zero trimmed rlp format

// This map holds 'live' objects, which will get modified while processing
// a state transition.
Expand Down Expand Up @@ -157,8 +157,8 @@ func New(root common.Hash, db Database, snaps *snapshot.Tree) (*StateDB, error)
snaps: snaps,
accounts: make(map[common.Hash][]byte),
storages: make(map[common.Hash]map[common.Hash][]byte),
accountsOrigin: make(map[common.Address][]byte),
storagesOrigin: make(map[common.Address]map[common.Hash][]byte),
accountsOrigin: make(map[common.Hash][]byte),
storagesOrigin: make(map[common.Hash]map[common.Hash][]byte),
stateObjects: make(map[common.Address]*stateObject),
stateObjectsPending: make(map[common.Address]struct{}),
stateObjectsDirty: make(map[common.Address]struct{}),
Expand Down Expand Up @@ -555,11 +555,11 @@ func (s *StateDB) updateStateObject(obj *stateObject) {
// Track the original value of mutated account, nil means it was not present.
// Skip if it has been tracked (because updateStateObject may be called
// multiple times in a block).
if _, ok := s.accountsOrigin[obj.address]; !ok {
if _, ok := s.accountsOrigin[obj.addrHash]; !ok {
if obj.origin == nil {
s.accountsOrigin[obj.address] = nil
s.accountsOrigin[obj.addrHash] = nil
} else {
s.accountsOrigin[obj.address] = types.SlimAccountRLP(*obj.origin)
s.accountsOrigin[obj.addrHash] = types.SlimAccountRLP(*obj.origin)
}
}
}
Expand Down Expand Up @@ -676,7 +676,7 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)
// 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.
prevAccount, ok := s.accountsOrigin[prev.address]
prevAccount, ok := s.accountsOrigin[prev.addrHash]
s.journal.append(resetObjectChange{
account: &addr,
prev: prev,
Expand All @@ -685,12 +685,12 @@ func (s *StateDB) createObject(addr common.Address) (newobj, prev *stateObject)
prevStorage: s.storages[prev.addrHash],
prevAccountOriginExist: ok,
prevAccountOrigin: prevAccount,
prevStorageOrigin: s.storagesOrigin[prev.address],
prevStorageOrigin: s.storagesOrigin[prev.addrHash],
})
delete(s.accounts, prev.addrHash)
delete(s.storages, prev.addrHash)
delete(s.accountsOrigin, prev.address)
delete(s.storagesOrigin, prev.address)
delete(s.accountsOrigin, prev.addrHash)
delete(s.storagesOrigin, prev.addrHash)
}

newobj.created = true
Expand Down Expand Up @@ -766,8 +766,8 @@ func (s *StateDB) Copy() *StateDB {
originalRoot: s.originalRoot,
accounts: make(map[common.Hash][]byte),
storages: make(map[common.Hash]map[common.Hash][]byte),
accountsOrigin: make(map[common.Address][]byte),
storagesOrigin: make(map[common.Address]map[common.Hash][]byte),
accountsOrigin: make(map[common.Hash][]byte),
storagesOrigin: make(map[common.Hash]map[common.Hash][]byte),
stateObjects: make(map[common.Address]*stateObject, len(s.journal.dirties)),
stateObjectsPending: make(map[common.Address]struct{}, len(s.stateObjectsPending)),
stateObjectsDirty: make(map[common.Address]struct{}, len(s.journal.dirties)),
Expand Down Expand Up @@ -824,10 +824,10 @@ func (s *StateDB) Copy() *StateDB {
}
// Deep copy the state changes made in the scope of block
// along with their original values.
state.accounts = copySet(s.accounts)
state.storages = copy2DSet(s.storages)
state.accountsOrigin = copySet(state.accountsOrigin)
state.storagesOrigin = copy2DSet(state.storagesOrigin)
state.accounts = copyAccounts(s.accounts)
state.storages = copyStorages(s.storages)
state.accountsOrigin = copyAccounts(state.accountsOrigin)
state.storagesOrigin = copyStorages(state.storagesOrigin)

// Deep copy the logs occurred in the scope of block
for hash, logs := range s.logs {
Expand Down Expand Up @@ -917,10 +917,10 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) {
// Note, we can't do this only at the end of a block because multiple
// transactions within the same block might self destruct and then
// resurrect an account; but the snapshotter needs both events.
delete(s.accounts, obj.addrHash) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(s.storages, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(s.accountsOrigin, obj.address) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(s.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(s.accounts, obj.addrHash) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(s.storages, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(s.accountsOrigin, obj.addrHash) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(s.storagesOrigin, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
} else {
obj.finalise(true) // Prefetch slots in the background
}
Expand Down Expand Up @@ -1098,8 +1098,8 @@ func (s *StateDB) deleteStorage(addr common.Address, addrHash common.Hash, root
//
// In case (d), **original** account along with its storages should be deleted,
// with their values be tracked as original value.
func (s *StateDB) handleDestruction(nodes *trienode.MergedNodeSet) (map[common.Address]struct{}, error) {
incomplete := make(map[common.Address]struct{})
func (s *StateDB) handleDestruction(nodes *trienode.MergedNodeSet) (map[common.Hash]struct{}, error) {
incomplete := make(map[common.Hash]struct{})
for addr, prev := range s.stateObjectsDestruct {
// The original account was non-existing, and it's marked as destructed
// in the scope of block. It can be case (a) or (b).
Expand All @@ -1109,12 +1109,12 @@ func (s *StateDB) handleDestruction(nodes *trienode.MergedNodeSet) (map[common.A
addrHash := crypto.Keccak256Hash(addr[:])
if prev == nil {
if _, ok := s.accounts[addrHash]; ok {
s.accountsOrigin[addr] = nil // case (b)
s.accountsOrigin[addrHash] = nil // case (b)
}
continue
}
// It can overwrite the data in s.accountsOrigin set by 'updateStateObject'.
s.accountsOrigin[addr] = types.SlimAccountRLP(*prev) // case (c) or (d)
s.accountsOrigin[addrHash] = types.SlimAccountRLP(*prev) // case (c) or (d)

// Short circuit if the storage was empty.
if prev.Root == types.EmptyRootHash {
Expand All @@ -1130,17 +1130,17 @@ func (s *StateDB) handleDestruction(nodes *trienode.MergedNodeSet) (map[common.A
// created. In this case, wipe the entire storage state diff because
// of aborted deletion.
if aborted {
incomplete[addr] = struct{}{}
delete(s.storagesOrigin, addr)
incomplete[addrHash] = struct{}{}
delete(s.storagesOrigin, addrHash)
continue
}
if s.storagesOrigin[addr] == nil {
s.storagesOrigin[addr] = slots
if s.storagesOrigin[addrHash] == nil {
s.storagesOrigin[addrHash] = slots
} else {
// It can overwrite the data in s.storagesOrigin[addrHash] set by
// 'object.updateTrie'.
for key, val := range slots {
s.storagesOrigin[addr][key] = val
s.storagesOrigin[addrHash][key] = val
}
}
if err := nodes.Merge(set); err != nil {
Expand Down Expand Up @@ -1290,8 +1290,8 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er
// Clear all internal flags at the end of commit operation.
s.accounts = make(map[common.Hash][]byte)
s.storages = make(map[common.Hash]map[common.Hash][]byte)
s.accountsOrigin = make(map[common.Address][]byte)
s.storagesOrigin = make(map[common.Address]map[common.Hash][]byte)
s.accountsOrigin = make(map[common.Hash][]byte)
s.storagesOrigin = make(map[common.Hash]map[common.Hash][]byte)
s.stateObjectsDirty = make(map[common.Address]struct{})
s.stateObjectsDestruct = make(map[common.Address]*types.StateAccount)
return root, nil
Expand Down Expand Up @@ -1387,18 +1387,18 @@ func (s *StateDB) convertAccountSet(set map[common.Address]*types.StateAccount)
return ret
}

// copySet returns a deep-copied set.
func copySet[k comparable](set map[k][]byte) map[k][]byte {
copied := make(map[k][]byte, len(set))
// copyAccounts returns a deep-copied account set of the provided one.
func copyAccounts(set map[common.Hash][]byte) map[common.Hash][]byte {
copied := make(map[common.Hash][]byte, len(set))
for key, val := range set {
copied[key] = common.CopyBytes(val)
}
return copied
}

// copy2DSet returns a two-dimensional deep-copied set.
func copy2DSet[k comparable](set map[k]map[common.Hash][]byte) map[k]map[common.Hash][]byte {
copied := make(map[k]map[common.Hash][]byte, len(set))
// copyStorages returns a deep-copied storage set of the provided one.
func copyStorages(set map[common.Hash]map[common.Hash][]byte) map[common.Hash]map[common.Hash][]byte {
copied := make(map[common.Hash]map[common.Hash][]byte, len(set))
for addr, subset := range set {
copied[addr] = make(map[common.Hash][]byte, len(subset))
for key, val := range subset {
Expand Down
23 changes: 10 additions & 13 deletions core/state/statedb_fuzz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/rawdb"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/rlp"
"github.com/ethereum/go-ethereum/trie"
"github.com/ethereum/go-ethereum/trie/triestate"
Expand Down Expand Up @@ -172,11 +171,11 @@ func (test *stateTest) String() string {
func (test *stateTest) run() bool {
var (
roots []common.Hash
accountList []map[common.Address][]byte
storageList []map[common.Address]map[common.Hash][]byte
accountList []map[common.Hash][]byte
storageList []map[common.Hash]map[common.Hash][]byte
onCommit = func(states *triestate.Set) {
accountList = append(accountList, copySet(states.Accounts))
storageList = append(storageList, copy2DSet(states.Storages))
accountList = append(accountList, copyAccounts(states.Accounts))
storageList = append(storageList, copyStorages(states.Storages))
}
disk = rawdb.NewMemoryDatabase()
tdb = trie.NewDatabaseWithConfig(disk, &trie.Config{OnCommit: onCommit})
Expand Down Expand Up @@ -236,9 +235,8 @@ func (test *stateTest) run() bool {
// - the account was indeed not present in trie
// - the account is present in new trie, nil->nil is regarded as invalid
// - the slots transition is correct
func (test *stateTest) verifyAccountCreation(next common.Hash, db *trie.Database, otr, ntr *trie.Trie, addr common.Address, slots map[common.Hash][]byte) error {
func (test *stateTest) verifyAccountCreation(next common.Hash, db *trie.Database, otr, ntr *trie.Trie, addrHash common.Hash, slots map[common.Hash][]byte) error {
// Verify account change
addrHash := crypto.Keccak256Hash(addr.Bytes())
oBlob, err := otr.Get(addrHash.Bytes())
if err != nil {
return err
Expand Down Expand Up @@ -287,9 +285,8 @@ func (test *stateTest) verifyAccountCreation(next common.Hash, db *trie.Database
// - the account was indeed present in trie
// - the account in old trie matches the provided value
// - the slots transition is correct
func (test *stateTest) verifyAccountUpdate(next common.Hash, db *trie.Database, otr, ntr *trie.Trie, addr common.Address, origin []byte, slots map[common.Hash][]byte) error {
func (test *stateTest) verifyAccountUpdate(next common.Hash, db *trie.Database, otr, ntr *trie.Trie, addrHash common.Hash, origin []byte, slots map[common.Hash][]byte) error {
// Verify account change
addrHash := crypto.Keccak256Hash(addr.Bytes())
oBlob, err := otr.Get(addrHash.Bytes())
if err != nil {
return err
Expand Down Expand Up @@ -341,7 +338,7 @@ func (test *stateTest) verifyAccountUpdate(next common.Hash, db *trie.Database,
return nil
}

func (test *stateTest) verify(root common.Hash, next common.Hash, db *trie.Database, accountsOrigin map[common.Address][]byte, storagesOrigin map[common.Address]map[common.Hash][]byte) error {
func (test *stateTest) verify(root common.Hash, next common.Hash, db *trie.Database, accountsOrigin map[common.Hash][]byte, storagesOrigin map[common.Hash]map[common.Hash][]byte) error {
otr, err := trie.New(trie.StateTrieID(root), db)
if err != nil {
return err
Expand All @@ -350,12 +347,12 @@ func (test *stateTest) verify(root common.Hash, next common.Hash, db *trie.Datab
if err != nil {
return err
}
for addr, account := range accountsOrigin {
for addrHash, account := range accountsOrigin {
var err error
if len(account) == 0 {
err = test.verifyAccountCreation(next, db, otr, ntr, addr, storagesOrigin[addr])
err = test.verifyAccountCreation(next, db, otr, ntr, addrHash, storagesOrigin[addrHash])
} else {
err = test.verifyAccountUpdate(next, db, otr, ntr, addr, accountsOrigin[addr], storagesOrigin[addr])
err = test.verifyAccountUpdate(next, db, otr, ntr, addrHash, accountsOrigin[addrHash], storagesOrigin[addrHash])
}
if err != nil {
return err
Expand Down
6 changes: 3 additions & 3 deletions trie/triestate/state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import "github.com/ethereum/go-ethereum/common"
// The value refers to the original content of state before the transition
// is made. Nil means that the state was not present previously.
type Set struct {
Accounts map[common.Address][]byte // Mutated account set, nil means the account was not present
Storages map[common.Address]map[common.Hash][]byte // Mutated storage set, nil means the slot was not present
Incomplete map[common.Address]struct{} // Indicator whether the storage slot is incomplete due to large deletion
Accounts map[common.Hash][]byte // Mutated account set, nil means the account was not present
Storages map[common.Hash]map[common.Hash][]byte // Mutated storage set, nil means the slot was not present
Incomplete map[common.Hash]struct{} // Indicator whether the storage slot is incomplete due to large deletion
}

0 comments on commit 562be5a

Please sign in to comment.