core/state,triedb/pathdb: bintrie flat state support #34706
Draft
CPerezz wants to merge 36 commits intoethereum:masterfrom
Draft
core/state,triedb/pathdb: bintrie flat state support #34706CPerezz wants to merge 36 commits intoethereum:masterfrom
CPerezz wants to merge 36 commits intoethereum:masterfrom
Conversation
binaryHasher.updateAccount computed codeLen from len(account.Code.Code), which is only non-zero when the code itself was modified in the current block. For balance- or nonce-only updates account.Code is nil and the computed codeLen was 0, silently overwriting the code_size field packed into the bintrie BasicData leaf (EIP-7864 bytes 5-7) with zero every time a contract was touched without a code write. The TODO(rjl493456442) on updateAccount acknowledged this. Fix it by adding a CodeSize field to AccountMut and having the caller at StateDB.IntermediateRoot populate it via stateObject.CodeSize(), which returns len(obj.code) when the bytes are loaded, otherwise falls back to a code-size lookup via the reader. The binary hasher then passes account.CodeSize straight to BinaryTrie.UpdateAccount as its codeLen argument, and the TODO is removed. Rationale for placing CodeSize on AccountMut rather than Account: AccountMut already carries Code *CodeMut — the new bytecode, which is not a field of Account — because code is write-time data that is not persisted in the flat-state format (SlimAccountRLP). CodeSize has the identical lifecycle: it is not in SlimAccountRLP, it is not populated by any reader, and it is only consumed by the hasher at write time. Mirroring Code's placement keeps the read-side/write-side split honest (Account models the persisted flat-state record; AccountMut adds the code-related write-time parameters). If the bintrie flat-state format is later extended to carry code_size, CodeSize can be promoted onto Account at that time. merkleHasher is unaffected: StateTrie.UpdateAccount ignores its codeLen parameter, so the wrapTrie.UpdateAccount shim continues to pass 0 and no state-root divergence is introduced on the MPT path. Regression test TestVerkleCodeSizePreserved verifies that the state root produced by "create contract, commit, reload, modify balance, commit" matches the root of a single-step construction of the same final state. Before the fix the roots diverge: path A (reload + balance): 1a675599... path B (fresh, same state): de0cfb03...
The bintrie node iterator previously discarded its `start` parameter, forcing every iteration to begin at the root. This makes resumable generators (snapshot/flat-state population) impossible — any interruption restarts from scratch. Implement seek(start []byte) by walking down the trie following start's bit path, building the iterator stack as we go. When the chosen path dead-ends (Empty, missing child, or a stem strictly less than start), backtrack through the existing stack to find the next in-order subtree and descend to its leftmost leaf. Also wire BinaryTrie.NodeIterator(startKey) to actually pass startKey through (was hardcoded to nil). Tests cover: empty start (no-op), exact key match, between-keys, into empty subtree, past end, within-stem offsets, resume simulation, and deep tree.
Introduce flatStateCodec, a small interface that captures the trie-specific aspects of flat-state storage: key derivation from (address, slot), persistence of account/storage entries, clean-cache key disambiguation, iterator setup, and progress-marker handling. Mirrors the existing nodeHasher pattern and complements the Hasher interface from state-hasher-iface-2 (which abstracts trie-side hashing and commit). The codec is stored on Database alongside the existing hasher field, ready to be threaded through the flat-state call sites (disklayer, flush, generator, reader) in the next commit. Provides merkleFlatCodec, a thin wrapper over the existing rawdb snapshot accessors and helpers. This is a pure refactor: behavior is unchanged. The bintrie-side codec implementation is added in a later commit, after all call sites have been routed through the abstraction.
Route the flatStateCodec from Database through every flat-state call
site so that the trie-specific aspects of persistence and key derivation
live behind a single abstraction. Pure refactor: merkle behavior and
on-disk layout are unchanged because the only codec wired up is
merkleFlatCodec, whose methods are thin wrappers over the existing
rawdb accessors.
Threaded sites:
disklayer.account/storage use codec.{Read,AccountCacheKey,
StorageCacheKey} instead of direct
rawdb calls and bare hash slicing.
flush.writeStates takes a codec parameter; persistence
goes through codec.{Write,Delete}
{Account,Storage}.
buffer.flush carries the codec down into writeStates.
states.write/dbsize takes the codec for prefix-size
accounting.
generate.go (g.codec) the generator owns a codec, used by
generateAccounts/generateStorages
callbacks; the unused top-level
splitMarker helper is removed in favor
of codec.SplitMarker.
context.go the generator context owns the codec
and uses codec.{AccountPrefix,
StoragePrefix,Account/StorageKeyLength}
to construct iterators.
reader.go (HistoricalState) uses codec.{Account,Storage}Key for
caller-side key derivation.
The marker comparisons in writeStates remain merkle-shaped (two-tier
account+storage marker) because the bintrie path will use a separate
writer over single-tier stem markers in a later commit.
All existing pathdb tests pass.
Reserve journal version 4 for the upcoming bintrie flat-state layout (per-stem blobs). Bumping now — with no on-disk format change yet — ensures that any v3 journals belonging to a bintrie database are discarded on load, so the new layout can be introduced cleanly in follow-up commits without a migration shim. MPT behavior is unchanged at this point: the only codec wired to the pathdb Database is still merkleFlatCodec. All pathdb, core/state, core/rawdb, and trie tests pass.
Reserve a new top-level key-value namespace for bintrie flat-state entries: BinTrieStemPrefix = "X" (chosen to be free at the top level so unwrapped-db callers do not collide with blockBodyPrefix "b"). In practice pathdb nests this namespace under VerklePrefix "v" via rawdb.NewTable, so the on-disk key is effectively "v"+"X"+stem. A stem is the 31-byte common prefix of the 32-byte tree key defined in EIP-7864. The stored value is a packed blob containing the populated (offset, value) pairs at that stem; BasicData (offset 0), CodeHash (offset 1), header storage (offsets 64-127), code chunks (offsets 128-255) and main-storage slots can all be extracted from the same blob by a single Pebble read, matching the on-trie StemNode grouping. Add Read/Write/DeleteBinTrieStem alongside the existing snapshot accessors, and a private binTrieStemKey helper in schema.go. No callers yet — the bintrieFlatCodec in the next commit consumes them. Matches the same style as ReadAccountSnapshot et al.; the codec and integration tests downstream will exercise them.
Introduce the codec and on-disk blob format for the bintrie flat-state layer. This commit only defines the types; the codec is NOT wired into pathdb.Database.New yet (that happens in a later commit once the leaf-production hook in binaryHasher and the stateUpdate wiring are in place). Three pieces: 1. trie/bintrie/pack.go Canonical PackBasicData / UnpackBasicData helpers that encode an account's (codeSize, nonce, balance) into the 32-byte BasicData leaf defined by EIP-7864. Preserves the existing BinaryTrie.UpdateAccount layout byte-for-byte (4-byte code_size at offset 4 rather than the spec's 3-byte field at offset 5 — any realistic code size has byte 4 always zero and the two encodings are bit-equivalent in practice). BinaryTrie.UpdateAccount is refactored to delegate to PackBasicData so the flat-state codec can produce a bit-identical BasicData encoding without duplicating the layout logic. 2. triedb/pathdb/stem_blob.go Packed encoding of the populated (offset, value) pairs at a bintrie stem. A stem can hold up to 256 offsets per EIP-7864 but in practice only a handful are set; the layout is a 32-byte bitmap followed by N 32-byte values in ascending offset order, where N = popcount. Empty stems encode to nil so the caller knows to delete the on-disk key rather than write a zero-length value. Provides encodeStemBlob / decodeStemBlob / extractStemOffset / mergeStemBlob and a stemBuilder type for accumulating writes. The tombstone convention (32 zero bytes = "present with zero" as used by DeleteStorage) is preserved. 11 unit tests cover: empty blob, BasicData+CodeHash roundtrip, all 256 offsets populated, sparse high offsets, set/clear roundtrip, load-from-existing-blob RMW, merge helper, merge-to-empty, tombstone zero bytes, malformed input detection, bitmap rank sanity. 3. triedb/pathdb/flat_codec_bintrie.go bintrieFlatCodec implements flatStateCodec over the stem-blob layout. Unlike merkleFlatCodec it is stateful: it holds a ethdb.KeyValueReader reference used by applyWrites to read the existing stem blob before merging in new writes. ethdb.Batch is write-only so the batch passed to Write* cannot be used to fetch current state. Pre-aggregation requirement is documented explicitly: within a single flush, the caller must NOT issue two Write* calls targeting the same stem, because the RMW read comes from the store (not the in-flight batch). Commit 8 of the bintrie flat-state plan restructures writeStates to pre-aggregate per-stem writes so callers don't have to handle this manually. Cache keys are prefix-disambiguated with a one-byte 0x01 to keep bintrie stem lookups disjoint from merkle 32-byte account keys and 64-byte storage keys in the shared clean-state fastcache. SplitMarker is a single-tier (stem-only) format, not the merkle two-tier (account, account+storage) format. 7 unit tests cover: account roundtrip, storage roundtrip, multiple writes to the same stem, DeleteAccount preserving unrelated offsets, DeleteStorage removing the final offset collapsing the key, cache key disjointness from merkle, SplitMarker semantics. The codec is not dispatched by anything yet; MPT continues through the merkle codec and bintrie mode still runs on the (soon-to-be-replaced) keccak-shaped path until Commit 10 wires things up.
binaryHasher now implements the new LeafProducer optional extension to
the Hasher interface. Every UpdateAccount, UpdateStorage, and delete
path records the corresponding (stem, offset, value) write into an
internal buffer, which the caller drains once per block via
DrainStemWrites() and hands to the pathdb flat-state layer through the
stateUpdate (wired up in the next commit).
Three kinds of writes are recorded:
- Account create/update: two writes (BasicData at offset 0,
CodeHash at offset 1), sharing the same 31-byte stem. BasicData
is produced via bintrie.PackBasicData so the flat-state blob
is bit-identical to what the trie layer packs internally.
- Storage update: one write per slot. Non-zero values become
right-justified 32-byte blobs; the zero value (the bintrie's
"delete" convention) becomes 32 zero bytes, matching the trie's
tombstone-with-zero semantics so the flat-state mirror stays
bit-identical to the StemNode.Values entry.
- Account delete: two clear writes (nil Value) for offsets 0 and 1.
Storage slots and code chunks at the same or other stems are NOT
touched; pre-EIP-6780 full-wipe is a documented scope limitation.
The LeafProducer interface lives on Hasher and is strictly opt-in —
merkleHasher does not implement it, and callers detect capability via
a type assertion. This keeps the read-side/write-side split of the
existing Hasher cleanly extended: hashers that have a concept of
flat-state leaves can expose them; hashers that don't (MPT) are
unaffected.
Tests cover:
- TestBinaryHasherLeafProduction: account update produces 2 writes
at offsets 0+1 with matching stem; drain is destructive; storage
update emits one matching write; zero-value storage writes 32 zero
bytes; delete emits 2 clear writes.
- TestMerkleHasherNoLeafProducer: merkleHasher does NOT satisfy the
LeafProducer interface (the capability is opt-in per hasher).
The collected stem writes are not yet propagated anywhere — a later
commit wires DrainStemWrites into StateDB.IntermediateRoot so the
writes flow through stateUpdate and the pathdb stateSet into the
flat-state layer.
Drains the binaryHasher's LeafProducer side-channel in StateDB.commit and threads the stem writes through stateUpdate.encodeBinary into the pathdb state set as per-offset accountData entries (key = stem||offset, value = 32-byte leaf or nil for clears). The flat-state codec gains a Flush method that owns the in-memory→disk write path, replacing the codec-agnostic per-entry loop in writeStates. The merkle codec preserves its historical per-entry behavior verbatim; the bintrie codec aggregates per-offset writes by stem so each stem hits disk via a single read-modify-write, satisfying the codec's pre-aggregation requirement and updating the clean cache with the merged blob it just produced (no extra disk read). stateUpdate.encodeBinary returns empty origin maps for the bintrie path: state-history rollback for bintrie is deferred to a follow-up PR (see BINTRIE_FLAT_STATE_REORG_GAP.md), and the diskLayer.revert path will panic before consuming origins anyway.
Adds generateBinTrieStems, the bintrie analogue of generateAccounts. It opens the bintrie via a sha256-aware bintrieDiskStore (the merkle disk store would always fail root validation against a binary node), iterates all leaves with binaryNodeIterator, aggregates them into per-stem builders, and emits one stem blob per stem boundary. Resume support is structural: ctx.marker is fed straight to the trie's NodeIterator, which uses binaryNodeIterator.seek (Commit 1) to position on the first leaf >= marker. Range proofs are deliberately skipped — the bintrie's Prove path is unimplemented and an iteration-only generation cycle is acceptable for a one-time startup cost. A bintrieGeneratorContext mirrors generatorContext but is much smaller: no holdable iterators (we walk the trie, not the existing flat state) and no two-tier marker (the bintrie key space is unified). checkAndFlushBin journals progress as a single 32-byte (stem || offset) key so resume can pick up mid-stem. generator.run dispatches on codec type so callers see a uniform lifecycle whether the underlying scheme is merkle or bintrie.
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.
Wires the pieces from Commits 1-9 into a running system: * triedb/pathdb.New: install the bintrieFlatCodec when isVerkle is set, backed by the same verkle-namespaced db used for trie nodes. * triedb/pathdb.database.go: drop isVerkle from the noBuild guard so the bintrie generator (Commit 9) runs on startup, and remove it from the generateSnapshot call path for the same reason. * triedb/pathdb.disklayer.revert: hard-fail on bintrie because the reorg path would replay merkle-shaped origin records against a per-stem layout. Tracked in BINTRIE_FLAT_STATE_REORG_GAP.md. * triedb/pathdb.journal: add IsBintrie to journalGenerator (rlp:"optional" so v3 journals still decode) and make journalProgress a method on generator so it stamps the active scheme; loadGenerator discards any journal whose scheme does not match the database, forcing a fresh regeneration. * triedb/pathdb.reader: export RawStateReader, a small extension of database.StateReader that exposes AccountRLP so callers outside the package can reach the raw flat-state bytes without going through the slim-RLP decode path that assumes merkle shape. * core/state.reader: add bintrieFlatReader, the bintrie equivalent of flatReader. It derives the EIP-7864 stem keys from (addr, slot), performs two AccountRLP lookups per Account call (BasicData + CodeHash), and decodes via bintrie.UnpackBasicData. Storage reads go through a single AccountRLP lookup at the slot's full bintrie key. * core/state.database.StateReader: dispatch to bintrieFlatReader when the path database is in verkle mode; merkle path unchanged. Depends on the lookup sentinel fix in the previous commit; without it missing-account reads on bintrie misreport as "layer stale".
…per-offset extraction
Addresses review finding C2 (+ I5, S5, T2, T3, T12).
Before this commit, bintrieFlatCodec.ReadAccount returned the FULL
variable-length stem blob from disk while the in-memory diff-layer
buffer stored per-offset 32-byte values. The consumer,
bintrieFlatReader.Account, enforced len(basicBlob)!=32 → error, so
every disk-layer hit produced "bintrie BasicData leaf invalid length"
in production the moment the write buffer flushed. TestBintrieFlatReaderEndToEnd
did not catch this because it never forced a buffer → disk flush.
Fix: make bintrieFlatCodec.ReadAccount extract the offset from the
stem blob (mirroring ReadStorage), so the disk path and the buffer
path return the same 32-byte per-offset shape. Update
AccountCacheKey/StorageCacheKey to embed the full 32-byte key
(prefix + 31-byte stem + 1-byte offset), since caching under a
stem-only key would collapse BasicData and CodeHash into the same
slot and return the wrong value on the second hit. Update
Flush's cache-update loop to store per-offset entries from the
aggregated write set.
Design note: I considered the alternative of introducing a new
StemBlob(stem) interface method that returns the full blob synthesized
from a stem-level lookup index. Rejected because (a) the index is a
new data structure with its own consistency invariants, (b) the
per-offset approach is strictly local to the codec + reader, and (c)
the "1 Pebble read per Account" locality benefit is preserved at the
OS page cache level — both offsets at the same stem live in the same
Pebble block, so the second read is effectively free.
bintrieFlatReader.Account still does two AccountRLP lookups; the
torn-read hazard is gated by a new load-bearing invariant test,
TestBinaryHasherWritesBothBasicAndCodeHash, which asserts that
binaryHasher.updateAccount always emits both BasicData and CodeHash
leaves together. A future code-only update that broke this invariant
would fail the test.
Tests added:
* TestBintrieFlatReaderEndToEndAfterFlush — explicitly flushes via
tdb.Commit(root, false) and re-reads through a fresh StateReader.
This is the smoking-gun regression for C2.
* TestBintrieFlatReaderMultipleOffsetsPerStem — multiple offsets at
the same stem (BasicData, CodeHash, header storage slots) all
round-trip post-flush.
* TestBintrieCodecCrossFlushRMW — two Flush calls to the same stem
from different "blocks" correctly merge on disk, with prior
offsets preserved.
* TestBinaryHasherWritesBothBasicAndCodeHash — locks down the hasher
co-write invariant that bintrieFlatReader.Account relies on.
Existing tests updated to match the new per-offset ReadAccount
semantics:
* TestBintrieCodecAccountRoundTrip, TestBintrieCodecMultipleWritesSameStem,
TestBintrieCodecDeleteAccount — now read per-offset rather than
calling extractStemOffset on the raw blob.
* TestBintrieCodecCacheKeysDisjoint — additionally verifies two
offsets at the same stem produce distinct cache keys.
Error messages in bintrieFlatReader now include address and length
context (S5).
…atReader 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.
Addresses review finding C1.
Before this commit, flushStem in generateBinTrieStems used
builder.encode() to overwrite the on-disk stem blob unconditionally.
When a crash+restart interrupted generation mid-stem (e.g., at offset 3
of stemA), the resume iterator positioned at stemA||3, the builder
accumulated only offsets 3+, and flushStem overwrote the disk blob with
a partial result — silently losing offsets 0, 1, 2 that were written in
the prior pass.
Fix: make flushStem a read-modify-write. It now reads the existing
on-disk stem blob (if any), converts the builder's accumulated offsets
to []stemOffsetValue via a new toOffsetValues() helper, and merges them
via the existing mergeStemBlob function. The merge semantics are
"builder values win" — new offsets overwrite their existing counterparts,
and gaps are filled from the prior blob. This makes the RMW idempotent
across resume cycles: the same stem can be re-walked from any midpoint
and the final disk blob always contains the union of all passes.
New helper: stemBuilder.toOffsetValues() converts the builder's
populated bitmap entries into a []stemOffsetValue slice suitable for
mergeStemBlob. ~20 LOC in stem_blob.go.
Tests:
* TestBintrieGeneratorResumeMidStem — pre-seeds disk with a partial
stem (offsets 0, 1), resumes generator at offset 1, asserts all
offsets survive including the pre-seeded offset 0. Before the fix
this test fails with "BasicData lost after mid-stem resume".
* TestBintrieGeneratorResumeStemBoundary — renamed from the original
TestBintrieGeneratorResume, unchanged behavior (stem-boundary
resume).
…storicStateReader rlp.Split bug
Addresses review finding C4 + Opus agent audit secondary bug.
Bug 1 — fail-open gate in disklayer.storage:
disklayer.storage() compared a 64-byte merkle-shaped combinedKey
(accountHash || storageHash) against the 32-byte bintrie generator
marker via codec.MarkerCompare. For bintrie, accountHash is always
common.Hash{} (since bintrieFlatCodec.StorageKey returns zero for
the account key), so the combinedKey started with 32 zero bytes.
The sha256-derived marker's first byte is essentially never 0x00,
so bytes.Compare returned -1, the > 0 branch never fired, and the
generator-progress gate was silently DISABLED. During active
generation, disklayer.storage served whatever was on disk (nil or
stale) without returning errNotCoveredYet.
Fix: add StorageMarkerKey(accountHash, storageHash) to the
flatStateCodec interface. Merkle returns the 64-byte concatenation
(preserving existing behavior); bintrie returns storageHash[:]
(the 32-byte stem||offset key matching the generator marker shape).
disklayer.storage now uses the codec method.
Bug 2 — rlp.Split on raw bintrie storage leaves in historicStateReader:
historicStateReader.Storage at core/state/database_history.go:87
calls rlp.Split(blob) on whatever bytes the pathdb historical reader
returns. Merkle storage values are RLP-encoded (trimmed-left-zeros);
bintrie leaves are raw 32 bytes. rlp.Split on raw 32-byte input
either errors or decodes garbage. Even after fixing Bug 1, bintrie
historical storage reads were broken end-to-end.
Fix: add isVerkle bool to historicStateReader; when true, bypass
rlp.Split and copy the raw 32-byte blob directly. The flag is set
from db.triedb.IsVerkle() at construction time.
Addresses review finding I1. diskLayer.node() returned crypto.Keccak256Hash(blob) as the hash for ALL trie nodes regardless of the database's scheme. For the binary trie the correct hash is sha256 via binaryNodeHasher. The wrong hash was masked by noHashCheck=true in pathdb.NodeReader for the bintrie path, but HistoricalNodeReader.Node (which does NOT set noHashCheck) would never match the returned hash, silently falling through to the slow freezer-backed read on every call. Fix: replace crypto.Keccak256Hash(blob) with dl.db.hasher(blob) at both the clean-cache-hit and disk-read return paths. The hasher is already set to the correct function (merkleNodeHasher or binaryNodeHasher) at Database construction time.
… Flush Addresses review finding I2. The bintrieFlatCodec had a crit() helper whose doc claimed "delegates to log.Crit" but whose body was panic(fmt.Sprintf(...)). A corrupt on-disk stem blob would cause the buffer flush goroutine to panic, killing the process. On restart the same blob would cause the same panic — unrecoverable crash loop. Fix: applyWrites now returns ([]byte, error) instead of panicking. The Flush method on flatStateCodec gains an error return: (int, int) → (int, int, error) The error propagates up through writeStates → stateSet.write → buffer.flush → flushErr. A corrupted stem blob now causes a flush failure that the database can react to instead of a crash loop. The per-entry methods (WriteAccount, WriteStorage, DeleteAccount, DeleteStorage) — which are NOT on the production flush path — use log.Crit (the real function, not the deleted shim) on error, matching the merkle codec's existing convention for unrecoverable corruption at the per-entry level. The crit shim is deleted entirely.
… and encodeStemBlob Addresses review findings I13 and S6. encodeBinary: reject non-nil bintrie leaves with length != 32 at the trust boundary between the hasher and the state update. Previously a wrong-length leaf silently made it into the diff layer's accountData and only surfaced as a panic deep in the Flush path (stemBuilder.set). encodeStemBlob: add an upper-bound check on the value count (must be <= 256, the maximum offsets per stem). Previously a buggy producer could pass an arbitrarily long values slice.
…, multi-block evolution, and contract code generation
Addresses review test gaps T8, T9, T10.
TestBintrieFlatReaderStorageTombstone (T8):
Write slot=0x42 in block 1; clear to zero in block 2. Read at
block 2 → common.Hash{} (tombstone, not absent). Read at block 1 →
0x42 (original value preserved in the diff layer chain).
TestBintrieFlatReaderMultiBlockEvolution (T9):
Write nonce=1/balance=100 in block 1, nonce=2 in block 2,
balance=200 in block 3. Open StateReaders at each root and assert
the correct snapshot for each block. Validates diff-layer chaining
under the bintrie path.
TestBintrieGeneratorWithContractCode (T10):
Build a bintrie with a contract having ~100 bytes of code (4
chunks at offsets 128..131). Run the generator and verify code-chunk
offsets appear in the resulting stem blob. Validates the plan's
claim that code chunks are handled by the generator.
…markers Addresses review suggestion S2/S7/S22. Remove "NOT wired in yet", "in a later commit", and "Commit N" references that were accurate at the time of their original commit but became stale after subsequent commits landed on the same branch. Update cross-references to name the actual functions and files rather than commit numbers. Specific fixes: * flat_codec_bintrie.go:50-57: "NOT wired" → "wired when isVerkle" * flat_codec_bintrie.go:360: "in a later commit" → "generate_bintrie.go" * database_hasher_binary.go:132: "in a later commit" → "StateDB.commit()" * journal.go:53-56: v4 comment updated from "reserved for" → actual description
Comprehensive validation suite designed to confirm the flat state is working before running benchmarks. TestBintrieFlatStateConsistencyOracle (core/state): The "if this passes, we're good to benchmark" test. Builds realistic state over 15 blocks (48 accounts, 14 storage slots), explicitly flushes to disk at block 5, and verifies structural invariants of every flat-state read at every block commit. Covers: - Diff-layer chaining across 15 blocks - Disk-layer reads after explicit flush (A1 smoking gun) - Account creation, balance/nonce modification, code deployment - Storage write, update, and tombstone (zero-value clear) - Post-flush state evolution with fresh diff layers on top of disk Stem blob edge cases (triedb/pathdb): TestStemBlobOffset127_128Boundary — validates the bitmap byte boundary between the last header storage offset (127) and the first code chunk offset (128). A bitmapRank off-by-one here would cause extractStemOffset to return the adjacent offset's value. TestStemBlobFull256DeleteMiddle — fully populates all 256 offsets, deletes one from the middle (offset 128), verifies all 255 remaining values survive at the correct positions. TestFlushIdempotency — verifies that flushing the same data twice produces a byte-identical on-disk blob. The RMW in applyWrites must be idempotent to handle overlapping diff-layer merges.
commitAndFlush discarded errors from db.Reader() and db.Hasher() at
lines 1040-1041, storing the result directly into s.reader/s.hasher.
When Hasher() fails (e.g. newBinaryHasher returns nil, err), Go wraps
the nil *binaryHasher in the Hasher interface as a typed nil — an
interface value {type: *binaryHasher, value: nil} that passes == nil
checks. StopPrefetcher then type-asserts to Prefetcher (succeeds) and
calls TermPrefetch on the nil receiver, panicking at h.trie.term().
Fix: propagate errors from Reader() and Hasher() in commitAndFlush
instead of discarding them, and add defensive nil-receiver guards to
both binaryHasher.TermPrefetch and merkleHasher.TermPrefetch.
Contributor
Author
|
Built on top of @rjl493456442 's Hasher interface. Will rebase once he PRs and it gets merged. But at least I can show the code to others easily. Collecting benchmarks atm against MPT. Hopefully this leaves BT much closer to the performance target. |
CPerezz
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements per-stem flat state storage for the binary trie (EIP-7864), closing the ~1.7x read performance gap vs MPT by enabling snapshot-style reads for bintrie accounts and storage.
This PR includes:
Hasherinterface) — decouples trie hashing fromStateDB, withmerkleHasherandbinaryHasherimplementations. (From @rjl493456442, will rebase once his PR lands).flatStateCodecabstraction — interface abstracting flat-state key derivation, persistence, cache keys, and flush logic for both MPT (merkleFlatCodec) and bintrie (bintrieFlatCodec)BinTrieStemPrefix), read-modify-write flushgenerateBinTrieStemswith RMWflushStem, sha256 validation, mid-stem resume support viamergeStemBlobLeafProducerinterface onbinaryHashertracking flat-state mutations alongside trie updatesbintrieFlatReader— flat-state reader with per-offset extraction,errBintrieFlatStateMisssentinel for trie fallbackbinaryNodeIterator.seek()for generator range iterationStopPrefetcher+ defensive nil-receiver guards