Skip to content

Commit

Permalink
fix UT test and contention issue (bnb-chain#7)
Browse files Browse the repository at this point in the history
* fix several UT with racing issues

* fix incorrect nonce balance codehash issue

case: TestEIP1559 / TestDeleteThenCreate

* Fix ExecutionSpec tests

mainly root caused by balance not updated to dirty correctly.
also fix similar issue with nonce and codehash.

* fix TestBlockChain testcase issue

    TestBlockchain/ValidBlocks/bcStateTests/refundReset.json

* fix concurrent racing issue of state.accounts.

fix incorrect use of s.accountStorageParallelLock, it is designed
to be used for dirty/pending/original storages, not the accounts and
storages.

Use statedb.AccountMux and statedb.StorageMux for accounts/storages
lock.

* fix issue of DAOTransactions

handle the issue of updateObject of mainDB object touched by DAO transaction.

---------

Co-authored-by: Sunny <[email protected]>
  • Loading branch information
DavidZangNR and sunny2022da committed Aug 6, 2024
1 parent 99959ec commit 15456d6
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 16 deletions.
20 changes: 13 additions & 7 deletions core/state/parallel_statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -1450,12 +1450,15 @@ func (s *ParallelStateDB) FinaliseForParallel(deleteEmptyObjects bool, mainDB *S
// 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.
mainDB.accountStorageParallelLock.Lock()
mainDB.AccountMux.Lock()
delete(mainDB.accounts, obj.addrHash) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(mainDB.storages, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(mainDB.accountsOrigin, obj.address) // Clear out any previously updated account data (may be recreated via a resurrect)
mainDB.AccountMux.Unlock()

mainDB.StorageMux.Lock()
delete(mainDB.storages, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(mainDB.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect)
mainDB.accountStorageParallelLock.Unlock()
mainDB.StorageMux.Unlock()
} else {
obj.finalise(true) // Prefetch slots in the background
}
Expand Down Expand Up @@ -1508,13 +1511,16 @@ func (s *ParallelStateDB) FinaliseForParallel(deleteEmptyObjects bool, mainDB *S
// 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.
mainDB.accountStorageParallelLock.Lock()
mainDB.AccountMux.Lock()
delete(mainDB.accounts, obj.addrHash) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(mainDB.storages, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(mainDB.accountsOrigin, obj.address) // Clear out any previously updated account data (may be recreated via a resurrect)
mainDB.AccountMux.Unlock()
mainDB.StorageMux.Lock()
delete(mainDB.storages, obj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(mainDB.storagesOrigin, obj.address) // Clear out any previously updated storage data (may be recreated via a resurrect)
mainDB.accountStorageParallelLock.Unlock()
mainDB.StorageMux.Unlock()

// todo: The following record seems unnecessary.
if s.parallel.isSlotDB {
s.parallel.accountsDeletedRecord = append(s.parallel.accountsDeletedRecord, obj.addrHash)
s.parallel.storagesDeleteRecord = append(s.parallel.storagesDeleteRecord, obj.addrHash)
Expand Down Expand Up @@ -1615,7 +1621,7 @@ func (s *ParallelStateDB) IntermediateRootForSlotDB(deleteEmptyObjects bool, mai
if s.TxIndex() == 0 && len(mainDB.stateObjectsPending) > 0 {
usedAddrs = make([][]byte, 0, len(s.stateObjectsPending)+len(mainDB.stateObjectsPending))
for addr := range mainDB.stateObjectsPending {
if obj, _ := s.getStateObjectFromStateObjects(addr); obj.deleted {
if obj, _ := mainDB.getStateObjectFromStateObjects(addr); obj.deleted {
mainDB.deleteStateObject(obj)
mainDB.AccountDeleted += 1
} else {
Expand Down
6 changes: 4 additions & 2 deletions core/state/state_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -518,10 +518,10 @@ func (s *stateObject) finaliseRWSet() {
func (s *stateObject) updateTrie() (Trie, error) {
maindb := s.db
if s.db.isParallel && s.db.parallel.isSlotDB {
// we need to fixup the origin storage with the mainDB. otherwise the changes maybe problem since the origin
// we need to fixup the origin storage with the mainDB. otherwise the changes maybe problematic since the origin
// is wrong.
maindb = s.db.parallel.baseStateDB
// TODO: consider delete as it is dup with accountMux and storageMux
// For dirty/pending/origin Storage access and update.
maindb.accountStorageParallelLock.Lock()
defer maindb.accountStorageParallelLock.Unlock()
}
Expand Down Expand Up @@ -844,6 +844,8 @@ func (s *stateObject) deepCopy(db *StateDB) *stateObject {
}

object.code = s.code

// The lock is unnecessary since deepCopy only invoked at global phase. No concurrent racing.
object.dirtyStorage = s.dirtyStorage.Copy()
object.originStorage = s.originStorage.Copy()
object.pendingStorage = s.pendingStorage.Copy()
Expand Down
34 changes: 27 additions & 7 deletions core/state/statedb.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,14 +756,18 @@ func (s *StateDB) updateStateObject(obj *stateObject) {
}
// Encode the account and update the account trie
addr := obj.Address()
s.trieParallelLock.Lock()
if err := s.trie.UpdateAccount(addr, &obj.data); err != nil {
s.setError(fmt.Errorf("updateStateObject (%x) error: %v", addr[:], err))
}
if obj.dirtyCode {
s.trie.UpdateContractCode(obj.Address(), common.BytesToHash(obj.CodeHash()), obj.code)
}
s.trieParallelLock.Unlock()
}

s.AccountMux.Lock()
defer s.AccountMux.Unlock()
// Cache the data until commit. Note, this update mechanism is not symmetric
// to the deletion, because whereas it is enough to track account updates
// at commit time, deletions need tracking at transaction boundary level to
Expand Down Expand Up @@ -1004,10 +1008,14 @@ func (s *StateDB) createObject(addr common.Address) (newobj *stateObject) {
prevAccountOrigin: prevAccount,
prevStorageOrigin: s.storagesOrigin[prev.address],
})
s.AccountMux.Lock()
delete(s.accounts, prev.addrHash)
delete(s.storages, prev.addrHash)
delete(s.accountsOrigin, prev.address)
s.AccountMux.Unlock()
s.StorageMux.Lock()
delete(s.storages, prev.addrHash)
delete(s.storagesOrigin, prev.address)
s.StorageMux.Unlock()
}

newobj.created = true
Expand Down Expand Up @@ -1128,10 +1136,15 @@ func (s *StateDB) copyInternal(doPrefetch bool) *StateDB {
}
// Deep copy the state changes made in the scope of block
// along with their original values.
s.AccountMux.Lock()
state.accounts = copySet(s.accounts)
state.storages = copy2DSet(s.storages)
state.accountsOrigin = copySet(state.accountsOrigin)
s.AccountMux.Unlock()

s.StorageMux.Lock()
state.storages = copy2DSet(s.storages)
state.storagesOrigin = copy2DSet(state.storagesOrigin)
s.StorageMux.Unlock()

// Deep copy the logs occurred in the scope of block
for hash, logs := range s.logs {
Expand Down Expand Up @@ -1436,14 +1449,16 @@ func (s *StateDB) CopyForSlot() *ParallelStateDB {
// state.prefetcher = s.prefetcher
}

s.accountStorageParallelLock.RLock()
// Deep copy the state changes made in the scope of block
// along with their original values.
s.AccountMux.Lock()
state.accounts = copySet(s.accounts)
state.storages = copy2DSet(s.storages)
state.accountsOrigin = copySet(state.accountsOrigin)
s.AccountMux.Unlock()
s.StorageMux.Lock()
state.storages = copy2DSet(s.storages)
state.storagesOrigin = copy2DSet(state.storagesOrigin)
s.accountStorageParallelLock.RUnlock()
s.StorageMux.Unlock()

return state
}
Expand Down Expand Up @@ -2598,13 +2613,18 @@ func (s *StateDB) MergeSlotDB(slotDb *ParallelStateDB, slotReceipt *types.Receip
// remove the addr from snapAccounts&snapStorage only when object is deleted.
// "deleted" is not equal to "snapDestructs", since createObject() will add an addr for
// snapDestructs to destroy previous object, while it will keep the addr in snapAccounts & snapAccounts
s.snapParallelLock.Lock()
delete(s.snapAccounts, addr)
delete(s.snapStorage, addr)
s.snapParallelLock.Unlock()
s.AccountMux.Lock()
delete(s.accounts, dirtyObj.addrHash) // Clear out any previously updated account data (may be recreated via a resurrect)
delete(s.storages, dirtyObj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(s.accountsOrigin, dirtyObj.address) // Clear out any previously updated account data (may be recreated via a resurrect)
s.AccountMux.Unlock()
s.StorageMux.Lock()
delete(s.storages, dirtyObj.addrHash) // Clear out any previously updated storage data (may be recreated via a resurrect)
delete(s.storagesOrigin, dirtyObj.address) // Clear out any previously updated storage data (may be recreated via a resurrect)
s.accountStorageParallelLock.Unlock()
s.StorageMux.Unlock()
}
} else {
// addr already in main DB, do merge: balance, KV, code, State(create, suicide)
Expand Down

0 comments on commit 15456d6

Please sign in to comment.