Skip to content

Commit

Permalink
Fix getStateObject in statedb when MVHashmap is enabled
Browse files Browse the repository at this point in the history
getDeletedStateObject is renamed to getStateObject after a recent upstream merge. We should wrap the entire function call with MVRead.
  • Loading branch information
cffls committed Aug 13, 2024
1 parent b1419eb commit a23701f
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 92 deletions.
91 changes: 5 additions & 86 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand All @@ -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
}
Expand All @@ -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.
Expand Down
11 changes: 5 additions & 6 deletions core/state/statedb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"maps"
"math"
"math/big"
"math/rand"
"reflect"
"slices"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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) {
Expand Down

0 comments on commit a23701f

Please sign in to comment.