diff --git a/core/state/reader.go b/core/state/reader.go index 27102602ea..833253207d 100644 --- a/core/state/reader.go +++ b/core/state/reader.go @@ -180,23 +180,6 @@ func (r *flatReader) Storage(addr common.Address, key common.Hash) (common.Hash, 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 // the StateReader interface backed by the path database's per-stem flat // state, doing the EIP-7864 key derivation locally so the underlying @@ -254,15 +237,8 @@ func newBintrieFlatReader(reader database.StateReader) *bintrieFlatReader { // Return value contract: // - both leaves 32 bytes → decoded Account, nil error. // - either leaf invalid length → corruption error, surfaced as-is. -// - both leaves absent (nil from AccountRLP) → errBintrieFlatStateMiss -// sentinel. This does NOT mean "account does not exist" — it means -// "the flat state has no data for this stem right now" (possibly -// because the generator hasn't reached it, or because the account -// has simply never been touched by a block that flushed). The -// 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. +// - both leaves absent → (nil, nil): authoritative non-membership. +// Uncovered keys already fail with errNotCoveredYet at the pathdb layer. func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) { basicKey := common.BytesToHash(bintrie.GetBinaryTreeKeyBasicData(addr)) codeKey := common.BytesToHash(bintrie.GetBinaryTreeKeyCodeHash(addr)) @@ -276,10 +252,7 @@ func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) { return nil, fmt.Errorf("bintrie CodeHash read %x: %w", addr, err) } if len(basicBlob) == 0 && len(codeBlob) == 0 { - // 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) + return nil, nil // Authoritative absence: pathdb confirmed key is covered } // A bintrie leaf is always either absent or exactly 32 bytes. A // shorter blob is a corruption signal; surface it with enough @@ -322,11 +295,9 @@ func (r *bintrieFlatReader) Account(addr common.Address) (*Account, error) { // Return value contract: // - 32-byte leaf found → decode as common.Hash and return. // - 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. +// - no leaf → (common.Hash{}, nil): authoritative non-membership. +// A slot explicitly set to zero is NOT absent — the bintrie +// tombstone convention writes 32 zero bytes (a present leaf). func (r *bintrieFlatReader) Storage(addr common.Address, slot common.Hash) (common.Hash, error) { fullKey := bintrie.GetBinaryTreeKeyStorageSlot(addr, slot[:]) blob, err := r.reader.AccountRLP(common.BytesToHash(fullKey)) @@ -334,10 +305,7 @@ 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) } if len(blob) == 0 { - // 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) + return common.Hash{}, nil // Authoritative absence: pathdb confirmed key is covered } 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)) diff --git a/core/state/reader_bintrie_oracle_test.go b/core/state/reader_bintrie_oracle_test.go index 33e42a7e95..5302c4c618 100644 --- a/core/state/reader_bintrie_oracle_test.go +++ b/core/state/reader_bintrie_oracle_test.go @@ -97,11 +97,10 @@ func TestBintrieFlatStateConsistencyOracle(t *testing.T) { t.Fatalf("block %d: StateReader: %v", blockNum, err) } - // For each known address, read via the flat reader. The flat - // reader may return errBintrieFlatStateMiss (which the - // multiStateReader falls through to the trie reader for), so - // the final answer comes from the highest-priority reader that - // has data. We compare the FINAL answer to what we expect. + // For each known address, read via the multiStateReader which + // tries the flat reader first (authoritative for covered keys) + // and falls through to the trie reader only if the pathdb + // returns errNotCoveredYet for keys not yet generated. for _, a := range addrs { got, err := flatReader.Account(a) if err != nil { diff --git a/core/state/reader_bintrie_test.go b/core/state/reader_bintrie_test.go index 518b82242a..dfb18aa899 100644 --- a/core/state/reader_bintrie_test.go +++ b/core/state/reader_bintrie_test.go @@ -17,7 +17,6 @@ package state import ( - "errors" "testing" "github.com/ethereum/go-ethereum/common" @@ -135,22 +134,9 @@ func TestBintrieFlatReaderEndToEnd(t *testing.T) { } } -// TestBintrieFlatReaderMissingAccountSentinel verifies that the bintrie -// flat reader returns errBintrieFlatStateMiss (a non-nil error sentinel) -// for an account that was never written to the flat state. -// -// 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) { +// TestBintrieFlatReaderMissingAccountAuthoritative verifies that the flat +// reader returns (nil, nil) for absent accounts after generation completes. +func TestBintrieFlatReaderMissingAccountAuthoritative(t *testing.T) { disk := rawdb.NewMemoryDatabase() tdb := triedb.NewDatabase(disk, triedb.VerkleDefaults) sdb := NewDatabase(tdb, nil) @@ -166,10 +152,13 @@ func TestBintrieFlatReaderMissingAccountSentinel(t *testing.T) { if err != nil { t.Fatalf("commit: %v", err) } + // Flush to disk so the generator completes (genMarker → nil). + if err := tdb.Commit(root, false); err != nil { + t.Fatalf("tdb.Commit (flush to disk): %v", err) + } // Get the pathdb reader so we can test the bintrieFlatReader in - // isolation — not through multiStateReader (which would fall - // through to the buggy trie reader). + // isolation. pathdbReader, err := tdb.StateReader(root) if err != nil { t.Fatalf("pathdb StateReader: %v", err) @@ -180,12 +169,12 @@ func TestBintrieFlatReaderMissingAccountSentinel(t *testing.T) { } missing := common.HexToAddress("0xfeedfacefeedfacefeedfacefeedfacefeedface") - _, flatErr := br.Account(missing) - if flatErr == nil { - t.Fatal("expected errBintrieFlatStateMiss for missing account, got nil error") + acct, err := br.Account(missing) + if err != nil { + t.Fatalf("expected authoritative nil for missing account, got error: %v", err) } - if !errors.Is(flatErr, errBintrieFlatStateMiss) { - t.Fatalf("expected errBintrieFlatStateMiss, got: %v", flatErr) + if acct != nil { + t.Fatalf("expected nil account for missing address, got: %+v", acct) } }