fix(stateless): audit of swallowed database errors that fabricate execution results

build.zig

@@ -535,6 +535,10 @@ pub fn build(b: *std.Build) void {
         .{ .m = mods.precompile, .name = "precompile" },
         .{ .m = mods.interpreter, .name = "interpreter" },
         .{ .m = mods.handler, .name = "handler" },
+        // `context` owns journal.zig. Tests only run for the module passed to
+        // addTest, not for its imported modules, so without this entry nothing in
+        // src/evm/context/ is covered by `zig build test`.
+        .{ .m = mods.context, .name = "context" },
         .{ .m = mods.mpt, .name = "mpt" },
         .{ .m = mods.rlp_decode, .name = "rlp_decode" },
         .{ .m = mods.executor, .name = "executor" },

src/evm/context/journal.zig

@@ -1652,8 +1652,21 @@ pub fn Journal(comptime DB: type) type {
 
         /// Returns true if the address has any non-zero storage in the DB.
         /// Used by CREATE collision check; returns false for DB types without this method.
-        pub fn hasNonZeroStorageForAddress(self: *const @This(), addr: primitives.Address) bool {
-            if (comptime @hasDecl(DB, "hasNonZeroStorageForAddress")) return self.getDb().hasNonZeroStorageForAddress(addr);
+        ///
+        /// A DB that can fail (a stateless witness) is not allowed to have its
+        /// failure silently become "no storage here": that would let a CREATE
+        /// succeed where the reference rejects it. The error is recorded so the
+        /// block is rejected after execution, and `true` is returned meanwhile so
+        /// the in-flight CREATE fails closed rather than proceeding on a guess.
+        /// Propagates rather than reporting "no storage here" on a database error:
+        /// this feeds the CREATE collision check, so a swallowed failure would let
+        /// a CREATE succeed where the reference rejects it. The caller (which owns
+        /// ctx_error) is responsible for marking the block invalid. Databases that
+        /// cannot fail (InMemoryDB returns a plain bool) are unaffected.
+        pub fn hasNonZeroStorageForAddress(self: *const @This(), addr: primitives.Address) !bool {
+            if (comptime @hasDecl(DB, "hasNonZeroStorageForAddress")) {
+                return self.getDb().hasNonZeroStorageForAddress(addr);
+            }
             return false;
         }
 
@@ -1671,3 +1684,48 @@ pub fn Journal(comptime DB: type) type {
         }
     };
 }
+
+// ─── Tests: swallowed-database-error channel ──────────────────────────────────
+
+// A stub DB whose hasNonZeroStorageForAddress always fails, standing in for a
+// stateless witness that cannot prove the account. Only the members Journal
+// touches on this path are provided.
+const FailingStorageDb = struct {
+    pub fn hasNonZeroStorageForAddress(_: *const @This(), _: primitives.Address) !bool {
+        return error.InvalidWitness;
+    }
+};
+
+// The CREATE collision check must not read a DB failure as "no storage here":
+// that would let a CREATE succeed at an address the reference rejects. The journal
+// propagates instead, leaving the caller (which owns ctx_error) to mark the block
+// invalid.
+test "a failing hasNonZeroStorageForAddress propagates instead of answering false" {
+    var j = Journal(FailingStorageDb).new(.{});
+    defer j.deinit();
+
+    try std.testing.expectError(error.InvalidWitness, j.hasNonZeroStorageForAddress(@splat(0x11)));
+}
+
+// An infallible DB (InMemoryDB returns a plain bool) must keep working unchanged —
+// the @hasDecl/duck-typed path must not require an error union.
+test "an infallible DB still answers hasNonZeroStorageForAddress directly" {
+    const PlainDb = struct {
+        pub fn hasNonZeroStorageForAddress(_: *const @This(), _: primitives.Address) bool {
+            return true;
+        }
+    };
+    var j = Journal(PlainDb).new(.{});
+    defer j.deinit();
+
+    try std.testing.expect(try j.hasNonZeroStorageForAddress(@splat(0x22)));
+}
+
+// A DB without the method at all falls back to false, as before.
+test "a DB without hasNonZeroStorageForAddress reports false" {
+    const NoDeclDb = struct {};
+    var j = Journal(NoDeclDb).new(.{});
+    defer j.deinit();
+
+    try std.testing.expect(!(try j.hasNonZeroStorageForAddress(@splat(0x33))));
+}

src/evm/context/main.zig

@@ -117,3 +117,11 @@ pub const testing = struct {
         std.debug.print("Context tests passed.\n", .{});
     }
 };
+
+// `zig build test` only collects tests from a module's root file, so tests living
+// in the other files of this module are invisible unless referenced here. Without
+// this block, everything in journal.zig — including the swallowed-database-error
+// channel — is silently untested even when `context` is in build.zig's test list.
+test {
+    _ = @import("journal.zig");
+}

src/evm/interpreter/host.zig

@@ -360,7 +360,10 @@ pub const Host = struct {
 
     /// Load account info. Returns null on database error.
     pub fn accountInfo(self: *Host, addr: primitives.Address) ?struct { balance: primitives.U256, is_cold: bool, is_empty: bool } {
-        const load = self.js_vtable.accountInfo(self.js, addr) catch return null;
+        const load = self.js_vtable.accountInfo(self.js, addr) catch {
+            self.ctx_error.* = context_mod.ContextError.database_error;
+            return null;
+        };
         return .{
             .balance = load.info.balance,
             .is_cold = load.is_cold,
@@ -425,12 +428,18 @@ pub const Host = struct {
     }
 
     pub fn sload(self: *Host, addr: primitives.Address, key: primitives.U256) ?struct { value: primitives.U256, is_cold: bool } {
-        const load = self.js_vtable.sload(self.js, addr, key) catch return null;
+        const load = self.js_vtable.sload(self.js, addr, key) catch {
+            self.ctx_error.* = context_mod.ContextError.database_error;
+            return null;
+        };
         return .{ .value = load.data, .is_cold = load.is_cold };
     }
 
     pub fn sstore(self: *Host, addr: primitives.Address, key: primitives.U256, val: primitives.U256) ?struct { original: primitives.U256, current: primitives.U256, new: primitives.U256, is_cold: bool } {
-        const result = self.js_vtable.sstore(self.js, addr, key, val) catch return null;
+        const result = self.js_vtable.sstore(self.js, addr, key, val) catch {
+            self.ctx_error.* = context_mod.ContextError.database_error;
+            return null;
+        };
         return .{
             .original = result.data.original_value,
             .current = result.data.present_value,
@@ -452,7 +461,10 @@ pub const Host = struct {
     }
 
     pub fn selfdestruct(self: *Host, addr: primitives.Address, target: primitives.Address) ?SelfDestructLoadResult {
-        const result = self.js_vtable.selfdestruct(self.js, addr, target) catch return null;
+        const result = self.js_vtable.selfdestruct(self.js, addr, target) catch {
+            self.ctx_error.* = context_mod.ContextError.database_error;
+            return null;
+        };
         return .{
             .had_value = result.data.had_value,
             .target_exists = result.data.target_exists,
@@ -847,7 +859,10 @@ fn recordCreateTargetCore(
         accel.keccak256(init_code, &init_hash);
         break :blk create2Address(caller, salt, init_hash);
     } else createAddress(caller, nonce);
-    const load = js.loadAccount(new_addr) catch return null;
+    const load = js.loadAccount(new_addr) catch {
+        host.ctx_error.* = context_mod.ContextError.database_error;
+        return null;
+    };
     // is_account_alive: reference generic_create charges NEW_ACCOUNT only when the target
     // leaf does not already exist (balance/nonce/code present).
     const info = load.data.info;
@@ -924,7 +939,14 @@ fn setupCreateCore(
         }
     }
 
-    _ = js.loadAccount(new_addr) catch return .{ .failed = CreateResult.preExecFailure(gas_limit) };
+    // Mark the block invalid before failing: with a stateless witness this error
+    // means the CREATE target could not be resolved, so "create failed" would be a
+    // fabricated outcome. Note js.loadAccount is a Journal call and so bypasses the
+    // Host accessors that already set ctx_error on a database error.
+    _ = js.loadAccount(new_addr) catch {
+        host.ctx_error.* = context_mod.ContextError.database_error;
+        return .{ .failed = CreateResult.preExecFailure(gas_limit) };
+    };
 
     // EIP-8037 (Amsterdam+): was the target already alive (pre-funded) before creation?
     // Captured before createAccountCheckpoint transfers value / bumps nonce. A deployable
@@ -949,12 +971,24 @@ fn setupCreateCore(
     {
         const storage_wiped = if (js.inner.evm_state.get(new_addr)) |acct| acct.status.storage_wiped else false;
         if (!storage_wiped) {
-            if (js.hasNonZeroStorageForAddress(new_addr)) {
+            // A database error here means we cannot tell whether the target has
+            // storage. Treating that as "no storage" would let the CREATE proceed
+            // at an address the reference rejects, so mark the block invalid and
+            // fail the CREATE closed.
+            const has_storage = js.hasNonZeroStorageForAddress(new_addr) catch {
+                host.ctx_error.* = context_mod.ContextError.database_error;
+                return .{ .failed = CreateResult.failure() };
+            };
+            if (has_storage) {
                 return .{ .failed = CreateResult.failure() };
             }
         }
     }
 
+    // Safe to swallow: createAccountCheckpoint's error set is exactly TransferError
+    // (OutOfFunds, OverflowPayment, CreateCollision), all of which are legitimate
+    // CREATE failures rather than "outcome could not be determined". It cannot
+    // surface a database error, so there is nothing to record here.
     const checkpoint = js.createAccountCheckpoint(caller, new_addr, value, spec_id) catch {
         return .{ .failed = CreateResult.failure() };
     };
@@ -1172,3 +1206,171 @@ pub fn create2Address(sender: primitives.Address, salt: primitives.U256, init_co
     @memcpy(&addr, hash[12..32]);
     return addr;
 }
+
+// ─── Tests ────────────────────────────────────────────────────────────────────
+
+// A database that can resolve one address and fails for every other, standing in
+// for a stateless witness that is missing the CREATE target account.
+const MissingTargetDb = struct {
+    pub const CALLER: primitives.Address = @splat(0xC0);
+
+    pub fn basic(_: *@This(), address: primitives.Address) !?state_mod.AccountInfo {
+        if (std.mem.eql(u8, &address, &CALLER)) {
+            var info = state_mod.AccountInfo.default();
+            info.balance = 1_000_000;
+            return info;
+        }
+        return error.InvalidWitness;
+    }
+
+    pub fn codeByHash(_: *@This(), _: primitives.Hash) !bytecode_mod.Bytecode {
+        return bytecode_mod.Bytecode.newLegacy(&.{});
+    }
+
+    pub fn storage(_: *@This(), _: primitives.Address, _: primitives.StorageKey) !primitives.StorageValue {
+        return error.InvalidWitness;
+    }
+
+    pub fn blockHash(_: *@This(), _: u64) !primitives.Hash {
+        return error.InvalidWitness;
+    }
+};
+
+// setupCreateCore cannot return an error (it yields a plain CreateSetupResult), so
+// a database failure while loading the CREATE target can only become "create
+// failed". With a stateless witness that outcome is fabricated, so ctx_error must
+// be marked -- the block-level driver (stateless/executor/main.zig) turns a
+// non-ok ctx_error into InvalidWitness. Note js.loadAccount is a Journal call and
+// bypasses the Host accessors that already do this, which is why it needs its own
+// marking here.
+test "setupCreateCore marks ctx_error when the CREATE target cannot be loaded" {
+    var ctx = context_mod.Context(MissingTargetDb).new(.{}, primitives.SpecId.prague);
+    defer ctx.journaled_state.deinit();
+
+    var host = Host.init(MissingTargetDb, &ctx, null);
+
+    // The caller is read from evm_state, not the DB, so load it first: we want the
+    // CREATE to fail on the *target*, not on its own caller lookup.
+    _ = try ctx.journaled_state.loadAccount(MissingTargetDb.CALLER);
+
+    try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error);
+
+    const setup = setupCreateCore(
+        &ctx.journaled_state,
+        &host,
+        MissingTargetDb.CALLER,
+        0,
+        &[_]u8{0x00},
+        100_000,
+        false,
+        0,
+        false,
+        0,
+        true,
+    );
+
+    switch (setup) {
+        .failed => {},
+        else => return error.ExpectedCreateToFail,
+    }
+    // The block must be rejected rather than accepting the fabricated failure.
+    try std.testing.expectEqual(context_mod.ContextError.database_error, ctx.ctx_error);
+}
+
+// Reported by Cursor Bugbot on #99: making WitnessDatabase.storage() return
+// InvalidWitness was not sufficient on its own. Host.sload swallowed it with
+// `catch return null`, and opSload turns null into halt(.invalid_opcode) — a
+// consensus-visible EVM failure — so the transaction failed and the block was
+// still ACCEPTED. Same fabricated success, different mask than the zero it
+// replaced. These accessors now mark ctx_error like their siblings
+// (blockHash, codeInfo) already did.
+test "Host.sload marks ctx_error when the slot cannot be proven" {
+    var ctx = context_mod.Context(MissingTargetDb).new(.{}, primitives.SpecId.prague);
+    defer ctx.journaled_state.deinit();
+    var host = Host.init(MissingTargetDb, &ctx, null);
+
+    _ = try ctx.journaled_state.loadAccount(MissingTargetDb.CALLER);
+    try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error);
+
+    const result = host.sload(MissingTargetDb.CALLER, 1);
+
+    try std.testing.expect(result == null);
+    try std.testing.expectEqual(context_mod.ContextError.database_error, ctx.ctx_error);
+}
+
+test "Host.sstore marks ctx_error when the slot cannot be proven" {
+    var ctx = context_mod.Context(MissingTargetDb).new(.{}, primitives.SpecId.prague);
+    defer ctx.journaled_state.deinit();
+    var host = Host.init(MissingTargetDb, &ctx, null);
+
+    _ = try ctx.journaled_state.loadAccount(MissingTargetDb.CALLER);
+    try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error);
+
+    const result = host.sstore(MissingTargetDb.CALLER, 1, 42);
+
+    try std.testing.expect(result == null);
+    try std.testing.expectEqual(context_mod.ContextError.database_error, ctx.ctx_error);
+}
+
+// The remaining accessors that used to swallow database errors. All five are the
+// same defect: a stateless witness that cannot answer becomes a plain `null`, the
+// caller turns that into an EVM-level failure, and the block is accepted with a
+// fabricated result. Each is pinned separately so reverting any one site alone
+// fails a test.
+test "Host.accountInfo marks ctx_error when the account cannot be proven" {
+    var ctx = context_mod.Context(MissingTargetDb).new(.{}, primitives.SpecId.prague);
+    defer ctx.journaled_state.deinit();
+    var host = Host.init(MissingTargetDb, &ctx, null);
+
+    const unknown: primitives.Address = @splat(0xE1);
+    try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error);
+
+    const result = host.accountInfo(unknown);
+
+    try std.testing.expect(result == null);
+    try std.testing.expectEqual(context_mod.ContextError.database_error, ctx.ctx_error);
+}
+
+test "Host.selfdestruct marks ctx_error when the target cannot be proven" {
+    var ctx = context_mod.Context(MissingTargetDb).new(.{}, primitives.SpecId.prague);
+    defer ctx.journaled_state.deinit();
+    var host = Host.init(MissingTargetDb, &ctx, null);
+
+    _ = try ctx.journaled_state.loadAccount(MissingTargetDb.CALLER);
+    const unknown: primitives.Address = @splat(0xE2);
+    try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error);
+
+    const result = host.selfdestruct(MissingTargetDb.CALLER, unknown);
+
+    try std.testing.expect(result == null);
+    try std.testing.expectEqual(context_mod.ContextError.database_error, ctx.ctx_error);
+}
+
+// recordCreateTargetCore mirrors setupCreateCore's pre-checks to decide whether to
+// charge NEW_ACCOUNT gas, and loads the CREATE target the same way. It takes
+// `js: anytype`, so the same stub injection works.
+test "recordCreateTargetCore marks ctx_error when the CREATE target cannot be loaded" {
+    // Amsterdam: recordCreateTargetCore returns null immediately on earlier specs
+    // because EIP-7928 BAL recording is Amsterdam+ only, so it would never reach
+    // the target load and the test would pass for the wrong reason.
+    var ctx = context_mod.Context(MissingTargetDb).new(.{}, primitives.SpecId.amsterdam);
+    defer ctx.journaled_state.deinit();
+    var host = Host.init(MissingTargetDb, &ctx, null);
+
+    _ = try ctx.journaled_state.loadAccount(MissingTargetDb.CALLER);
+    try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error);
+
+    const result = recordCreateTargetCore(
+        &ctx.journaled_state,
+        &host,
+        MissingTargetDb.CALLER,
+        0,
+        &[_]u8{0x00},
+        false,
+        0,
+        0,
+    );
+
+    try std.testing.expect(result == null);
+    try std.testing.expectEqual(context_mod.ContextError.database_error, ctx.ctx_error);
+}