fix: eliminate phantom BAL entries at opcode level

Gabriel-Trintinalia · claude · Gabriel-Trintinalia · commit 1224e3576802 · 2026-04-03T17:58:31.000+02:00
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 &lt;noreply@anthropic.com&gt;
diff --git a/src/context/journal.zig b/src/context/journal.zig
@@ -442,8 +442,6 @@ pub const JournalInner = struct {
     bal_pending_storage: std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, primitives.StorageValue)),
     // Slots committed to a non-pre-block value at any tx boundary.
     bal_committed_changed: std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, void)),
-    // Addresses loaded only for gas calculation that went OOG (excluded from BAL).
-    bal_untracked: std.AutoHashMap(primitives.Address, void),
 
     pub fn new() JournalInner {
         return .{
@@ -460,7 +458,6 @@ pub const JournalInner = struct {
             .bal_pending_accounts = std.AutoHashMap(primitives.Address, AccountPreState).init(alloc_mod.get()),
             .bal_pending_storage = std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, primitives.StorageValue)).init(alloc_mod.get()),
             .bal_committed_changed = std.AutoHashMap(primitives.Address, std.AutoHashMap(primitives.StorageKey, void)).init(alloc_mod.get()),
-            .bal_untracked = std.AutoHashMap(primitives.Address, void).init(alloc_mod.get()),
         };
     }
 
@@ -483,7 +480,6 @@ pub const JournalInner = struct {
         var cc_it = self.bal_committed_changed.valueIterator();
         while (cc_it.next()) |m| m.deinit();
         self.bal_committed_changed.deinit();
-        self.bal_untracked.deinit();
     }
 
     // ── EIP-7928 BAL tracking helpers ─────────────────────────────────────────
@@ -516,25 +512,10 @@ pub const JournalInner = struct {
         gop.value_ptr.put(key, value) catch {};
     }
 
-    /// Mark an address as an OOG phantom — it was loaded for gas calc but the
-    /// operation failed before the address was truly accessed. Removed from pending.
-    pub fn untrackAddress(self: *JournalInner, address: primitives.Address) void {
-        _ = self.bal_pending_accounts.remove(address);
-        self.bal_untracked.put(address, {}) catch {};
-    }
-
-    /// Force-add an address to the current-tx pending set (for EIP-7702 delegation targets
-    /// that execute but are never loaded via basic()).
-    pub fn forceTrackAddress(self: *JournalInner, address: primitives.Address) void {
-        if (self.bal_pre_accounts.contains(address) or self.bal_pending_accounts.contains(address)) return;
-        self.bal_pending_accounts.put(address, .{}) catch {};
-    }
-
-    /// Returns true if the address is legitimately tracked (not an OOG phantom).
+    /// Returns true if the address has been accessed (appears in the BAL).
+    /// Phantom accesses are prevented at the opcode level, so all tracked addresses are legitimate.
     pub fn isTrackedAddress(self: *const JournalInner, address: primitives.Address) bool {
-        if (self.bal_pre_accounts.contains(address)) return true;
-        if (self.bal_untracked.contains(address)) return false;
-        return self.bal_pending_accounts.contains(address);
+        return self.bal_pre_accounts.contains(address) or self.bal_pending_accounts.contains(address);
     }
 
     /// Drain the accumulated Block Access Log. Flushes any remaining pending state
@@ -634,7 +615,6 @@ pub const JournalInner = struct {
                 e.value_ptr.deinit();
             }
             self.bal_pending_storage.clearRetainingCapacity();
-            self.bal_untracked.clearRetainingCapacity();
         }
 
         // Per EIP-2200/EIP-3529: after committing a transaction, the "original value"
@@ -685,7 +665,6 @@ pub const JournalInner = struct {
         var ps_it = self.bal_pending_storage.valueIterator();
         while (ps_it.next()) |m| m.deinit();
         self.bal_pending_storage.clearRetainingCapacity();
-        self.bal_untracked.clearRetainingCapacity();
     }
 
     /// Take the [`EvmState`] and clears the journal by resetting it to initial state.
@@ -1623,17 +1602,7 @@ pub fn Journal(comptime DB: type) type {
             return self.inner.isStorageCold(address, key);
         }
 
-        /// Mark an address as an OOG phantom — loaded for gas calc but operation went OOG.
-        pub fn untrackAddress(self: *@This(), address: primitives.Address) void {
-            self.inner.untrackAddress(address);
-        }
-
-        /// Force-add an address to the current-tx access log (EIP-7702 delegation targets).
-        pub fn forceTrackAddress(self: *@This(), address: primitives.Address) void {
-            self.inner.forceTrackAddress(address);
-        }
-
-        /// Returns true if the address is legitimately tracked (not an OOG phantom).
+        /// Returns true if the address has been accessed (appears in the BAL).
         pub fn isTrackedAddress(self: *const @This(), address: primitives.Address) bool {
             return self.inner.isTrackedAddress(address);
         }
diff --git a/src/interpreter/host.zig b/src/interpreter/host.zig
@@ -114,8 +114,6 @@ pub const JournalVTable = struct {
     // Simple entries — operate on the type-erased journal pointer directly.
     isAddressCold: *const fn (*anyopaque, primitives.Address) bool,
     isStorageCold: *const fn (*anyopaque, primitives.Address, primitives.StorageKey) bool,
-    untrackAddress: *const fn (*anyopaque, primitives.Address) void,
-    forceTrackAddress: *const fn (*anyopaque, primitives.Address) void,
     isAddressLoaded: *const fn (*anyopaque, primitives.Address) bool,
     accountInfo: *const fn (*anyopaque, primitives.Address) anyerror!context_mod.AccountInfoLoad,
     loadAccountWithCode: *const fn (*anyopaque, primitives.Address) anyerror!context_mod.StateLoad(*const state_mod.Account),
@@ -139,8 +137,6 @@ pub const JournalVTable = struct {
             const vtable: JournalVTable = .{
                 .isAddressCold = isAddressColdFn,
                 .isStorageCold = isStorageColdFn,
-                .untrackAddress = untrackAddressFn,
-                .forceTrackAddress = forceTrackAddressFn,
                 .isAddressLoaded = isAddressLoadedFn,
                 .accountInfo = accountInfoFn,
                 .loadAccountWithCode = loadAccountWithCodeFn,
@@ -167,12 +163,6 @@ pub const JournalVTable = struct {
             fn isStorageColdFn(ptr: *anyopaque, addr: primitives.Address, key: primitives.StorageKey) bool {
                 return j(ptr).isStorageCold(addr, key);
             }
-            fn untrackAddressFn(ptr: *anyopaque, addr: primitives.Address) void {
-                j(ptr).untrackAddress(addr);
-            }
-            fn forceTrackAddressFn(ptr: *anyopaque, addr: primitives.Address) void {
-                j(ptr).forceTrackAddress(addr);
-            }
             fn isAddressLoadedFn(ptr: *anyopaque, addr: primitives.Address) bool {
                 return j(ptr).isAddressLoaded(addr);
             }
@@ -345,16 +335,6 @@ pub const Host = struct {
         return self.js_vtable.isStorageCold(self.js, addr, key);
     }
 
-    /// Un-record a pending address access in the database fallback.
-    pub fn untrackAddress(self: *Host, addr: primitives.Address) void {
-        self.js_vtable.untrackAddress(self.js, addr);
-    }
-
-    /// Force-add an address to the current-tx access log in the database fallback.
-    pub fn forceTrackAddress(self: *Host, addr: primitives.Address) void {
-        self.js_vtable.forceTrackAddress(self.js, addr);
-    }
-
     /// Check whether an address is already in the EVM state cache.
     pub fn isAddressLoaded(self: *const Host, addr: primitives.Address) bool {
         return self.js_vtable.isAddressLoaded(@constCast(self.js), addr);
@@ -684,12 +664,10 @@ fn setupCallCore(js: anytype, host: *Host, inputs: CallInputs, frame_depth: usiz
     // 5. Value transfer
     if (inputs.value > 0 and inputs.scheme != .delegatecall) {
         const transfer_err = js.transfer(inputs.caller, inputs.target, inputs.value) catch {
-            js.revertFrame();
             js.checkpointRevert(checkpoint);
             return .{ .failed = CallResult.preExecFailure(inputs.gas_limit) };
         };
         if (transfer_err != null) {
-            js.revertFrame();
             js.checkpointRevert(checkpoint);
             return .{ .failed = CallResult.preExecFailure(inputs.gas_limit) };
         }
@@ -782,28 +760,22 @@ fn setupCreateCore(
         break :blk create2Address(caller, salt, init_hash);
     } else createAddress(caller, caller_nonce);
 
-    _ = js.loadAccount(new_addr) catch return .{ .failed = CreateResult.preExecFailure(gas_limit) };
-    const new_addr_was_nonexistent = if (js.inner.evm_state.get(new_addr)) |na|
-        na.status.loaded_as_not_existing
-    else
-        true;
-
-    // Balance check.
-    if (value > 0) {
-        const caller_acct = js.inner.evm_state.getPtr(caller) orelse {
-            if (is_opcode_create and primitives.isEnabledIn(spec_id, .amsterdam))
-                js.checkpointRevert(pre_bump_checkpoint);
-            if (new_addr_was_nonexistent) js.untrackAddress(new_addr);
+    // Balance check BEFORE loading new_addr to avoid phantom BAL entries.
+    // Pre-Amsterdam already checked balance before the nonce bump (above); this handles
+    // Amsterdam (checked after nonce bump) without needing an untrackAddress on failure.
+    if (primitives.isEnabledIn(spec_id, .amsterdam) and value > 0) {
+        const ca = js.inner.evm_state.getPtr(caller) orelse {
+            if (is_opcode_create) js.checkpointRevert(pre_bump_checkpoint);
             return .{ .failed = CreateResult.preExecFailure(gas_limit) };
         };
-        if (caller_acct.info.balance < value) {
-            if (is_opcode_create and primitives.isEnabledIn(spec_id, .amsterdam))
-                js.checkpointRevert(pre_bump_checkpoint);
-            if (new_addr_was_nonexistent) js.untrackAddress(new_addr);
+        if (ca.info.balance < value) {
+            if (is_opcode_create) js.checkpointRevert(pre_bump_checkpoint);
             return .{ .failed = CreateResult.preExecFailure(gas_limit) };
         }
     }
 
+    _ = js.loadAccount(new_addr) catch return .{ .failed = CreateResult.preExecFailure(gas_limit) };
+
     // Address collision: storage already exists at the target address.
     if (js.inner.evm_state.get(new_addr)) |acct| {
         var slot_it = acct.storage.valueIterator();
diff --git a/src/interpreter/opcodes/call.zig b/src/interpreter/opcodes/call.zig
@@ -132,13 +132,15 @@ fn callImpl(
         }
     }
 
-    // Pre-check: if Berlin+, ensure there's enough gas for the access cost before loading
-    // the callee. This avoids a DB read when the CALL itself runs OOG before the callee
-    // is accessed, which would incorrectly add the callee to the EIP-7928 BAL.
     const pre_is_cold = h.isAddressCold(target_addr);
+    // transfers_value only depends on the stack value — no account load needed.
+    const transfers_value = has_value and value > 0;
+    // Worst-case pre-check (assume account non-existent) before any DB load.
+    // getCallGasCost(..., false) >= exact cost, so if this passes the account can never
+    // become a phantom BAL entry (the exact-cost check below can never OOG).
     if (primitives.isEnabledIn(spec, .berlin)) {
-        const access_cost: u64 = if (pre_is_cold) gas_costs.COLD_ACCOUNT_ACCESS else gas_costs.WARM_ACCOUNT_ACCESS;
-        if (ctx.interpreter.gas.remaining < access_cost) {
+        const worst_case = gas_costs.getCallGasCost(spec, pre_is_cold, transfers_value, false);
+        if (ctx.interpreter.gas.remaining < worst_case) {
             ctx.interpreter.halt(.out_of_gas);
             return;
         }
@@ -147,7 +149,6 @@ fn callImpl(
     // Load target account info to determine warm/cold access and whether the account exists.
     const acct_info = h.accountInfo(target_addr);
     const is_cold = if (acct_info) |info| info.is_cold else pre_is_cold;
-    const transfers_value = has_value and value > 0;
     // G_NEWACCOUNT applies to the ETH *recipient*, not the code source.
     // For CALLCODE/DELEGATECALL, ETH goes to self (always exists). Otherwise ETH goes to target_addr.
     const account_exists = switch (scheme) {
@@ -175,14 +176,8 @@ fn callImpl(
     const base_cost = call_cost_no_delegation + delegation_gas;
 
     // Determine forwarded gas (EIP-150 introduces 63/64 rule; pre-EIP-150 uses all remaining).
+    // worst_case >= call_cost_no_delegation, so the pre-check above guarantees remaining >= it.
     const remaining = ctx.interpreter.gas.remaining;
-    if (remaining < call_cost_no_delegation) {
-        // OOG before the target was "accessed" in EIP-7928 terms (can't pay transfer/new-account).
-        // Per EELS: target is not yet in accessed_addresses at this point → untrack it.
-        h.untrackAddress(target_addr);
-        ctx.interpreter.halt(.out_of_gas);
-        return;
-    }
     if (remaining < base_cost) {
         // OOG after target access but before delegation (oog_after_target_access /
         // oog_success_minus_1). Per EELS second check_gas: target IS in BAL, delegation NOT loaded.
diff --git a/src/interpreter/opcodes/host_ops.zig b/src/interpreter/opcodes/host_ops.zig
@@ -153,7 +153,10 @@ pub fn opExtcodecopy(ctx: *InstructionContext) void {
 
     const addr = host_module.u256ToAddress(addr_val);
 
-    // Post-Berlin: charge dynamic warm/cold cost BEFORE loading the code.
+    // Charge all gas (warm/cold + copy + memory expansion) BEFORE loading the code.
+    // This eliminates phantom BAL entries: the account is only loaded if gas succeeds.
+
+    // Post-Berlin: charge dynamic warm/cold cost.
     if (primitives.isEnabledIn(ctx.interpreter.runtime_flags.spec_id, .berlin)) {
         const dyn_gas: u64 = if (h.isAddressCold(addr)) gas_costs.COLD_ACCOUNT_ACCESS else gas_costs.WARM_ACCOUNT_ACCESS;
         if (!ctx.interpreter.gas.spend(dyn_gas)) {
@@ -162,61 +165,44 @@ pub fn opExtcodecopy(ctx: *InstructionContext) void {
         }
     }
 
-    const info = h.codeInfo(addr) orelse {
-        // Address doesn't exist; still need to expand memory and pay copy cost
-        if (size == 0) return;
+    // Charge copy cost and expand memory (gas inputs depend only on stack values, not code content).
+    const mem_off_u: usize = blk: {
+        if (size == 0) break :blk 0;
         if (mem_off > std.math.maxInt(usize) or size > std.math.maxInt(usize)) {
             ctx.interpreter.halt(.memory_limit_oog);
             return;
         }
-        const mem_off_u: usize = @intCast(mem_off);
-        const size_u: usize = @intCast(size);
-        const num_words = std.math.divCeil(usize, size_u, 32) catch unreachable;
+        const mo: usize = @intCast(mem_off);
+        const sz: usize = @intCast(size);
+        const new_size = std.math.add(usize, mo, sz) catch {
+            ctx.interpreter.halt(.memory_limit_oog);
+            return;
+        };
+        // Dynamic: copy cost — use divCeil to avoid (size + 31) overflow when size = maxInt(usize)
+        const num_words = std.math.divCeil(usize, sz, 32) catch unreachable;
         if (!ctx.interpreter.gas.spend(gas_costs.G_COPY * @as(u64, @intCast(num_words)))) {
             ctx.interpreter.halt(.out_of_gas);
             return;
         }
-        const end_off = std.math.add(usize, mem_off_u, size_u) catch {
-            ctx.interpreter.halt(.memory_limit_oog);
-            return;
-        };
-        if (!expandMemory(ctx, end_off)) {
+        if (!expandMemory(ctx, new_size)) {
             ctx.interpreter.halt(.out_of_gas);
             return;
         }
-        @memset(ctx.interpreter.memory.buffer.items[mem_off_u..end_off], 0);
-        return;
+        break :blk mo;
     };
 
-    if (size == 0) return;
-
-    if (mem_off > std.math.maxInt(usize) or size > std.math.maxInt(usize)) {
-        h.untrackAddress(addr);
-        ctx.interpreter.halt(.memory_limit_oog);
-        return;
-    }
-
-    const mem_off_u: usize = @intCast(mem_off);
-    const size_u: usize = @intCast(size);
-    const new_size = std.math.add(usize, mem_off_u, size_u) catch {
-        h.untrackAddress(addr);
-        ctx.interpreter.halt(.memory_limit_oog);
+    const info = h.codeInfo(addr) orelse {
+        // Address doesn't exist: gas and memory already handled above.
+        if (size == 0) return;
+        const size_u: usize = @intCast(size);
+        @memset(ctx.interpreter.memory.buffer.items[mem_off_u .. mem_off_u + size_u], 0);
         return;
     };
 
-    // Dynamic: copy cost — use divCeil to avoid (size + 31) overflow when size = maxInt(usize)
-    const num_words = std.math.divCeil(usize, size_u, 32) catch unreachable;
-    if (!ctx.interpreter.gas.spend(gas_costs.G_COPY * @as(u64, @intCast(num_words)))) {
-        h.untrackAddress(addr);
-        ctx.interpreter.halt(.out_of_gas);
-        return;
-    }
+    if (size == 0) return;
 
-    if (!expandMemory(ctx, new_size)) {
-        h.untrackAddress(addr);
-        ctx.interpreter.halt(.out_of_gas);
-        return;
-    }
+    const size_u: usize = @intCast(size);
+    const new_size = mem_off_u + size_u; // valid: overflow already checked above
 
     const code = info.bytecode.bytecode();
     const dest = ctx.interpreter.memory.buffer.items[mem_off_u..new_size];
@@ -605,6 +591,20 @@ pub fn opSelfdestruct(ctx: *InstructionContext) void {
     const self_addr = ctx.interpreter.input.target;
     const spec = ctx.interpreter.runtime_flags.spec_id;
 
+    // Worst-case pre-check before loading the target account.
+    // Assumes cold access (if warm, actual cost is lower) and new account with value (worst case
+    // for G_NEWACCOUNT). If this passes, the exact dyn_gas is <= max_dyn_gas so gas.spend() below
+    // always succeeds and the target is never a phantom BAL entry.
+    const pre_is_cold = h.isAddressCold(target);
+    var max_dyn_gas: u64 = if (primitives.isEnabledIn(spec, .berlin) and pre_is_cold) gas_costs.COLD_ACCOUNT_ACCESS else 0;
+    if (primitives.isEnabledIn(spec, .tangerine) and !primitives.isEnabledIn(spec, .amsterdam)) {
+        max_dyn_gas += 25000; // worst-case G_NEWACCOUNT (Amsterdam replaces with state gas)
+    }
+    if (ctx.interpreter.gas.remaining < max_dyn_gas) {
+        ctx.interpreter.halt(.out_of_gas);
+        return;
+    }
+
     const result = h.selfdestruct(self_addr, target) orelse {
         ctx.interpreter.halt(.invalid_opcode);
         return;
@@ -631,14 +631,8 @@ pub fn opSelfdestruct(ctx: *InstructionContext) void {
         dyn_gas += 25000;
     }
 
-    // regular gas before state gas.
-    if (!ctx.interpreter.gas.spend(dyn_gas)) {
-        // Untrack the beneficiary: it was loaded for gas calculation but SELFDESTRUCT
-        // never completed (OOG). EIP-7928 BAL should not include it.
-        h.untrackAddress(target);
-        ctx.interpreter.halt(.out_of_gas);
-        return;
-    }
+    // dyn_gas <= max_dyn_gas (pre-check passed) — spend always succeeds.
+    _ = ctx.interpreter.gas.spend(dyn_gas);
 
     // EIP-8037 (Amsterdam+): charge state gas for new account via SELFDESTRUCT.
     // Draws from reservoir first, spills to gas_left if needed.
