feat(eip7928): move BAL tracking to Journal; eliminate phantom entries#21
feat(eip7928): move BAL tracking to Journal; eliminate phantom entries#21Gabriel-Trintinalia wants to merge 10 commits intoConsensys:mainfrom
Conversation
f0c8828 to
1224e35
Compare
Add the journal/database plumbing required for correct EIP-7928 Block Access List construction: - primitives: fix TX_GAS_LIMIT_CAP to 16777216 (EIP-7825 value) - database: FallbackFns vtable gains commit_tx/discard_tx/snapshot_frame/ commit_frame/revert_frame/untrack_address hooks so a stateless witness DB can track accesses with frame-accurate reverts - context/journal: lazy access-list pre-warming (warmAccessList / setAccessList) replaces eager loadAccountWithCode+sload; adds isAddressCold/isStorageCold predicates; StorageChanged journal entry now records old_was_written for correct revert; snapshotFrame/commitFrame/revertFrame forwarded to db - handler/mainnet_builder: use warmAccessList for EIP-2929 pre-warming; call snapshotFrame/revertFrame around top-level CALL value transfer - interpreter/host: propagate snapshotFrame/commitFrame/revertFrame through CALL/CREATE frame lifecycle - interpreter/opcodes/call + host_ops: use isAddressCold/isStorageCold for pre-call gas checks without triggering DB loads; untrack_address on OOG - state: storage slot gains was_written flag for revert correctness Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove DBG prints from InMemoryDB.basic and JournalInner.setCodeWithHash - Fix isAddressCold: only treat coinbase as cross-tx warm (EIP-3651), not all previously-loaded addresses, to avoid unintended gas discounts for precompiles and access-list entries Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Split the CALL instruction OOG check into two stages to match EELS: 1. If remaining < call_cost_no_delegation: untrack target (oog_before_target_access) 2. If remaining < base_cost (includes delegation): halt OOG without untracking target (oog_after_target_access and oog_success_minus_1 — target IS in BAL, delegation NOT) Also add unconditional untrackAddress for OOG in EXTCODECOPY and CALL-type opcodes when gas is exhausted before the target is accessed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove FallbackFns C-style vtable and InMemoryDB.fallback field - Make Context generic: Context(comptime DB: type); DefaultContext = Context(InMemoryDB) - Add Journal(DB) tracking wrappers guarded by @hasDecl (snapshotFrame, commitFrame, revertFrame, commitTracking, discardTracking, notifyStorageSlotCommit, hasNonZeroStorageForAddress, untrackAddress, forceTrackAddress) - Replace Host.ctx with type-erased JournalVTable (18 entries); Host.init(DB, ctx, prec) and Host.fromCtx(ctx, prec) constructors; opcode handlers unchanged - Make Evm generic: EvmFor(comptime DB: type); Evm = EvmFor(InMemoryDB) alias - Make MainnetHandler.* functions accept anytype evm for zero-cost duck typing - Fix getDb() dangling pointer: change self: @this() → self: *const @this() - Update test files to use Host.fromCtx() instead of Host{ .ctx = ... } literal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ecute - Add nonzero_storage_count index to InMemoryDB maintained by new private putStorage helper; hasNonZeroStorageForAddress is now O(1) instead of O(n) storage scan on every CREATE - Drop explicit `comptime DB: type` parameter from Frame.execute; infer DB via anytype ctx to remove parameter sprawl Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JournalInner now maintains its own always-on BAL tracking state: - bal_pre_accounts / bal_pre_storage: permanent pre-block snapshots - bal_pending_accounts / bal_pending_storage: per-tx staging - bal_committed_changed: slots dirty at any tx boundary - bal_untracked: OOG phantom addresses commitTx() absorbs committed-changed detection (was in mainnet_builder) and flushes pending → pre. discardTx() clears pending without touching permanent state. Account/storage pre-states are recorded inline in loadAccountMutOptionalCode and sload at first access. Journal(DB) wrapper gains unconditional untrackAddress, forceTrackAddress, isTrackedAddress, and takeAccessLog methods. All 9 @hasDecl-guarded tracking callbacks (snapshotFrame, commitFrame, revertFrame, commitTracking, discardTracking, notifyStorageSlotCommit, notifyStorageRead + the 2 untrack/forceTrack guards) are removed. Frame tracking is eliminated entirely: EIP-7928 keeps reverted accesses so commit==revert for frames. DB is now a pure 4-method storage backend. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously, addresses could be loaded into the Journal (and thus the EIP-7928 BAL) during gas calculation and then immediately untracked when the operation ran OOG. This is replaced with worst-case pre-checks that abort before any DB load. CALL: replace access_cost pre-check with getCallGasCost(spec, pre_is_cold, transfers_value, false) worst-case check before loading the target account. EXTCODECOPY: charge warm/cold + copy + memory gas before h.codeInfo(). SELFDESTRUCT: worst-case pre-check (cold + G_NEWACCOUNT) before h.selfdestruct(). CREATE: move Amsterdam balance check before js.loadAccount(new_addr). Since phantoms can no longer enter the BAL, remove the entire untracking machinery: untrackAddress, forceTrackAddress, bal_untracked from JournalInner; the corresponding Journal(DB) wrapper methods; and untrackAddress/forceTrackAddress from the JournalVTable and Host. isTrackedAddress simplified (no untracked set to consult). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1224e35 to
72ed824
Compare
| if (ctx.interpreter.gas.remaining < max_dyn_gas) { | ||
| ctx.interpreter.halt(.out_of_gas); | ||
| return; | ||
| } |
There was a problem hiding this comment.
SELFDESTRUCT worst-case pre-check causes false OOG pre-Amsterdam
High Severity
The max_dyn_gas unconditionally adds 25000 for Tangerine through pre-Amsterdam, but the actual dyn_gas only includes 25000 when selfdestruct_charges_new_account is true (target doesn't exist, and for Spurious Dragon+ self has value). A SELFDESTRUCT to a warm existing account on Berlin with fewer than 25000 gas remaining will falsely OOG when the actual dynamic cost is 0. The pre-check condition needs to be gated on amsterdam rather than tangerine, since phantom BAL entries are only relevant for Amsterdam.
Reviewed by Cursor Bugbot for commit 72ed824. Configure here.
- isAddressCold: use warm_addresses.isCold() for stale-tx accounts instead of coinbase-only check, covering precompiles and EIP-2930 access-list entries (fixes false cold report for cached accounts) - CALL worst-case pre-check: gate on .amsterdam instead of .berlin; pre-Amsterdam has no phantom BAL entries and the G_NEWACCOUNT worst-case caused false OOG for existing accounts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Reviewed by Cursor Bugbot for commit a3f8e92. Configure here.
| self.bal_pending_accounts.clearRetainingCapacity(); | ||
| var ps_it = self.bal_pending_storage.valueIterator(); | ||
| while (ps_it.next()) |m| m.deinit(); | ||
| self.bal_pending_storage.clearRetainingCapacity(); |
There was a problem hiding this comment.
finalize() omits clearing new BAL tracking maps
Medium Severity
finalize() is documented to "clear the journal by resetting it to initial state," but it doesn't clear or deinit any of the five new BAL-related maps (bal_pre_accounts, bal_pre_storage, bal_pending_accounts, bal_pending_storage, bal_committed_changed). If finalize() is called without a prior takeAccessLog(), these maps retain stale data from previous transactions. Inner maps within bal_pre_storage, bal_pending_storage, and bal_committed_changed also leak since their heap allocations are never freed.
Reviewed by Cursor Bugbot for commit a3f8e92. Configure here.


Summary
WitnessDatabase(DB layer) intoJournalInner(always-on, no@hasDeclguards)bal_untracked,untrackAddress, andforceTrackAddress— no longer needed since phantoms are prevented at the opcode levelWitnessDatabasebecomes a pure 4-method DB backend (basic,codeByHash,storage,blockHash)Depends on
#20
Test plan
zig build testpasses🤖 Generated with Claude Code
Note
Medium Risk
Touches core execution paths (journal commit/discard and opcode gas charging) and changes EIP-7928 access tracking semantics; regressions could affect gas/OOG behavior or BAL validation across transactions.
Overview
Moves EIP-7928 Block Access List (BAL) tracking into
JournalInner, adding persistent pre-block account/storage snapshots plus per-tx staging, cross-txcommitted_changedslot tracking, and atakeAccessLog()API to drain the block log.Updates journal lifecycle to flush/clear BAL state on
commitTx()/discardTx()and records pre-block account/slot values on firstloadAccount*/sloadaccess, removing DB-layer/frame-based tracking hooks.Eliminates phantom BAL entries by restructuring opcode gas checks to occur before any account/code DB load (notably
CALL,CREATE,EXTCODECOPY,SELFDESTRUCT) and simplifying warm/cold handling to rely onwarm_addressesacross tx boundaries.Reviewed by Cursor Bugbot for commit a3f8e92. Bugbot is set up for automated code reviews on this repo. Configure here.