trie: cleanup stateTrie (ethereum#25640)

It's a trivial PR to hide the error log when the trie node is not found in the database. The idea for this change is for all TryXXX functions, the error is already returned and we don't need to fire a log explicitly.

Recently there are a few tickets ethereum#25613 ethereum#25589 reporting that the trie nodes are missing because of debug.SetHead. The root cause is after resetting, the chain rewinds to a historical point and re-imports the blocks on top.

Since the node is already synced and started to accept transactions previously, these transactions are still kept in the txpool and verified by txpool with a live state. This live state is constructed based on the live trie database, which is changed fast by node referencing and de-referencing.

Unfortunately, when we construct a live state(like the state in txpool), we don't reference the state we have. The blockchain will garbage collect the intermediate version nodes in another thread which leads the broken live state.

The best solution for this is to forcibly obtain a reference for all live states we create and call release function once it's used up. But it might end up with more junks persisted into disk. Will try to find an elegant solution later in the following PR.

trie/secure_trie.go

@@ -35,14 +35,14 @@ func NewSecure(owner common.Hash, root common.Hash, db *Database) (*SecureTrie,
 	return NewStateTrie(owner, root, db)
 }
 
-// StateTrie wraps a trie with key hashing. In a secure trie, all
+// StateTrie wraps a trie with key hashing. In a stateTrie trie, all
 // access operations hash the key using keccak256. This prevents
 // calling code from creating long chains of nodes that
 // increase the access time.
 //
 // Contrary to a regular trie, a StateTrie can only be created with
 // New and must have an attached database. The database also stores
-// the preimage of each key.
+// the preimage of each key if preimage recording is enabled.
 //
 // StateTrie is not safe for concurrent use.
 type StateTrie struct {
@@ -53,17 +53,11 @@ type StateTrie struct {
 	secKeyCacheOwner *StateTrie // Pointer to self, replace the key cache on mismatch
 }
 
-// NewStateTrie creates a trie with an existing root node from a backing database
-// and optional intermediate in-memory node pool.
+// NewStateTrie creates a trie with an existing root node from a backing database.
 //
 // If root is the zero hash or the sha3 hash of an empty string, the
 // trie is initially empty. Otherwise, New will panic if db is nil
 // and returns MissingNodeError if the root node cannot be found.
-//
-// Accessing the trie loads nodes from the database or node pool on demand.
-// Loaded nodes are kept around until their 'cache generation' expires.
-// A new cache generation is created by each call to Commit.
-// cachelimit sets the number of past cache generations to keep.
 func NewStateTrie(owner common.Hash, root common.Hash, db *Database) (*StateTrie, error) {
 	if db == nil {
 		panic("trie.NewSecure called without a database")
@@ -87,63 +81,46 @@ func (t *StateTrie) Get(key []byte) []byte {
 
 // TryGet returns the value for key stored in the trie.
 // The value bytes must not be modified by the caller.
-// If a node was not found in the database, a MissingNodeError is returned.
+// If the specified node is not in the trie, nil will be returned.
+// If a trie node is not found in the database, a MissingNodeError is returned.
 func (t *StateTrie) TryGet(key []byte) ([]byte, error) {
 	return t.trie.TryGet(t.hashKey(key))
 }
 
+// TryGetAccount attempts to retrieve an account with provided trie path.
+// If the specified account is not in the trie, nil will be returned.
+// If a trie node is not found in the database, a MissingNodeError is returned.
 func (t *StateTrie) TryGetAccount(key []byte) (*types.StateAccount, error) {
-	var ret types.StateAccount
 	res, err := t.trie.TryGet(t.hashKey(key))
-	if err != nil {
-		log.Error(fmt.Sprintf("Unhandled trie error: %v", err))
-		return &ret, err
-	}
-	if res == nil {
-		return nil, nil
+	if res == nil || err != nil {
+		return nil, err
 	}
-	err = rlp.DecodeBytes(res, &ret)
-	return &ret, err
+	ret := new(types.StateAccount)
+	err = rlp.DecodeBytes(res, ret)
+	return ret, err
 }
 
 // TryGetAccountWithPreHashedKey does the same thing as TryGetAccount, however
 // it expects a key that is already hashed. This constitutes an abstraction leak,
 // since the client code needs to know the key format.
 func (t *StateTrie) TryGetAccountWithPreHashedKey(key []byte) (*types.StateAccount, error) {
-	var ret types.StateAccount
 	res, err := t.trie.TryGet(key)
-	if err != nil {
-		log.Error(fmt.Sprintf("Unhandled trie error: %v", err))
-		return &ret, err
-	}
-	if res == nil {
-		return nil, nil
+	if res == nil || err != nil {
+		return nil, err
 	}
-	err = rlp.DecodeBytes(res, &ret)
-	return &ret, err
+	ret := new(types.StateAccount)
+	err = rlp.DecodeBytes(res, ret)
+	return ret, err
 }
 
 // TryGetNode attempts to retrieve a trie node by compact-encoded path. It is not
 // possible to use keybyte-encoding as the path might contain odd nibbles.
+// If the specified trie node is not in the trie, nil will be returned.
+// If a trie node is not found in the database, a MissingNodeError is returned.
 func (t *StateTrie) TryGetNode(path []byte) ([]byte, int, error) {
 	return t.trie.TryGetNode(path)
 }
 
-// TryUpdateAccount account will abstract the write of an account to the
-// secure trie.
-func (t *StateTrie) TryUpdateAccount(key []byte, acc *types.StateAccount) error {
-	hk := t.hashKey(key)
-	data, err := rlp.EncodeToBytes(acc)
-	if err != nil {
-		return err
-	}
-	if err := t.trie.TryUpdate(hk, data); err != nil {
-		return err
-	}
-	t.getSecKeyCache()[string(hk)] = common.CopyBytes(key)
-	return nil
-}
-
 // Update associates key with value in the trie. Subsequent calls to
 // Get will return value. If value has length zero, any existing value
 // is deleted from the trie and calls to Get will return nil.
@@ -163,7 +140,7 @@ func (t *StateTrie) Update(key, value []byte) {
 // The value bytes must not be modified by the caller while they are
 // stored in the trie.
 //
-// If a node was not found in the database, a MissingNodeError is returned.
+// If a node is not found in the database, a MissingNodeError is returned.
 func (t *StateTrie) TryUpdate(key, value []byte) error {
 	hk := t.hashKey(key)
 	err := t.trie.TryUpdate(hk, value)
@@ -174,6 +151,21 @@ func (t *StateTrie) TryUpdate(key, value []byte) error {
 	return nil
 }
 
+// TryUpdateAccount account will abstract the write of an account to the
+// secure trie.
+func (t *StateTrie) TryUpdateAccount(key []byte, acc *types.StateAccount) error {
+	hk := t.hashKey(key)
+	data, err := rlp.EncodeToBytes(acc)
+	if err != nil {
+		return err
+	}
+	if err := t.trie.TryUpdate(hk, data); err != nil {
+		return err
+	}
+	t.getSecKeyCache()[string(hk)] = common.CopyBytes(key)
+	return nil
+}
+
 // Delete removes any existing value for key from the trie.
 func (t *StateTrie) Delete(key []byte) {
 	if err := t.TryDelete(key); err != nil {
@@ -182,14 +174,15 @@ func (t *StateTrie) Delete(key []byte) {
 }
 
 // TryDelete removes any existing value for key from the trie.
-// If a node was not found in the database, a MissingNodeError is returned.
+// If the specified trie node is not in the trie, nothing will be changed.
+// If a node is not found in the database, a MissingNodeError is returned.
 func (t *StateTrie) TryDelete(key []byte) error {
 	hk := t.hashKey(key)
 	delete(t.getSecKeyCache(), string(hk))
 	return t.trie.TryDelete(hk)
 }
 
-// TryDeleteACcount abstracts an account deletion from the trie.
+// TryDeleteAccount abstracts an account deletion from the trie.
 func (t *StateTrie) TryDeleteAccount(key []byte) error {
 	hk := t.hashKey(key)
 	delete(t.getSecKeyCache(), string(hk))