From 0d16a418877b5491824521bcd1b2e5e6aefedf1e Mon Sep 17 00:00:00 2001 From: CPerezz Date: Wed, 8 Apr 2026 00:17:55 +0200 Subject: [PATCH] triedb/pathdb: fix lookup sentinel collision with zero disk layer root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- triedb/pathdb/layertree.go | 8 ++++---- triedb/pathdb/lookup.go | 37 ++++++++++++++++++------------------- 2 files changed, 22 insertions(+), 23 deletions(-) diff --git a/triedb/pathdb/layertree.go b/triedb/pathdb/layertree.go index 0d7ca5a5b4..b20e40bd05 100644 --- a/triedb/pathdb/layertree.go +++ b/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] diff --git a/triedb/pathdb/lookup.go b/triedb/pathdb/lookup.go index 719546f410..9b300ec871 100644 --- a/triedb/pathdb/lookup.go +++ b/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