triedb/pathdb: fix lookup sentinel collision with zero disk layer root (#34680)

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/layertree_test.go

@@ -916,3 +916,118 @@ func TestStorageLookup(t *testing.T) {
 		}
 	}
 }
+
+// TestLookupZeroBaseRootFallback is a regression test for a sentinel
+// collision in accountTip/storageTip: before the fix they returned
+// common.Hash{} as both the "stale" marker and the disk-layer fallback
+// when the disk root itself happened to be zero. lookupAccount/Storage
+// then misreported a legitimate fallback as errSnapshotStale.
+//
+// On the merkle path the collision was invisible because the empty
+// merkle trie hashes to types.EmptyRootHash (a concrete non-zero
+// keccak), so the disk layer's root was never the zero hash in
+// practice. The bug only surfaces 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{}).
+//
+// The test constructs a layer tree whose base layer's root IS the zero
+// hash, stacks diff layers on top, and exercises four cases:
+//
+//  1. Look up an account NEVER written → should fall through to the
+//     disk layer and return (diskLayer, nil). Before the fix this
+//     returned errSnapshotStale because the fallback hash collided
+//     with the sentinel.
+//  2. Symmetric case for lookupStorage.
+//  3. Look up an account written in a diff layer → should return that
+//     diff layer (the normal happy path is unaffected by the fix).
+//  4. Look up any key at a state root that isn't part of the tree
+//     (neither the disk root nor a descendant of it) → MUST still
+//     return errSnapshotStale. This pins the "other half" of the
+//     contract so a future refactor that always returns ok=true would
+//     fail here.
+func TestLookupZeroBaseRootFallback(t *testing.T) {
+	// Build a layer tree whose disk-layer root is common.Hash{} —
+	// mirrors the bintrie/verkle configuration where the empty trie
+	// hashes to EmptyVerkleHash. newTestLayerTree can't be reused
+	// because it hard-codes common.Hash{0x1}.
+	db := New(rawdb.NewMemoryDatabase(), nil, false)
+	base := newDiskLayer(common.Hash{}, 0, db, nil, nil, newBuffer(0, nil, nil, 0), nil)
+	tr := newLayerTree(base)
+
+	// Stack two diff layers on the zero-rooted disk layer, each
+	// touching a known account and slot so we have something for the
+	// happy-path lookups to find later.
+	if err := tr.add(
+		common.Hash{0x2}, common.Hash{},
+		1,
+		NewNodeSetWithOrigin(nil, nil),
+		NewStateSetWithOrigin(
+			randomAccountSet("0xa"),
+			randomStorageSet([]string{"0xa"}, [][]string{{"0x1"}}, nil),
+			nil, nil, false),
+	); err != nil {
+		t.Fatalf("add first diff layer: %v", err)
+	}
+	if err := tr.add(
+		common.Hash{0x3}, common.Hash{0x2},
+		2,
+		NewNodeSetWithOrigin(nil, nil),
+		NewStateSetWithOrigin(
+			randomAccountSet("0xb"),
+			nil, nil, nil, false),
+	); err != nil {
+		t.Fatalf("add second diff layer: %v", err)
+	}
+
+	// Case 1: unknown account queried at the head. The lookup must
+	// fall through the diff layers, hit the disk-layer fallback at
+	// base=common.Hash{}, and return the disk layer with no error —
+	// NOT errSnapshotStale.
+	l, err := tr.lookupAccount(common.HexToHash("0xdead"), common.Hash{0x3})
+	if err != nil {
+		t.Fatalf("lookupAccount on zero-base disk layer: unexpected error %v", err)
+	}
+	if l.rootHash() != (common.Hash{}) {
+		t.Errorf("expected fall-through to disk layer (root=0), got %x", l.rootHash())
+	}
+
+	// Case 2: symmetric check for storage. Slot 0x99 was never written,
+	// so the lookup must fall through to the disk layer just like
+	// Case 1.
+	l, err = tr.lookupStorage(
+		common.HexToHash("0xdead"), common.HexToHash("0x99"), common.Hash{0x3})
+	if err != nil {
+		t.Fatalf("lookupStorage on zero-base disk layer: unexpected error %v", err)
+	}
+	if l.rootHash() != (common.Hash{}) {
+		t.Errorf("expected fall-through to disk layer (root=0), got %x", l.rootHash())
+	}
+
+	// Case 3: happy path. Account 0xa was written at diff layer 0x2.
+	// The lookup must return that layer, proving the fix didn't break
+	// the normal resolution path.
+	l, err = tr.lookupAccount(common.HexToHash("0xa"), common.Hash{0x3})
+	if err != nil {
+		t.Fatalf("lookupAccount(known): %v", err)
+	}
+	if l.rootHash() != (common.Hash{0x2}) {
+		t.Errorf("known account tip: want %x, got %x",
+			common.Hash{0x2}, l.rootHash())
+	}
+
+	// Case 4: truly stale state root. This pins the other half of the
+	// contract — the boolean must actually signal not-found for an
+	// unknown state, otherwise a refactor that always returned
+	// ok=true would still pass cases 1–3.
+	_, err = tr.lookupAccount(common.HexToHash("0xa"), common.HexToHash("0xdeadbeef"))
+	if !errors.Is(err, errSnapshotStale) {
+		t.Errorf("lookupAccount(stale state): want errSnapshotStale, got %v", err)
+	}
+	_, err = tr.lookupStorage(
+		common.HexToHash("0xa"), common.HexToHash("0x1"),
+		common.HexToHash("0xdeadbeef"))
+	if !errors.Is(err, errSnapshotStale) {
+		t.Errorf("lookupStorage(stale state): want errSnapshotStale, got %v", err)
+	}
+}

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,31 +127,26 @@ 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
@@ -155,17 +154,17 @@ func (l *lookup) storageTip(accountHash common.Hash, slotHash common.Hash, state
 		// 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