FIX: issue in fixUpOriginAndResetPendingStorage (bnb-chain#14)

The originStorage will miss some loading in txn execution,do merge
rather than simple copy

This fix also use stateObject specific lock for storage update, rather
than the one in stateDB.

Co-authored-by: Sunny <[email protected]>

core/state/state_object.go

@@ -172,7 +172,8 @@ type stateObject struct {
 
 	// isParallel indicates this state object is used in parallel mode, in which mode the
 	// storage would be sync.Map instead of map
-	isParallel bool
+	isParallel         bool
+	storageRecordsLock sync.RWMutex // for pending/dirty/origin storage read (lightCopy) and write (Intermediate/FixupOrigin)
 
 	originStorage  Storage // Storage cache of original entries to dedup rewrites
 	pendingStorage Storage // Storage entries that need to be flushed to disk, at the end of an entire block
@@ -532,8 +533,8 @@ func (s *stateObject) updateTrie() (Trie, error) {
 		// is wrong.
 		maindb = s.db.parallel.baseStateDB
 		// For dirty/pending/origin Storage access and update.
-		maindb.accountStorageParallelLock.Lock()
-		defer maindb.accountStorageParallelLock.Unlock()
+		s.storageRecordsLock.Lock()
+		defer s.storageRecordsLock.Unlock()
 	}
 	// Make sure all dirty slots are finalized into the pending storage area
 	s.finalise(false)
@@ -829,10 +830,10 @@ func (s *stateObject) lightCopy(db *ParallelStateDB) *stateObject {
 	// the problem. fortunately, the KVRead will record this and compare it with mainDB.
 
 	//object.dirtyStorage = s.dirtyStorage.Copy()
-	s.db.accountStorageParallelLock.RLock()
+	s.storageRecordsLock.RLock()
 	object.originStorage = s.originStorage.Copy()
 	object.pendingStorage = s.pendingStorage.Copy()
-	s.db.accountStorageParallelLock.RUnlock()
+	s.storageRecordsLock.RUnlock()
 	return object
 }
 
@@ -985,14 +986,27 @@ func (s *stateObject) fixUpOriginAndResetPendingStorage() {
 	if s.db.isParallel && s.db.parallel.isSlotDB {
 		mainDB := s.db.parallel.baseStateDB
 		origObj := mainDB.getStateObjectNoUpdate(s.address)
-		mainDB.accountStorageParallelLock.RLock()
+		s.storageRecordsLock.RLock()
 		if origObj != nil && origObj.originStorage.Length() != 0 {
-			s.originStorage = origObj.originStorage.Copy()
+			originStorage := origObj.originStorage.Copy()
+			// During the tx execution, the originStorage can be updated with GetCommittedState()
+			// But is never get updated for the already existed one as there is no finalise called in execution.
+			// so here get the latest object in MainDB, and update the object storage with
+			s.originStorage.Range(func(keyItf, valueItf interface{}) bool {
+				key := keyItf.(common.Hash)
+				value := valueItf.(common.Hash)
+				// Skip noop changes, persist actual changes
+				if _, ok := originStorage.GetValue(key); !ok {
+					originStorage.StoreValue(key, value)
+				}
+				return true
+			})
+			s.originStorage = originStorage
 		}
 		// isParallel is unnecessary since the pendingStorage for slotObject will be used serially from now on.
 		if s.pendingStorage.Length() > 0 {
 			s.pendingStorage = newStorage(false)
 		}
-		mainDB.accountStorageParallelLock.RUnlock()
+		s.storageRecordsLock.RUnlock()
 	}
 }

core/state/statedb.go

@@ -196,11 +196,9 @@ type StateDB struct {
 	parallelStateAccessLock sync.RWMutex
 	snapParallelLock        sync.RWMutex // for parallel mode, for main StateDB, slot will read snapshot, while processor will write.
 	trieParallelLock        sync.Mutex   // for parallel mode of trie, mostly for get states/objects from trie, lock required to handle trie tracer.
-	// TODO: is it possible to remove this accountStorageParallelLock?
-	accountStorageParallelLock sync.RWMutex // for global state account/storage read (copyForSlot) and write (Intermediate)
-	snapDestructs              map[common.Address]struct{}
-	snapAccounts               map[common.Address][]byte
-	snapStorage                map[common.Address]map[string][]byte
+	snapDestructs           map[common.Address]struct{}
+	snapAccounts            map[common.Address][]byte
+	snapStorage             map[common.Address]map[string][]byte
 
 	// originalRoot is the pre-state root, before any changes were made.
 	// It will be updated when the Commit is called.
@@ -760,7 +758,8 @@ func (s *StateDB) GetTransientState(addr common.Address, key common.Hash) common
 // updateStateObject writes the given object to the trie.
 func (s *StateDB) updateStateObject(obj *stateObject) {
 	if !(s.isParallel && s.parallel.isSlotDB) {
-		s.accountStorageParallelLock.Lock()
+		obj.storageRecordsLock.Lock()
+		defer obj.storageRecordsLock.Unlock()
 	}
 	if !s.noTrie {
 		// Track the amount of time wasted on updating the account from the trie
@@ -797,10 +796,6 @@ func (s *StateDB) updateStateObject(obj *stateObject) {
 			s.accountsOrigin[obj.address] = types.SlimAccountRLP(*obj.origin)
 		}
 	}
-
-	if !(s.isParallel && s.parallel.isSlotDB) {
-		s.accountStorageParallelLock.Unlock()
-	}
 }
 
 // deleteStateObject removes the given object from the state trie.