From a23701f2d4b824ff30885d2061f6f71a11710dfc Mon Sep 17 00:00:00 2001 From: Jerry Date: Tue, 13 Aug 2024 11:29:01 -0700 Subject: [PATCH] Fix getStateObject in statedb when MVHashmap is enabled getDeletedStateObject is renamed to getStateObject after a recent upstream merge. We should wrap the entire function call with MVRead. --- core/state/statedb.go | 91 +++----------------------------------- core/state/statedb_test.go | 11 +++-- 2 files changed, 10 insertions(+), 92 deletions(-) diff --git a/core/state/statedb.go b/core/state/statedb.go index 70fde92552..85c198055e 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -422,7 +422,7 @@ func (s *StateDB) ApplyMVWriteSet(writes []blockstm.WriteDescriptor) { case CodePath: s.SetCode(addr, sr.GetCode(addr)) case SuicidePath: - stateObject := sr.getDeletedStateObject(addr) + stateObject := sr.getStateObject(addr) if stateObject != nil { s.SelfDestruct(addr) } @@ -973,99 +973,25 @@ func (s *StateDB) deleteStateObject(addr common.Address) { // getStateObject retrieves a state object given by the address, returning nil if // the object is not found or was deleted in this execution context. func (s *StateDB) getStateObject(addr common.Address) *stateObject { - // Prefer live objects if any is available - if obj := s.stateObjects[addr]; obj != nil { - return obj - } - // Short circuit if the account is already destructed in this block. - if _, ok := s.stateObjectsDestruct[addr]; ok { - return nil - } - // If no live objects are available, attempt to use snapshots - var data *types.StateAccount - if s.snap != nil { - start := time.Now() - acc, err := s.snap.Account(crypto.HashData(s.hasher, addr.Bytes())) - s.SnapshotAccountReads += time.Since(start) - if err == nil { - if acc == nil { - return nil - } - data = &types.StateAccount{ - Nonce: acc.Nonce, - Balance: acc.Balance, - CodeHash: acc.CodeHash, - Root: common.BytesToHash(acc.Root), - } - if len(data.CodeHash) == 0 { - data.CodeHash = types.EmptyCodeHash.Bytes() - } - if data.Root == (common.Hash{}) { - data.Root = types.EmptyRootHash - } - } - } - // If snapshot unavailable or reading from it failed, load from the database - if data == nil { - start := time.Now() - var err error - data, err = s.trie.GetAccount(addr) - s.AccountReads += time.Since(start) - - if err != nil { - s.setError(fmt.Errorf("getDeleteStateObject (%x) error: %w", addr.Bytes(), err)) - return nil - } - if data == nil { - return nil - } - } - // Independent of where we loaded the data from, add it to the prefetcher. - // Whilst this would be a bit weird if snapshots are disabled, but we still - // want the trie nodes to end up in the prefetcher too, so just push through. - if s.prefetcher != nil { - if err := s.prefetcher.prefetch(common.Hash{}, s.originalRoot, common.Address{}, [][]byte{addr[:]}, true); err != nil { - log.Error("Failed to prefetch account", "addr", addr, "err", err) - } - } - // Insert into the live set - obj := newObject(s, addr, data) - s.setStateObject(obj) - return obj -} - -// getDeletedStateObject is similar to getStateObject, but instead of returning -// nil for a deleted state object, it returns the actual object with the deleted -// flag set. This is needed by the state journal to revert to the correct s- -// destructed object instead of wiping all knowledge about the state object. -func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { return MVRead(s, blockstm.NewAddressKey(addr), nil, func(s *StateDB) *stateObject { // Prefer live objects if any is available if obj := s.stateObjects[addr]; obj != nil { return obj } - // Short circuit if the account is already destructed in this block. if _, ok := s.stateObjectsDestruct[addr]; ok { return nil } - // If no live objects are available, attempt to use snapshots var data *types.StateAccount - - if s.snap != nil { // nolint + if s.snap != nil { start := time.Now() - acc, err := s.snap.Account(crypto.HashData(crypto.NewKeccakState(), addr.Bytes())) - - if metrics.Enabled { - s.SnapshotAccountReads += time.Since(start) - } - + acc, err := s.snap.Account(crypto.HashData(s.hasher, addr.Bytes())) + s.SnapshotAccountReads += time.Since(start) if err == nil { if acc == nil { return nil } - data = &types.StateAccount{ Nonce: acc.Nonce, Balance: acc.Balance, @@ -1075,7 +1001,6 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { if len(data.CodeHash) == 0 { data.CodeHash = types.EmptyCodeHash.Bytes() } - if data.Root == (common.Hash{}) { data.Root = types.EmptyRootHash } @@ -1084,24 +1009,18 @@ func (s *StateDB) getDeletedStateObject(addr common.Address) *stateObject { // If snapshot unavailable or reading from it failed, load from the database if data == nil { start := time.Now() - var err error data, err = s.trie.GetAccount(addr) - - if metrics.Enabled { - s.AccountReads += time.Since(start) - } + s.AccountReads += time.Since(start) if err != nil { s.setError(fmt.Errorf("getDeleteStateObject (%x) error: %w", addr.Bytes(), err)) return nil } - if data == nil { return nil } } - // Independent of where we loaded the data from, add it to the prefetcher. // Whilst this would be a bit weird if snapshots are disabled, but we still // want the trie nodes to end up in the prefetcher too, so just push through. diff --git a/core/state/statedb_test.go b/core/state/statedb_test.go index e6301fa3cd..31a28dd2f2 100644 --- a/core/state/statedb_test.go +++ b/core/state/statedb_test.go @@ -23,7 +23,6 @@ import ( "fmt" "maps" "math" - "math/big" "math/rand" "reflect" "slices" @@ -810,7 +809,7 @@ func TestMVHashMapReadWriteDelete(t *testing.T) { b = states[4].GetBalance(addr) assert.Equal(t, common.Hash{}, v) - assert.Equal(t, common.Big0, b) + assert.Equal(t, uint256.NewInt(0), b) } func TestMVHashMapRevert(t *testing.T) { @@ -846,7 +845,7 @@ func TestMVHashMapRevert(t *testing.T) { states[1].SetState(addr, key, common.HexToHash("0x02")) v := states[1].GetState(addr, key) b := states[1].GetBalance(addr) - assert.Equal(t, new(big.Int).SetUint64(uint64(200)), b) + assert.Equal(t, uint256.NewInt(200), b) assert.Equal(t, common.HexToHash("0x02"), v) states[1].SelfDestruct(addr) @@ -1014,7 +1013,7 @@ func TestMVHashMapOverwrite(t *testing.T) { b = states[2].GetBalance(addr) assert.Equal(t, common.Hash{}, v) - assert.Equal(t, common.Big0, b) + assert.Equal(t, uint256.NewInt(0), b) } func TestMVHashMapWriteNoConflict(t *testing.T) { @@ -1088,7 +1087,7 @@ func TestMVHashMapWriteNoConflict(t *testing.T) { assert.Equal(t, common.Hash{}, states[3].GetState(addr, key1)) assert.Equal(t, common.Hash{}, states[3].GetState(addr, key2)) - assert.Equal(t, common.Big0, states[3].GetBalance(addr)) + assert.Equal(t, uint256.NewInt(0), states[3].GetBalance(addr)) // Tx1 delete for _, v := range states[1].writeMap { @@ -1099,7 +1098,7 @@ func TestMVHashMapWriteNoConflict(t *testing.T) { assert.Equal(t, common.Hash{}, states[3].GetState(addr, key1)) assert.Equal(t, common.Hash{}, states[3].GetState(addr, key2)) - assert.Equal(t, common.Big0, states[3].GetBalance(addr)) + assert.Equal(t, uint256.NewInt(0), states[3].GetBalance(addr)) } func TestApplyMVWriteSet(t *testing.T) {