core/state: simplify storage trie update and commit

core/state/state_object.go

@@ -264,15 +264,15 @@ func (s *stateObject) finalise(prefetch bool) {
 	}
 }
 
-// updateTrie writes cached storage modifications into the object's storage trie.
-// It will return nil if the trie has not been loaded and no changes have been
-// made. An error will be returned if the trie can't be loaded/updated correctly.
-func (s *stateObject) updateTrie() (Trie, error) {
-	// Make sure all dirty slots are finalized into the pending storage area
-	s.finalise(false) // Don't prefetch anymore, pull directly if need be
-	if len(s.pendingStorage) == 0 {
-		return s.trie, nil
-	}
+// updateTrie writes cached storage modifications to the object's storage trie.
+// If the storage trie hasn't been loaded, it will be loaded automatically. If
+// there's a problem with loading or updating the trie properly, an error will
+// be returned to handle the situation.
+//
+// This function assumes that there are cached storage modifications present;
+// otherwise, calling it would be unnecessary due to the additional overhead
+// of loading the trie.
+func (s *stateObject) updateTrie() error {
 	// Track the amount of time wasted on updating the storage trie
 	if metrics.EnabledExpensive {
 		defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now())
@@ -281,14 +281,13 @@ func (s *stateObject) updateTrie() (Trie, error) {
 	var (
 		storage map[common.Hash][]byte
 		origin  map[common.Hash][]byte
-		hasher  = s.db.hasher
 	)
 	tr, err := s.getTrie()
 	if err != nil {
 		s.db.setError(err)
-		return nil, err
+		return err
 	}
-	// Insert all the pending updates into the trie
+	// Insert all the pending storage updates into the trie
 	usedStorage := make([][]byte, 0, len(s.pendingStorage))
 	for key, value := range s.pendingStorage {
 		// Skip noop changes, persist actual changes
@@ -298,21 +297,20 @@ func (s *stateObject) updateTrie() (Trie, error) {
 		prev := s.originStorage[key]
 		s.originStorage[key] = value
 
-		// rlp-encoded value to be used by the snapshot
-		var snapshotVal []byte
+		var encoded []byte // rlp-encoded value to be used by the snapshot
 		if (value == common.Hash{}) {
 			if err := tr.DeleteStorage(s.address, key[:]); err != nil {
 				s.db.setError(err)
-				return nil, err
+				return err
 			}
 			s.db.StorageDeleted += 1
 		} else {
-			trimmedVal := common.TrimLeftZeroes(value[:])
 			// Encoding []byte cannot fail, ok to ignore the error.
-			snapshotVal, _ = rlp.EncodeToBytes(trimmedVal)
-			if err := tr.UpdateStorage(s.address, key[:], trimmedVal); err != nil {
+			trimmed := common.TrimLeftZeroes(value[:])
+			encoded, _ = rlp.EncodeToBytes(trimmed)
+			if err := tr.UpdateStorage(s.address, key[:], trimmed); err != nil {
 				s.db.setError(err)
-				return nil, err
+				return err
 			}
 			s.db.StorageUpdated += 1
 		}
@@ -323,8 +321,8 @@ func (s *stateObject) updateTrie() (Trie, error) {
 				s.db.storages[s.addrHash] = storage
 			}
 		}
-		khash := crypto.HashData(hasher, key[:])
-		storage[khash] = snapshotVal // snapshotVal will be nil if it's deleted
+		khash := crypto.HashData(s.db.hasher, key[:])
+		storage[khash] = encoded // encoded will be nil if it's deleted
 
 		// Cache the original value of mutated storage slots
 		if origin == nil {
@@ -349,38 +347,35 @@ func (s *stateObject) updateTrie() (Trie, error) {
 	if s.db.prefetcher != nil {
 		s.db.prefetcher.used(s.addrHash, s.data.Root, usedStorage)
 	}
-	if len(s.pendingStorage) > 0 {
-		s.pendingStorage = make(Storage)
-	}
-	return tr, nil
+	s.pendingStorage = make(Storage) // reset pending map
+	return nil
 }
 
-// UpdateRoot sets the trie root to the current root hash of. An error
-// will be returned if trie root hash is not computed correctly.
+// updateRoot flushes all cached storage mutations to trie, recalculating the
+// new storage trie root. This function assumes all storage mutations have
+// already been moved to pendingStorage map by finalise.
 func (s *stateObject) updateRoot() {
-	tr, err := s.updateTrie()
-	if err != nil {
+	// Short circuit if nothing changed, don't bother with hashing anything
+	if len(s.pendingStorage) == 0 {
 		return
 	}
-	// If nothing changed, don't bother with hashing anything
-	if tr == nil {
+	// Flush cached storage mutations into trie.
+	if err := s.updateTrie(); err != nil {
 		return
 	}
 	// Track the amount of time wasted on hashing the storage trie
 	if metrics.EnabledExpensive {
 		defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now())
 	}
-	s.data.Root = tr.Hash()
+	s.data.Root = s.trie.Hash() // the trie must be loaded.
 }
 
-// commit returns the changes made in storage trie and updates the account data.
+// commit obtains a set of dirty storage trie nodes and updates the account data.
+// The returned set can be nil if nothing to commit. This function assumes all
+// storage mutations have already been flushed into trie by updateRoot.
 func (s *stateObject) commit() (*trienode.NodeSet, error) {
-	tr, err := s.updateTrie()
-	if err != nil {
-		return nil, err
-	}
-	// If nothing changed, don't bother with committing anything
-	if tr == nil {
+	// Short circuit if trie is not even loaded, don't bother with committing anything
+	if s.trie == nil {
 		s.origin = s.data.Copy()
 		return nil, nil
 	}
@@ -388,7 +383,10 @@ func (s *stateObject) commit() (*trienode.NodeSet, error) {
 	if metrics.EnabledExpensive {
 		defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now())
 	}
-	root, nodes, err := tr.Commit(false)
+	// The trie is currently in an open state and could potentially contain
+	// cached mutations. Call commit to acquire a set of nodes that have been
+	// modified, the set can be nil if nothing to commit.
+	root, nodes, err := s.trie.Commit(false)
 	if err != nil {
 		return nil, err
 	}