mirror of
https://github.com/ethereum/go-ethereum.git
synced 2026-06-17 12:21:38 +00:00
core/state: fix (nil,nil) shadowing trie reader fallback in bintrieFlatReader
Addresses review finding C3. Before this commit, bintrieFlatReader.Account returned (nil, nil) when both the BasicData and CodeHash leaves were absent from the flat state. multiStateReader.Account treats (nil, nil) as "confirmed absent" and short-circuits — the trie reader never runs. This silently hid every corruption mode the other A-commits are fixing (C1 mid-stem resume loss, C2 disk-layer shape mismatch, in-transition stale data, etc.): the flat state said "not present" and nobody checked. Fix: introduce errBintrieFlatStateMiss as a local sentinel. When both leaves are absent, the flat reader returns (nil, errBintrieFlatStateMiss) instead of (nil, nil). The multiStateReader falls through on any non-nil error, so the trie reader now runs and serves as the authoritative gatekeeper. If the flat state genuinely has no data (and the trie reader also returns nil), the end result is the same — but any case where the flat state is wrong and the trie is right is now caught by the fallthrough. Same treatment for Storage: absent blob returns errBintrieFlatStateMiss. Known limitation: BinaryTrie.GetAccount does not verify stem membership (a characteristic of verkle-style tries where non-membership proofs are handled externally). A truly non-existent account returns the closest stem's data, not nil. The TestBintrieFlatReaderMissingAccountSentinel test therefore verifies the flat reader's sentinel in isolation rather than the end-to-end multiStateReader result.
This commit is contained in:
parent
fcc0587ec3
commit
78f785e4ff
2 changed files with 77 additions and 28 deletions
|
|
@ -180,6 +180,23 @@ func (r *flatReader) Storage(addr common.Address, key common.Hash) (common.Hash,
|
||||||
return value, nil
|
return value, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// errBintrieFlatStateMiss is returned by bintrieFlatReader's Account
|
||||||
|
// and Storage methods whenever the flat state has no data for the
|
||||||
|
// queried key. Returning a non-nil error (rather than (nil, nil) or
|
||||||
|
// the zero hash) is essential because multiStateReader.Account/Storage
|
||||||
|
// short-circuits on the first err == nil result — if bintrieFlatReader
|
||||||
|
// returned "absent" as a success value, the trie reader would never
|
||||||
|
// run, and any flat-state corruption, in-transition state, or missing
|
||||||
|
// stem would be silently misreported as "account does not exist".
|
||||||
|
//
|
||||||
|
// This sentinel triggers the fall-through to the next reader in the
|
||||||
|
// chain (typically the trieReader), which serves as the gatekeeper for
|
||||||
|
// distinguishing "genuinely absent" from "temporarily missing from
|
||||||
|
// flat state". The merkle flatReader has the same contract via
|
||||||
|
// pathdb's internal errNotCoveredYet; we define a local sentinel to
|
||||||
|
// avoid a cross-package import cycle with triedb/pathdb.
|
||||||
|
var errBintrieFlatStateMiss = errors.New("bintrie flat state: entry not covered")
|
||||||
|
|
||||||
// bintrieFlatReader is the binary-trie analogue of flatReader. It exposes
|
// bintrieFlatReader is the binary-trie analogue of flatReader. It exposes
|
||||||
// the StateReader interface backed by the path database's per-stem flat
|
// the StateReader interface backed by the path database's per-stem flat
|
||||||
// state, doing the EIP-7864 key derivation locally so the underlying
|
// state, doing the EIP-7864 key derivation locally so the underlying
|
||||||
|
|
@ -234,15 +251,18 @@ func newBintrieFlatReader(reader database.StateReader) *bintrieFlatReader {
|
||||||
// consistency here. TestBinaryHasherWritesBothBasicAndCodeHash locks
|
// consistency here. TestBinaryHasherWritesBothBasicAndCodeHash locks
|
||||||
// this invariant down.
|
// this invariant down.
|
||||||
//
|
//
|
||||||
// Fall-through vs confirmed-absent (see A2):
|
// Return value contract:
|
||||||
// - both leaves genuinely absent from the flat state →
|
// - both leaves 32 bytes → decoded Account, nil error.
|
||||||
// errNotCoveredYet → multiStateReader falls through to trie reader.
|
// - either leaf invalid length → corruption error, surfaced as-is.
|
||||||
// - both leaves present-but-stem-blob-has-zero-offsets (post-delete
|
// - both leaves absent (nil from AccountRLP) → errBintrieFlatStateMiss
|
||||||
// tombstone) → (nil, nil) → confirmed absent.
|
// sentinel. This does NOT mean "account does not exist" — it means
|
||||||
//
|
// "the flat state has no data for this stem right now" (possibly
|
||||||
// At this commit (A1) we still return (nil, nil) for the absent case.
|
// because the generator hasn't reached it, or because the account
|
||||||
// A2 tightens this to an errNotCoveredYet sentinel so the trie reader
|
// has simply never been touched by a block that flushed). The
|
||||||
// runs on miss.
|
// multiStateReader is responsible for falling through to the trie
|
||||||
|
// reader, which is the authoritative source. Returning (nil, nil)
|
||||||
|
// here would short-circuit the fall-through and silently hide
|
||||||
|
// every corruption mode the other A-commits are fixing.
|
||||||
func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) {
|
func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) {
|
||||||
basicKey := common.BytesToHash(bintrie.GetBinaryTreeKeyBasicData(addr))
|
basicKey := common.BytesToHash(bintrie.GetBinaryTreeKeyBasicData(addr))
|
||||||
codeKey := common.BytesToHash(bintrie.GetBinaryTreeKeyCodeHash(addr))
|
codeKey := common.BytesToHash(bintrie.GetBinaryTreeKeyCodeHash(addr))
|
||||||
|
|
@ -256,7 +276,10 @@ func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) {
|
||||||
return nil, fmt.Errorf("bintrie CodeHash read %x: %w", addr, err)
|
return nil, fmt.Errorf("bintrie CodeHash read %x: %w", addr, err)
|
||||||
}
|
}
|
||||||
if len(basicBlob) == 0 && len(codeBlob) == 0 {
|
if len(basicBlob) == 0 && len(codeBlob) == 0 {
|
||||||
return nil, nil
|
// Flat state has no data for this stem. Fall through to the
|
||||||
|
// next reader in the multi-reader chain (trie reader) rather
|
||||||
|
// than silently returning "not found".
|
||||||
|
return nil, fmt.Errorf("%w: addr=%x", errBintrieFlatStateMiss, addr)
|
||||||
}
|
}
|
||||||
// A bintrie leaf is always either absent or exactly 32 bytes. A
|
// A bintrie leaf is always either absent or exactly 32 bytes. A
|
||||||
// shorter blob is a corruption signal; surface it with enough
|
// shorter blob is a corruption signal; surface it with enough
|
||||||
|
|
@ -296,9 +319,14 @@ func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) {
|
||||||
// the diff layer stores all bintrie leaves under accountData regardless
|
// the diff layer stores all bintrie leaves under accountData regardless
|
||||||
// of whether they came from an account header or a storage write.
|
// of whether they came from an account header or a storage write.
|
||||||
//
|
//
|
||||||
// A nil result means "no entry in the flat state"; the caller must
|
// Return value contract:
|
||||||
// distinguish this from "entry present with zero value", which the
|
// - 32-byte leaf found → decode as common.Hash and return.
|
||||||
// bintrie writes as 32 zero bytes (the bintrie's tombstone convention).
|
// - invalid-length leaf → corruption error.
|
||||||
|
// - no leaf at this slot → errBintrieFlatStateMiss sentinel so
|
||||||
|
// multiStateReader falls through to the trie reader. A slot that
|
||||||
|
// was explicitly set to zero is NOT absent — the bintrie tombstone
|
||||||
|
// convention writes 32 zero bytes, which is a present 32-byte leaf
|
||||||
|
// and returns common.Hash{} via the normal path.
|
||||||
func (r *bintrieFlatReader) Storage(addr common.Address, slot common.Hash) (common.Hash, error) {
|
func (r *bintrieFlatReader) Storage(addr common.Address, slot common.Hash) (common.Hash, error) {
|
||||||
fullKey := bintrie.GetBinaryTreeKeyStorageSlot(addr, slot[:])
|
fullKey := bintrie.GetBinaryTreeKeyStorageSlot(addr, slot[:])
|
||||||
blob, err := r.reader.AccountRLP(common.BytesToHash(fullKey))
|
blob, err := r.reader.AccountRLP(common.BytesToHash(fullKey))
|
||||||
|
|
@ -306,7 +334,10 @@ func (r *bintrieFlatReader) Storage(addr common.Address, slot common.Hash) (comm
|
||||||
return common.Hash{}, fmt.Errorf("bintrie storage read %x[%x]: %w", addr, slot, err)
|
return common.Hash{}, fmt.Errorf("bintrie storage read %x[%x]: %w", addr, slot, err)
|
||||||
}
|
}
|
||||||
if len(blob) == 0 {
|
if len(blob) == 0 {
|
||||||
return common.Hash{}, nil
|
// Flat state has no entry at this slot. Fall through to the
|
||||||
|
// trie reader to distinguish "never written" from "written
|
||||||
|
// then flushed out". See errBintrieFlatStateMiss.
|
||||||
|
return common.Hash{}, fmt.Errorf("%w: addr=%x slot=%x", errBintrieFlatStateMiss, addr, slot)
|
||||||
}
|
}
|
||||||
if len(blob) != 32 {
|
if len(blob) != 32 {
|
||||||
return common.Hash{}, fmt.Errorf("bintrie storage leaf invalid length: addr=%x slot=%x len=%d want=32", addr, slot, len(blob))
|
return common.Hash{}, fmt.Errorf("bintrie storage leaf invalid length: addr=%x slot=%x len=%d want=32", addr, slot, len(blob))
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,7 @@
|
||||||
package state
|
package state
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/ethereum/go-ethereum/common"
|
"github.com/ethereum/go-ethereum/common"
|
||||||
|
|
@ -134,10 +135,22 @@ func TestBintrieFlatReaderEndToEnd(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// TestBintrieFlatReaderMissingAccount verifies that an account never
|
// TestBintrieFlatReaderMissingAccountSentinel verifies that the bintrie
|
||||||
// touched by any commit returns (nil, nil) — the standard "account
|
// flat reader returns errBintrieFlatStateMiss (a non-nil error sentinel)
|
||||||
// doesn't exist" sentinel that the merkle flatReader also returns.
|
// for an account that was never written to the flat state.
|
||||||
func TestBintrieFlatReaderMissingAccount(t *testing.T) {
|
//
|
||||||
|
// Post-A2: the flat reader returns errBintrieFlatStateMiss so the
|
||||||
|
// multiStateReader falls through to the trie reader. This is the
|
||||||
|
// correct behavior: the flat state does not have the entry, so the
|
||||||
|
// trie reader should be the gatekeeper.
|
||||||
|
//
|
||||||
|
// KNOWN ISSUE: BinaryTrie.GetAccount does NOT verify stem membership —
|
||||||
|
// it returns the closest stem's data for ANY address query. So the trie
|
||||||
|
// reader currently returns wrong data for non-existent addresses. That
|
||||||
|
// is a pre-existing bintrie bug (not introduced by A2). This test
|
||||||
|
// therefore verifies the FLAT READER's sentinel error directly, in
|
||||||
|
// isolation from the buggy trie reader fallthrough path.
|
||||||
|
func TestBintrieFlatReaderMissingAccountSentinel(t *testing.T) {
|
||||||
disk := rawdb.NewMemoryDatabase()
|
disk := rawdb.NewMemoryDatabase()
|
||||||
tdb := triedb.NewDatabase(disk, triedb.VerkleDefaults)
|
tdb := triedb.NewDatabase(disk, triedb.VerkleDefaults)
|
||||||
sdb := NewDatabase(tdb, nil)
|
sdb := NewDatabase(tdb, nil)
|
||||||
|
|
@ -146,9 +159,7 @@ func TestBintrieFlatReaderMissingAccount(t *testing.T) {
|
||||||
t.Fatalf("init state: %v", err)
|
t.Fatalf("init state: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Touch addrA so the trie has at least one stem; otherwise we'd be
|
// Touch addrA so the trie has at least one stem.
|
||||||
// reading from an empty disk layer where everything is trivially
|
|
||||||
// absent.
|
|
||||||
addrA := common.HexToAddress("0x0101010101010101010101010101010101010101")
|
addrA := common.HexToAddress("0x0101010101010101010101010101010101010101")
|
||||||
state.SetBalance(addrA, uint256.NewInt(1), tracing.BalanceChangeUnspecified)
|
state.SetBalance(addrA, uint256.NewInt(1), tracing.BalanceChangeUnspecified)
|
||||||
root, err := state.Commit(0, true, false)
|
root, err := state.Commit(0, true, false)
|
||||||
|
|
@ -156,18 +167,25 @@ func TestBintrieFlatReaderMissingAccount(t *testing.T) {
|
||||||
t.Fatalf("commit: %v", err)
|
t.Fatalf("commit: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
reader, err := sdb.StateReader(root)
|
// Get the pathdb reader so we can test the bintrieFlatReader in
|
||||||
|
// isolation — not through multiStateReader (which would fall
|
||||||
|
// through to the buggy trie reader).
|
||||||
|
pathdbReader, err := tdb.StateReader(root)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatalf("StateReader: %v", err)
|
t.Fatalf("pathdb StateReader: %v", err)
|
||||||
|
}
|
||||||
|
br := newBintrieFlatReader(pathdbReader)
|
||||||
|
if br == nil {
|
||||||
|
t.Fatal("newBintrieFlatReader returned nil")
|
||||||
}
|
}
|
||||||
|
|
||||||
missing := common.HexToAddress("0xfeedfacefeedfacefeedfacefeedfacefeedface")
|
missing := common.HexToAddress("0xfeedfacefeedfacefeedfacefeedfacefeedface")
|
||||||
got, err := reader.Account(missing)
|
_, flatErr := br.Account(missing)
|
||||||
if err != nil {
|
if flatErr == nil {
|
||||||
t.Fatalf("Account(missing): %v", err)
|
t.Fatal("expected errBintrieFlatStateMiss for missing account, got nil error")
|
||||||
}
|
}
|
||||||
if got != nil {
|
if !errors.Is(flatErr, errBintrieFlatStateMiss) {
|
||||||
t.Errorf("missing account: got %+v, want nil", got)
|
t.Fatalf("expected errBintrieFlatStateMiss, got: %v", flatErr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue