triedb/pathdb: fix lookup sentinel collision with zero disk layer root

lookup.accountTip and storageTip used common.Hash{} as the "state is
stale" sentinel while ALSO returning common.Hash{} when the disk layer
itself happened to be keyed by the zero hash. lookupAccount/Storage
then blindly compared the returned value against common.Hash{} and
misreported a legitimate disk-layer fallback as errSnapshotStale.

For a merkle path database this sentinel collision is invisible: an
empty merkle trie hashes to types.EmptyRootHash which is a concrete
non-zero keccak, so the disk layer's root never equals common.Hash{}.
The collision only shows up once the disk layer root can legitimately
be zero — for example, a fresh verkle/bintrie database where the empty
binary trie hashes to EmptyVerkleHash = common.Hash{}. In that
configuration, any Account/Storage lookup for a key that has never
been written ends up taking the disk-layer fallback branch, which
correctly returns base=common.Hash{}, which lookupAccount then
misreads as "stale" and bubbles an error up to the caller.

Fix: change accountTip/storageTip to return (common.Hash, bool) so the
"found the tip" signal is carried out of band from the hash value.
lookupAccount/Storage now consult the boolean rather than comparing
the returned hash to zero. The returned hash itself may still be zero
(that is a valid disk-layer root on the bintrie path) and callers
must not treat it as a sentinel.

Noticed while wiring the bintrie flat-state reader in a separate
branch; the fix is scheme-agnostic and lands here so it can flow into
master independently of that work.

Author: CPerezz

triedb/pathdb/layertree.go

@@ -319,8 +319,8 @@ func (tree *layerTree) lookupAccount(accountHash common.Hash, state common.Hash)
 	tree.lock.RLock()
 	defer tree.lock.RUnlock()
 
-	tip := tree.lookup.accountTip(accountHash, state, tree.base.root)
-	if tip == (common.Hash{}) {
+	tip, ok := tree.lookup.accountTip(accountHash, state, tree.base.root)
+	if !ok {
 		return nil, fmt.Errorf("[%#x] %w", state, errSnapshotStale)
 	}
 	l := tree.layers[tip]
@@ -337,8 +337,8 @@ func (tree *layerTree) lookupStorage(accountHash common.Hash, slotHash common.Ha
 	tree.lock.RLock()
 	defer tree.lock.RUnlock()
 
-	tip := tree.lookup.storageTip(accountHash, slotHash, state, tree.base.root)
-	if tip == (common.Hash{}) {
+	tip, ok := tree.lookup.storageTip(accountHash, slotHash, state, tree.base.root)
+	if !ok {
 		return nil, fmt.Errorf("[%#x] %w", state, errSnapshotStale)
 	}
 	l := tree.layers[tip]

triedb/pathdb/lookup.go

@@ -92,12 +92,16 @@ func newLookup(head layer, descendant func(state common.Hash, ancestor common.Ha
 // stateID or is a descendant of it.
 //
 // If found, the account data corresponding to the supplied stateID resides
-// in that layer. Otherwise, two scenarios are possible:
+// in the layer identified by the returned hash (ok=true). Otherwise,
+// (common.Hash{}, false) is returned to signal that the supplied stateID is
+// stale.
 //
-// (a) the account remains unmodified from the current disk layer up to the state
-// layer specified by the stateID: fallback to the disk layer for data retrieval,
-// (b) or the layer specified by the stateID is stale: reject the data retrieval.
-func (l *lookup) accountTip(accountHash common.Hash, stateID common.Hash, base common.Hash) common.Hash {
+// Note the returned hash may itself be common.Hash{} when the disk layer's
+// root is zero — as is the case for a fresh verkle/bintrie database whose
+// empty trie hashes to EmptyVerkleHash. Callers must therefore consult the
+// boolean rather than comparing the returned hash against common.Hash{}
+// directly.
+func (l *lookup) accountTip(accountHash common.Hash, stateID common.Hash, base common.Hash) (common.Hash, bool) {
 	// Traverse the mutation history from latest to oldest one. Several
 	// scenarios are possible:
 	//
@@ -123,49 +127,44 @@ func (l *lookup) accountTip(accountHash common.Hash, stateID common.Hash, base c
 		// containing the modified data. Otherwise, the current state may be ahead
 		// of the requested one or belong to a different branch.
 		if list[i] == stateID || l.descendant(stateID, list[i]) {
-			return list[i]
+			return list[i], true
 		}
 	}
 	// No layer matching the stateID or its descendants was found. Use the
 	// current disk layer as a fallback.
 	if base == stateID || l.descendant(stateID, base) {
-		return base
+		return base, true
 	}
 	// The layer associated with 'stateID' is not the descendant of the current
 	// disk layer, it's already stale, return nothing.
-	return common.Hash{}
+	return common.Hash{}, false
 }
 
 // storageTip traverses the layer list associated with the given account and
 // slot hash in reverse order to locate the first entry that either matches
 // the specified stateID or is a descendant of it.
 //
-// If found, the storage data corresponding to the supplied stateID resides
-// in that layer. Otherwise, two scenarios are possible:
-//
-// (a) the storage slot remains unmodified from the current disk layer up to
-// the state layer specified by the stateID: fallback to the disk layer for
-// data retrieval, (b) or the layer specified by the stateID is stale: reject
-// the data retrieval.
-func (l *lookup) storageTip(accountHash common.Hash, slotHash common.Hash, stateID common.Hash, base common.Hash) common.Hash {
+// See accountTip for the returned-hash / ok convention — the same
+// bintrie-zero-root caveat applies here.
+func (l *lookup) storageTip(accountHash common.Hash, slotHash common.Hash, stateID common.Hash, base common.Hash) (common.Hash, bool) {
 	list := l.storages[storageKey(accountHash, slotHash)]
 	for i := len(list) - 1; i >= 0; i-- {
 		// If the current state matches the stateID, or the requested state is a
 		// descendant of it, return the current state as the most recent one
 		// containing the modified data. Otherwise, the current state may be ahead
 		// of the requested one or belong to a different branch.
 		if list[i] == stateID || l.descendant(stateID, list[i]) {
-			return list[i]
+			return list[i], true
 		}
 	}
 	// No layer matching the stateID or its descendants was found. Use the
 	// current disk layer as a fallback.
 	if base == stateID || l.descendant(stateID, base) {
-		return base
+		return base, true
 	}
 	// The layer associated with 'stateID' is not the descendant of the current
 	// disk layer, it's already stale, return nothing.
-	return common.Hash{}
+	return common.Hash{}, false
 }
 
 // addLayer traverses the state data retained in the specified diff layer and