From 2ab5df694b0fc49cfddefff032d3fe0f8a4e06ff Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 18 Jul 2024 15:24:12 +0800 Subject: [PATCH] Fix: avoid update the stateObjects at conflict check phase There can be a issue that the object updated by mainDB.GetNonce etc is obseleted. The fix use statedb.getStateObjectNoUpdate to avoid touching the stateObjects of mainDB. Case: TestBlockchain/ValidBlocks/bcEIP1559/intrinsic.json --- core/state/parallel_statedb.go | 32 ++++++++++++++++++++++------ core/state/state_object.go | 8 +++---- core/state/statedb.go | 39 +++++++++++++++++++++++++++++----- 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/core/state/parallel_statedb.go b/core/state/parallel_statedb.go index a67bf75e40..d7e2d0ee24 100644 --- a/core/state/parallel_statedb.go +++ b/core/state/parallel_statedb.go @@ -1248,7 +1248,13 @@ func (slotDB *ParallelStateDB) IsParallelReadsValid(isStage2 bool) bool { } } } - nonceMain := mainDB.GetNonce(addr) + + /* can not use mainDB.GetNonce() because we do not want to record the stateObject */ + var nonceMain uint64 = 0 + mainObj := mainDB.getStateObjectNoUpdate(addr) + if mainObj != nil { + nonceMain = mainObj.Nonce() + } if nonceSlot != nonceMain { log.Debug("IsSlotDBReadsValid nonce read is invalid", "addr", addr, "nonceSlot", nonceSlot, "nonceMain", nonceMain, "SlotIndex", slotDB.parallel.SlotIndex, @@ -1267,7 +1273,12 @@ func (slotDB *ParallelStateDB) IsParallelReadsValid(isStage2 bool) bool { } } - balanceMain := mainDB.GetBalance(addr) + balanceMain := common.Big0 + mainObj := mainDB.getStateObjectNoUpdate(addr) + if mainObj != nil { + balanceMain = mainObj.Balance() + } + if balanceSlot.Cmp(balanceMain) != 0 { log.Debug("IsSlotDBReadsValid balance read is invalid", "addr", addr, "balanceSlot", balanceSlot, "balanceMain", balanceMain, "SlotIndex", slotDB.parallel.SlotIndex, @@ -1353,7 +1364,11 @@ func (slotDB *ParallelStateDB) IsParallelReadsValid(isStage2 bool) bool { // check code for addr, codeSlot := range slotDB.parallel.codeReadsInSlot { - codeMain := mainDB.GetCode(addr) + var codeMain []byte = nil + object := mainDB.getStateObjectNoUpdate(addr) + if object != nil { + codeMain = object.Code() + } if !bytes.Equal(codeSlot, codeMain) { log.Debug("IsSlotDBReadsValid code read is invalid", "addr", addr, "len codeSlot", len(codeSlot), "len codeMain", len(codeMain), "SlotIndex", slotDB.parallel.SlotIndex, @@ -1363,7 +1378,11 @@ func (slotDB *ParallelStateDB) IsParallelReadsValid(isStage2 bool) bool { } // check codeHash for addr, codeHashSlot := range slotDB.parallel.codeHashReadsInSlot { - codeHashMain := mainDB.GetCodeHash(addr) + codeHashMain := common.Hash{} + object := mainDB.getStateObjectNoUpdate(addr) + if object != nil { + codeHashMain = common.BytesToHash(object.CodeHash()) + } if !bytes.Equal(codeHashSlot.Bytes(), codeHashMain.Bytes()) { log.Debug("IsSlotDBReadsValid codehash read is invalid", "addr", addr, "codeHashSlot", codeHashSlot, "codeHashMain", codeHashMain, "SlotIndex", slotDB.parallel.SlotIndex, @@ -1374,7 +1393,7 @@ func (slotDB *ParallelStateDB) IsParallelReadsValid(isStage2 bool) bool { // addr state check for addr, stateSlot := range slotDB.parallel.addrStateReadsInSlot { stateMain := false // addr not exist - if mainDB.getStateObject(addr) != nil { + if mainDB.getStateObjectNoUpdate(addr) != nil { stateMain = true // addr exist in main DB } if stateSlot != stateMain { @@ -1387,7 +1406,7 @@ func (slotDB *ParallelStateDB) IsParallelReadsValid(isStage2 bool) bool { } // snapshot destructs check for addr, destructRead := range slotDB.parallel.addrSnapDestructsReadsInSlot { - mainObj := mainDB.getStateObject(addr) + mainObj := mainDB.getStateObjectNoUpdate(addr) if mainObj == nil { log.Debug("IsSlotDBReadsValid snapshot destructs read invalid, address should exist", "addr", addr, "destruct", destructRead, @@ -1536,6 +1555,7 @@ func (s *ParallelStateDB) FinaliseForParallel(deleteEmptyObjects bool, mainDB *S obj.finalise(true) // Prefetch slots in the background } else { obj.fixUpOriginAndResetPendingStorage() + obj.finalise(false) } } diff --git a/core/state/state_object.go b/core/state/state_object.go index 0e183b4ab4..9e1e1bff27 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -258,7 +258,7 @@ func newObject(dbItf StateDBer, isParallel bool, address common.Address, acct *t address: address, addrHash: crypto.Keccak256Hash(address[:]), origin: origin, - data: *acct, + data: *acct.Copy(), isParallel: isParallel, originStorage: newStorage(isParallel), pendingStorage: newStorage(isParallel), @@ -267,7 +267,7 @@ func newObject(dbItf StateDBer, isParallel bool, address common.Address, acct *t } // dirty data when create a new account - if acct == nil { + if created { s.dirtyBalance = new(big.Int).Set(acct.Balance) s.dirtyNonce = new(uint64) *s.dirtyNonce = acct.Nonce @@ -834,7 +834,7 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject { address: s.address, addrHash: s.addrHash, origin: s.origin, - data: s.data, + data: *s.data.Copy(), isParallel: s.isParallel, } if s.trie != nil { @@ -972,7 +972,7 @@ func (s *stateObject) Root() common.Hash { func (s *stateObject) fixUpOriginAndResetPendingStorage() { if s.db.isParallel && s.db.parallel.isSlotDB { mainDB := s.db.parallel.baseStateDB - origObj := mainDB.getStateObject(s.address) + origObj := mainDB.getStateObjectNoUpdate(s.address) mainDB.accountStorageParallelLock.RLock() if origObj != nil && origObj.originStorage.Length() != 0 { s.originStorage = origObj.originStorage.Copy() diff --git a/core/state/statedb.go b/core/state/statedb.go index 61f60601cb..3b6f6a373a 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -76,6 +76,8 @@ func (s *StateObjectSyncMap) StoreStateObject(addr common.Address, stateObject * func (s *StateDB) loadStateObj(addr common.Address) (*stateObject, bool) { if s.isParallel { + s.parallelStateAccessLock.Lock() + defer s.parallelStateAccessLock.Unlock() ret, ok := s.parallel.stateObjects.LoadStateObject(addr) return ret, ok } @@ -90,9 +92,9 @@ func (s *StateDB) storeStateObj(addr common.Address, stateObject *stateObject) { // When a state object is stored into s.parallel.stateObjects, // it belongs to base StateDB, it is confirmed and valid. // TODO-dav: remove the lock/unlock? - stateObject.db.parallelStateAccessLock.Lock() - s.parallel.stateObjects.Store(addr, stateObject) - stateObject.db.parallelStateAccessLock.Unlock() + s.parallelStateAccessLock.Lock() + s.parallel.stateObjects.StoreStateObject(addr, stateObject) + s.parallelStateAccessLock.Unlock() } else { s.stateObjects[addr] = stateObject } @@ -819,6 +821,33 @@ func (s *StateDB) getStateObject(addr common.Address) *stateObject { return nil } +// getStateObjectNoUpdate is similar with getStateObject except that it does not +// update stateObjects records. +func (s *StateDB) getStateObjectNoUpdate(addr common.Address) *stateObject { + obj := s.getDeletedStateObjectNoUpdate(addr) + if obj != nil && !obj.deleted { + return obj + } + return nil +} + +func (s *StateDB) getDeletedStateObjectNoUpdate(addr common.Address) *stateObject { + s.RecordRead(types.AccountStateKey(addr, types.AccountSelf), struct{}{}) + + // Prefer live objects if any is available + if obj, _ := s.getStateObjectFromStateObjects(addr); obj != nil { + return obj + } + + data, ok := s.getStateObjectFromSnapshotOrTrie(addr) + if !ok { + return nil + } + // Insert into the live set + obj := newObject(s, s.isParallel, addr, data) + return obj +} + func (s *StateDB) GetStateObjectFromSnapshotOrTrie(addr common.Address) (data *types.StateAccount, ok bool) { return s.getStateObjectFromSnapshotOrTrie(addr) } @@ -881,11 +910,11 @@ func (s *StateDB) getStateObjectFromSnapshotOrTrie(addr common.Address) (data *t // If snapshot unavailable or reading from it failed, load from the database if data == nil { + s.trieParallelLock.Lock() + defer s.trieParallelLock.Unlock() var trie Trie if s.isParallel { // hold lock for parallel - s.trieParallelLock.Lock() - defer s.trieParallelLock.Unlock() if s.parallel.isSlotDB { if s.parallel.baseStateDB == nil { return nil, false