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.
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 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 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).
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".
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.
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.
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.
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...
StateDB.Commit first commits all storage changes into the storage trie,
then updates the account metadata with the new storage root into the
account trie.
Within StateDB.Commit, the new storage trie root has already been
computed and applied as the storage root. This PR explicitly skips the
redundant storage trie root assignment for readability.
This PR simplifies the implementation of EIP-7610 by eliminating the
need to check storage emptiness during contract deployment.
EIP-7610 specifies that contract creation must be rejected if the
destination account has a non-zero nonce, non-empty runtime code, or
**non-empty storage**.
After EIP-161, all newly deployed contracts are initialized with a nonce
of one. As a result, such accounts are no longer eligible as deployment
targets unless they are explicitly cleared.
However, prior to EIP-161, contracts were initialized with a nonce of
zero. This made it possible to end up with accounts that have:
- zero nonce
- empty runtime code
- non-empty storage (created during constructor execution)
- non-zero balance
These edge-case accounts complicate the storage emptiness check.
In practice, contract addresses are derived using one of the following
formulas:
- `Keccak256(rlp({sender, nonce}))[12:]`
- `Keccak256([]byte{0xff}, sender, salt[:], initHash)[12:]`
As such, an existing address is not selected as a deployment target
unless a collision occurs, which is extremely unlikely.
---
Previously, verifying storage emptiness relied on GetStorageRoot.
However, with the transition to the block-based access list (BAL),
the storage root is no longer available, as computing it would require
reconstructing the full storage trie from all mutations of preceding
transactions.
To address this, this PR introduces a simplified approach: it hardcodes
the set of known accounts that have zero nonce, empty runtime code,
but non-empty storage and non-zero balance. During contract deployment,
if the destination address belongs to this set, the deployment is
rejected.
This check is applied retroactively back to genesis. Since no address
collision events have occurred in Ethereum’s history, this change does
not
alter existing behavior. Instead, it serves as a safeguard for future
state
transitions.
runtime.setDefaults was unconditionally assigning cfg.Random =
&common.Hash{}, which silently overwrote any caller-provided Random
value. This made it impossible to simulate a specific PREVRANDAO and
also forced post-merge rules whenever London was active, regardless of
the intended environment.
This change only initializes cfg.Random when it is nil, matching how
other fields in Config are defaulted. Existing callers that did not set
Random keep the same behavior (a non-nil zero hash still enables
post-merge semantics), while callers that explicitly set Random now get
their value respected.
Return ErrInvalidOpCode with the executing opcode and offending
immediate for forbidden DUPN, SWAPN, and EXCHANGE operands. Extend
TestEIP8024_Execution to assert both opcode and operand for all
invalid-immediate paths.
This PR fixes https://github.com/ethereum/go-ethereum/issues/34623 by
changing the `vm.StateDB` interface:
Instead of `EmitLogsForBurnAccounts()` emitting burn logs, `LogsForBurnAccounts()
[]*types.Log` just returns these logs which are then emitted by the caller.
This way when tracing is used, `hookedStateDB.AddLog` will be used
automatically and there is no need to duplicate either the burn log
logic or the `OnLog` tracing hook.
ProcessBeaconBlockRoot (EIP-4788) and processRequestsSystemCall
(EIP-7002/7251) do not merge the EVM access events into the state after
execution. ProcessParentBlockHash (EIP-2935) already does this correctly
at line 290-291.
Without this merge, the Verkle witness will be missing the storage
accesses from the beacon root and request system calls, leading to
incomplete witnesses and potential consensus issues when Verkle
activates.
In this PR, the Database interface in `core/state` has been extended
with one more function:
```go
// Iteratee returns a state iteratee associated with the specified state root,
// through which the account iterator and storage iterator can be created.
Iteratee(root common.Hash) (Iteratee, error)
```
With this additional abstraction layer, the implementation details can be hidden
behind the interface. For example, state traversal can now operate directly on
the flat state for Verkle or binary trees, which do not natively support traversal.
Moreover, state dumping will now prefer using the flat state iterator as
the primary option, offering better efficiency.
Edit: this PR also fixes a tiny issue in the state dump, marshalling the
next field in the correct way.
Add persistent storage for Block Access Lists (BALs) in `core/rawdb/`.
This provides read/write/delete accessors for BALs in the active
key-value store.
---------
Co-authored-by: Jared Wasinger <j-wasinger@hotmail.com>
Co-authored-by: Gary Rong <garyrong0905@gmail.com>
`pool.signer.Sender(tx)` bypasses the sender cache used by types.Sender,
which can force an extra signature recovery for every promotable tx
(promotion runs frequently). Use `types.Sender(pool.signer, tx)` here to
keep sender derivation cached and consistent.
Problem:
The max-initcode sentinel moved from core to vm, but RPC pre-check
mapping still depended on core.ErrMaxInitCodeSizeExceeded. This mismatch
could surface inconsistent error mapping when oversized initcode is
submitted through JSON-RPC.
Solution:
- Remove core.ErrMaxInitCodeSizeExceeded from the core pre-check error
set.
- Map max-initcode validation errors in RPC from
vm.ErrMaxInitCodeSizeExceeded.
- Keep the RPC error code mapping unchanged (-38025).
Impact:
- Restores consistent max-initcode error mapping after the sentinel
move.
- Preserves existing JSON-RPC client expectations for error code -38025.
- No consensus, state, or protocol behavior changes.
Fix incorrect key length calculation for `numHashPairings` in
`InspectDatabase`, introduced in #34000.
The `headerHashKey` format is `headerPrefix + num + headerHashSuffix`
(10 bytes), but the check incorrectly included `common.HashLength`,
expecting 42 bytes.
This caused all number -- hash entries to be misclassified as
unaccounted data.
## Summary
In binary trie mode, `IntermediateRoot` calls `updateTrie()` once per
dirty account. But with the binary trie there is only one unified trie
(`OpenStorageTrie` returns `self`), so each call redundantly does
per-account trie setup: `getPrefetchedTrie`, `getTrie`, slice
allocations for deletions/used, and `prefetcher.used` — all for the same
trie pointer.
This PR replaces the per-account `updateTrie()` calls with a single flat
loop that applies all storage updates directly to `s.trie`. The MPT path
is unchanged. The prefetcher trie replacement is guarded to avoid
overwriting the binary trie that received updates.
This is the phase-1 counterpart to #34021 (H01). H01 fixes the commit
phase (`trie.Commit()` called N+1 times). This PR fixes the update phase
(`updateTrie()` called N times with redundant setup). Same root cause —
unified binary trie operated on per-account — different phases.
## Benchmark (Apple M4 Pro, 500K entries, `--benchtime=10s --count=3`,
on top of #34021)
| Metric | H01 baseline | H01 + this PR | Delta |
|--------|:------------:|:-------------:|:-----:|
| Approve (Mgas/s) | 368 | **414** | **+12.5%** |
| BalanceOf (Mgas/s) | 870 | 875 | +0.6% |
Should be rebased after #34021 is merged.
