Skip to content

Commit f0c8828

Browse files
fix: eliminate phantom BAL entries at opcode level
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>
1 parent f516930 commit f0c8828

File tree

4 files changed

+63
-131
lines changed

4 files changed

+63
-131
lines changed

src/context/journal.zig

Lines changed: 4 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -442,8 +442,6 @@ pub const JournalInner = struct {
442442
bal_pending_storage: std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, primitives.StorageValue)),
443443
// Slots committed to a non-pre-block value at any tx boundary.
444444
bal_committed_changed: std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, void)),
445-
// Addresses loaded only for gas calculation that went OOG (excluded from BAL).
446-
bal_untracked: std.AutoHashMap(primitives.Address, void),
447445

448446
pub fn new() JournalInner {
449447
return .{
@@ -460,7 +458,6 @@ pub const JournalInner = struct {
460458
.bal_pending_accounts = std.AutoHashMap(primitives.Address, AccountPreState).init(alloc_mod.get()),
461459
.bal_pending_storage = std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, primitives.StorageValue)).init(alloc_mod.get()),
462460
.bal_committed_changed = std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, void)).init(alloc_mod.get()),
463-
.bal_untracked = std.AutoHashMap(primitives.Address, void).init(alloc_mod.get()),
464461
};
465462
}
466463

@@ -483,7 +480,6 @@ pub const JournalInner = struct {
483480
var cc_it = self.bal_committed_changed.valueIterator();
484481
while (cc_it.next()) |m| m.deinit();
485482
self.bal_committed_changed.deinit();
486-
self.bal_untracked.deinit();
487483
}
488484

489485
// ── EIP-7928 BAL tracking helpers ─────────────────────────────────────────
@@ -516,25 +512,10 @@ pub const JournalInner = struct {
516512
gop.value_ptr.put(key, value) catch {};
517513
}
518514

519-
/// Mark an address as an OOG phantom — it was loaded for gas calc but the
520-
/// operation failed before the address was truly accessed. Removed from pending.
521-
pub fn untrackAddress(self: *JournalInner, address: primitives.Address) void {
522-
_ = self.bal_pending_accounts.remove(address);
523-
self.bal_untracked.put(address, {}) catch {};
524-
}
525-
526-
/// Force-add an address to the current-tx pending set (for EIP-7702 delegation targets
527-
/// that execute but are never loaded via basic()).
528-
pub fn forceTrackAddress(self: *JournalInner, address: primitives.Address) void {
529-
if (self.bal_pre_accounts.contains(address) or self.bal_pending_accounts.contains(address)) return;
530-
self.bal_pending_accounts.put(address, .{}) catch {};
531-
}
532-
533-
/// Returns true if the address is legitimately tracked (not an OOG phantom).
515+
/// Returns true if the address has been accessed (appears in the BAL).
516+
/// Phantom accesses are prevented at the opcode level, so all tracked addresses are legitimate.
534517
pub fn isTrackedAddress(self: *const JournalInner, address: primitives.Address) bool {
535-
if (self.bal_pre_accounts.contains(address)) return true;
536-
if (self.bal_untracked.contains(address)) return false;
537-
return self.bal_pending_accounts.contains(address);
518+
return self.bal_pre_accounts.contains(address) or self.bal_pending_accounts.contains(address);
538519
}
539520

540521
/// Drain the accumulated Block Access Log. Flushes any remaining pending state
@@ -634,7 +615,6 @@ pub const JournalInner = struct {
634615
e.value_ptr.deinit();
635616
}
636617
self.bal_pending_storage.clearRetainingCapacity();
637-
self.bal_untracked.clearRetainingCapacity();
638618
}
639619

640620
// Per EIP-2200/EIP-3529: after committing a transaction, the "original value"
@@ -685,7 +665,6 @@ pub const JournalInner = struct {
685665
var ps_it = self.bal_pending_storage.valueIterator();
686666
while (ps_it.next()) |m| m.deinit();
687667
self.bal_pending_storage.clearRetainingCapacity();
688-
self.bal_untracked.clearRetainingCapacity();
689668
}
690669

691670
/// Take the [`EvmState`] and clears the journal by resetting it to initial state.
@@ -1623,17 +1602,7 @@ pub fn Journal(comptime DB: type) type {
16231602
return self.inner.isStorageCold(address, key);
16241603
}
16251604

1626-
/// Mark an address as an OOG phantom — loaded for gas calc but operation went OOG.
1627-
pub fn untrackAddress(self: *@This(), address: primitives.Address) void {
1628-
self.inner.untrackAddress(address);
1629-
}
1630-
1631-
/// Force-add an address to the current-tx access log (EIP-7702 delegation targets).
1632-
pub fn forceTrackAddress(self: *@This(), address: primitives.Address) void {
1633-
self.inner.forceTrackAddress(address);
1634-
}
1635-
1636-
/// Returns true if the address is legitimately tracked (not an OOG phantom).
1605+
/// Returns true if the address has been accessed (appears in the BAL).
16371606
pub fn isTrackedAddress(self: *const @This(), address: primitives.Address) bool {
16381607
return self.inner.isTrackedAddress(address);
16391608
}

src/interpreter/host.zig

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ pub const JournalVTable = struct {
114114
// Simple entries — operate on the type-erased journal pointer directly.
115115
isAddressCold: *const fn (*anyopaque, primitives.Address) bool,
116116
isStorageCold: *const fn (*anyopaque, primitives.Address, primitives.StorageKey) bool,
117-
untrackAddress: *const fn (*anyopaque, primitives.Address) void,
118-
forceTrackAddress: *const fn (*anyopaque, primitives.Address) void,
119117
isAddressLoaded: *const fn (*anyopaque, primitives.Address) bool,
120118
accountInfo: *const fn (*anyopaque, primitives.Address) anyerror!context_mod.AccountInfoLoad,
121119
loadAccountWithCode: *const fn (*anyopaque, primitives.Address) anyerror!context_mod.StateLoad(*const state_mod.Account),
@@ -139,8 +137,6 @@ pub const JournalVTable = struct {
139137
const vtable: JournalVTable = .{
140138
.isAddressCold = isAddressColdFn,
141139
.isStorageCold = isStorageColdFn,
142-
.untrackAddress = untrackAddressFn,
143-
.forceTrackAddress = forceTrackAddressFn,
144140
.isAddressLoaded = isAddressLoadedFn,
145141
.accountInfo = accountInfoFn,
146142
.loadAccountWithCode = loadAccountWithCodeFn,
@@ -167,12 +163,6 @@ pub const JournalVTable = struct {
167163
fn isStorageColdFn(ptr: *anyopaque, addr: primitives.Address, key: primitives.StorageKey) bool {
168164
return j(ptr).isStorageCold(addr, key);
169165
}
170-
fn untrackAddressFn(ptr: *anyopaque, addr: primitives.Address) void {
171-
j(ptr).untrackAddress(addr);
172-
}
173-
fn forceTrackAddressFn(ptr: *anyopaque, addr: primitives.Address) void {
174-
j(ptr).forceTrackAddress(addr);
175-
}
176166
fn isAddressLoadedFn(ptr: *anyopaque, addr: primitives.Address) bool {
177167
return j(ptr).isAddressLoaded(addr);
178168
}
@@ -345,16 +335,6 @@ pub const Host = struct {
345335
return self.js_vtable.isStorageCold(self.js, addr, key);
346336
}
347337

348-
/// Un-record a pending address access in the database fallback.
349-
pub fn untrackAddress(self: *Host, addr: primitives.Address) void {
350-
self.js_vtable.untrackAddress(self.js, addr);
351-
}
352-
353-
/// Force-add an address to the current-tx access log in the database fallback.
354-
pub fn forceTrackAddress(self: *Host, addr: primitives.Address) void {
355-
self.js_vtable.forceTrackAddress(self.js, addr);
356-
}
357-
358338
/// Check whether an address is already in the EVM state cache.
359339
pub fn isAddressLoaded(self: *const Host, addr: primitives.Address) bool {
360340
return self.js_vtable.isAddressLoaded(@constCast(self.js), addr);
@@ -780,28 +760,22 @@ fn setupCreateCore(
780760
break :blk create2Address(caller, salt, init_hash);
781761
} else createAddress(caller, caller_nonce);
782762

783-
_ = js.loadAccount(new_addr) catch return .{ .failed = CreateResult.preExecFailure(gas_limit) };
784-
const new_addr_was_nonexistent = if (js.inner.evm_state.get(new_addr)) |na|
785-
na.status.loaded_as_not_existing
786-
else
787-
true;
788-
789-
// Balance check.
790-
if (value > 0) {
791-
const caller_acct = js.inner.evm_state.getPtr(caller) orelse {
792-
if (is_opcode_create and primitives.isEnabledIn(spec_id, .amsterdam))
793-
js.checkpointRevert(pre_bump_checkpoint);
794-
if (new_addr_was_nonexistent) js.untrackAddress(new_addr);
763+
// Balance check BEFORE loading new_addr to avoid phantom BAL entries.
764+
// Pre-Amsterdam already checked balance before the nonce bump (above); this handles
765+
// Amsterdam (checked after nonce bump) without needing an untrackAddress on failure.
766+
if (primitives.isEnabledIn(spec_id, .amsterdam) and value > 0) {
767+
const ca = js.inner.evm_state.getPtr(caller) orelse {
768+
if (is_opcode_create) js.checkpointRevert(pre_bump_checkpoint);
795769
return .{ .failed = CreateResult.preExecFailure(gas_limit) };
796770
};
797-
if (caller_acct.info.balance < value) {
798-
if (is_opcode_create and primitives.isEnabledIn(spec_id, .amsterdam))
799-
js.checkpointRevert(pre_bump_checkpoint);
800-
if (new_addr_was_nonexistent) js.untrackAddress(new_addr);
771+
if (ca.info.balance < value) {
772+
if (is_opcode_create) js.checkpointRevert(pre_bump_checkpoint);
801773
return .{ .failed = CreateResult.preExecFailure(gas_limit) };
802774
}
803775
}
804776

777+
_ = js.loadAccount(new_addr) catch return .{ .failed = CreateResult.preExecFailure(gas_limit) };
778+
805779
// Address collision: storage already exists at the target address.
806780
if (js.inner.evm_state.get(new_addr)) |acct| {
807781
var slot_it = acct.storage.valueIterator();

src/interpreter/opcodes/call.zig

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -132,13 +132,15 @@ fn callImpl(
132132
}
133133
}
134134

135-
// Pre-check: if Berlin+, ensure there's enough gas for the access cost before loading
136-
// the callee. This avoids a DB read when the CALL itself runs OOG before the callee
137-
// is accessed, which would incorrectly add the callee to the EIP-7928 BAL.
138135
const pre_is_cold = h.isAddressCold(target_addr);
136+
// transfers_value only depends on the stack value — no account load needed.
137+
const transfers_value = has_value and value > 0;
138+
// Worst-case pre-check (assume account non-existent) before any DB load.
139+
// getCallGasCost(..., false) >= exact cost, so if this passes the account can never
140+
// become a phantom BAL entry (the exact-cost check below can never OOG).
139141
if (primitives.isEnabledIn(spec, .berlin)) {
140-
const access_cost: u64 = if (pre_is_cold) gas_costs.COLD_ACCOUNT_ACCESS else gas_costs.WARM_ACCOUNT_ACCESS;
141-
if (ctx.interpreter.gas.remaining < access_cost) {
142+
const worst_case = gas_costs.getCallGasCost(spec, pre_is_cold, transfers_value, false);
143+
if (ctx.interpreter.gas.remaining < worst_case) {
142144
ctx.interpreter.halt(.out_of_gas);
143145
return;
144146
}
@@ -147,7 +149,6 @@ fn callImpl(
147149
// Load target account info to determine warm/cold access and whether the account exists.
148150
const acct_info = h.accountInfo(target_addr);
149151
const is_cold = if (acct_info) |info| info.is_cold else pre_is_cold;
150-
const transfers_value = has_value and value > 0;
151152
// G_NEWACCOUNT applies to the ETH *recipient*, not the code source.
152153
// For CALLCODE/DELEGATECALL, ETH goes to self (always exists). Otherwise ETH goes to target_addr.
153154
const account_exists = switch (scheme) {
@@ -175,14 +176,8 @@ fn callImpl(
175176
const base_cost = call_cost_no_delegation + delegation_gas;
176177

177178
// Determine forwarded gas (EIP-150 introduces 63/64 rule; pre-EIP-150 uses all remaining).
179+
// worst_case >= call_cost_no_delegation, so the pre-check above guarantees remaining >= it.
178180
const remaining = ctx.interpreter.gas.remaining;
179-
if (remaining < call_cost_no_delegation) {
180-
// OOG before the target was "accessed" in EIP-7928 terms (can't pay transfer/new-account).
181-
// Per EELS: target is not yet in accessed_addresses at this point → untrack it.
182-
h.untrackAddress(target_addr);
183-
ctx.interpreter.halt(.out_of_gas);
184-
return;
185-
}
186181
if (remaining < base_cost) {
187182
// OOG after target access but before delegation (oog_after_target_access /
188183
// oog_success_minus_1). Per EELS second check_gas: target IS in BAL, delegation NOT loaded.

src/interpreter/opcodes/host_ops.zig

Lines changed: 41 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,10 @@ pub fn opExtcodecopy(ctx: *InstructionContext) void {
153153

154154
const addr = host_module.u256ToAddress(addr_val);
155155

156-
// Post-Berlin: charge dynamic warm/cold cost BEFORE loading the code.
156+
// Charge all gas (warm/cold + copy + memory expansion) BEFORE loading the code.
157+
// This eliminates phantom BAL entries: the account is only loaded if gas succeeds.
158+
159+
// Post-Berlin: charge dynamic warm/cold cost.
157160
if (primitives.isEnabledIn(ctx.interpreter.runtime_flags.spec_id, .berlin)) {
158161
const dyn_gas: u64 = if (h.isAddressCold(addr)) gas_costs.COLD_ACCOUNT_ACCESS else gas_costs.WARM_ACCOUNT_ACCESS;
159162
if (!ctx.interpreter.gas.spend(dyn_gas)) {
@@ -162,61 +165,44 @@ pub fn opExtcodecopy(ctx: *InstructionContext) void {
162165
}
163166
}
164167

165-
const info = h.codeInfo(addr) orelse {
166-
// Address doesn't exist; still need to expand memory and pay copy cost
167-
if (size == 0) return;
168+
// Charge copy cost and expand memory (gas inputs depend only on stack values, not code content).
169+
const mem_off_u: usize = blk: {
170+
if (size == 0) break :blk 0;
168171
if (mem_off > std.math.maxInt(usize) or size > std.math.maxInt(usize)) {
169172
ctx.interpreter.halt(.memory_limit_oog);
170173
return;
171174
}
172-
const mem_off_u: usize = @intCast(mem_off);
173-
const size_u: usize = @intCast(size);
174-
const num_words = std.math.divCeil(usize, size_u, 32) catch unreachable;
175+
const mo: usize = @intCast(mem_off);
176+
const sz: usize = @intCast(size);
177+
const new_size = std.math.add(usize, mo, sz) catch {
178+
ctx.interpreter.halt(.memory_limit_oog);
179+
return;
180+
};
181+
// Dynamic: copy cost — use divCeil to avoid (size + 31) overflow when size = maxInt(usize)
182+
const num_words = std.math.divCeil(usize, sz, 32) catch unreachable;
175183
if (!ctx.interpreter.gas.spend(gas_costs.G_COPY * @as(u64, @intCast(num_words)))) {
176184
ctx.interpreter.halt(.out_of_gas);
177185
return;
178186
}
179-
const end_off = std.math.add(usize, mem_off_u, size_u) catch {
180-
ctx.interpreter.halt(.memory_limit_oog);
181-
return;
182-
};
183-
if (!expandMemory(ctx, end_off)) {
187+
if (!expandMemory(ctx, new_size)) {
184188
ctx.interpreter.halt(.out_of_gas);
185189
return;
186190
}
187-
@memset(ctx.interpreter.memory.buffer.items[mem_off_u..end_off], 0);
188-
return;
191+
break :blk mo;
189192
};
190193

191-
if (size == 0) return;
192-
193-
if (mem_off > std.math.maxInt(usize) or size > std.math.maxInt(usize)) {
194-
h.untrackAddress(addr);
195-
ctx.interpreter.halt(.memory_limit_oog);
196-
return;
197-
}
198-
199-
const mem_off_u: usize = @intCast(mem_off);
200-
const size_u: usize = @intCast(size);
201-
const new_size = std.math.add(usize, mem_off_u, size_u) catch {
202-
h.untrackAddress(addr);
203-
ctx.interpreter.halt(.memory_limit_oog);
194+
const info = h.codeInfo(addr) orelse {
195+
// Address doesn't exist: gas and memory already handled above.
196+
if (size == 0) return;
197+
const size_u: usize = @intCast(size);
198+
@memset(ctx.interpreter.memory.buffer.items[mem_off_u .. mem_off_u + size_u], 0);
204199
return;
205200
};
206201

207-
// Dynamic: copy cost — use divCeil to avoid (size + 31) overflow when size = maxInt(usize)
208-
const num_words = std.math.divCeil(usize, size_u, 32) catch unreachable;
209-
if (!ctx.interpreter.gas.spend(gas_costs.G_COPY * @as(u64, @intCast(num_words)))) {
210-
h.untrackAddress(addr);
211-
ctx.interpreter.halt(.out_of_gas);
212-
return;
213-
}
202+
if (size == 0) return;
214203

215-
if (!expandMemory(ctx, new_size)) {
216-
h.untrackAddress(addr);
217-
ctx.interpreter.halt(.out_of_gas);
218-
return;
219-
}
204+
const size_u: usize = @intCast(size);
205+
const new_size = mem_off_u + size_u; // valid: overflow already checked above
220206

221207
const code = info.bytecode.bytecode();
222208
const dest = ctx.interpreter.memory.buffer.items[mem_off_u..new_size];
@@ -605,6 +591,20 @@ pub fn opSelfdestruct(ctx: *InstructionContext) void {
605591
const self_addr = ctx.interpreter.input.target;
606592
const spec = ctx.interpreter.runtime_flags.spec_id;
607593

594+
// Worst-case pre-check before loading the target account.
595+
// Assumes cold access (if warm, actual cost is lower) and new account with value (worst case
596+
// for G_NEWACCOUNT). If this passes, the exact dyn_gas is <= max_dyn_gas so gas.spend() below
597+
// always succeeds and the target is never a phantom BAL entry.
598+
const pre_is_cold = h.isAddressCold(target);
599+
var max_dyn_gas: u64 = if (primitives.isEnabledIn(spec, .berlin) and pre_is_cold) gas_costs.COLD_ACCOUNT_ACCESS else 0;
600+
if (primitives.isEnabledIn(spec, .tangerine) and !primitives.isEnabledIn(spec, .amsterdam)) {
601+
max_dyn_gas += 25000; // worst-case G_NEWACCOUNT (Amsterdam replaces with state gas)
602+
}
603+
if (ctx.interpreter.gas.remaining < max_dyn_gas) {
604+
ctx.interpreter.halt(.out_of_gas);
605+
return;
606+
}
607+
608608
const result = h.selfdestruct(self_addr, target) orelse {
609609
ctx.interpreter.halt(.invalid_opcode);
610610
return;
@@ -631,14 +631,8 @@ pub fn opSelfdestruct(ctx: *InstructionContext) void {
631631
dyn_gas += 25000;
632632
}
633633

634-
// regular gas before state gas.
635-
if (!ctx.interpreter.gas.spend(dyn_gas)) {
636-
// Untrack the beneficiary: it was loaded for gas calculation but SELFDESTRUCT
637-
// never completed (OOG). EIP-7928 BAL should not include it.
638-
h.untrackAddress(target);
639-
ctx.interpreter.halt(.out_of_gas);
640-
return;
641-
}
634+
// dyn_gas <= max_dyn_gas (pre-check passed) — spend always succeeds.
635+
_ = ctx.interpreter.gas.spend(dyn_gas);
642636

643637
// EIP-8037 (Amsterdam+): charge state gas for new account via SELFDESTRUCT.
644638
// Draws from reservoir first, spills to gas_left if needed.

0 commit comments

Comments
 (0)