diff --git a/build.zig b/build.zig index d1fb13f..42fb00f 100644 --- a/build.zig +++ b/build.zig @@ -538,6 +538,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" }, diff --git a/src/evm/context/journal.zig b/src/evm/context/journal.zig index 677c958..f81be61 100644 --- a/src/evm/context/journal.zig +++ b/src/evm/context/journal.zig @@ -1672,8 +1672,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; } @@ -1691,3 +1704,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)))); +} diff --git a/src/evm/context/main.zig b/src/evm/context/main.zig index 482f67a..bdffaa5 100644 --- a/src/evm/context/main.zig +++ b/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"); +} diff --git a/src/evm/interpreter/host.zig b/src/evm/interpreter/host.zig index 0aeaf6f..c4f3bac 100644 --- a/src/evm/interpreter/host.zig +++ b/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() }; }; @@ -1169,3 +1203,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); +} diff --git a/src/stateless/db/main.zig b/src/stateless/db/main.zig index 8749648..890fbfd 100644 --- a/src/stateless/db/main.zig +++ b/src/stateless/db/main.zig @@ -130,11 +130,18 @@ pub const WitnessDatabase = struct { else => return DbError.InvalidWitness, }; + // These puts must propagate rather than `catch {}`. storageRootFor() is a + // bare cache `get()`, so a dropped entry is indistinguishable from "this + // account was never loaded", and the post-execution batch trie update + // treats that as "no pre-state storage" and rebuilds the storage trie from + // only the touched slots (see executor/output.zig:188). Swallowing the + // failure would therefore turn an allocation failure into a silently wrong + // state root reported as success. const as = account_state orelse { - self.storage_root_cache.put(address, EMPTY_TRIE_HASH) catch {}; + try self.storage_root_cache.put(address, EMPTY_TRIE_HASH); return null; }; - self.storage_root_cache.put(address, as.storage_root) catch {}; + try self.storage_root_cache.put(address, as.storage_root); return state.AccountInfo{ .balance = as.balance, .nonce = as.nonce, @@ -176,16 +183,29 @@ pub const WitnessDatabase = struct { address, self.node_index, ) catch |err| switch (err) { - error.InvalidProof => break :blk EMPTY_TRIE_HASH, + // InvalidProof means the witness lacks the node needed to prove + // anything here — genuine absence is reported as a null + // account_state below ("valid non-inclusion" in mpt/main.zig). + // Defaulting to EMPTY_TRIE_HASH would make every subsequent slot + // read on this account return 0, i.e. a wrong execution result + // from an incomplete witness. Matches basic()'s handling. + error.InvalidProof => return DbError.InvalidWitness, else => return DbError.InvalidWitness, }; const root = if (account_state) |as| as.storage_root else EMPTY_TRIE_HASH; - self.storage_root_cache.put(address, root) catch {}; + // Propagate, for the same reason as in basic(): a dropped entry later + // reads as "no pre-state storage" and yields a wrong state root. + try self.storage_root_cache.put(address, root); break :blk root; }; const slot = u256ToHash(index); const value = mpt.verifyStorageIndexed(storage_root, slot, self.node_index) catch |err| switch (err) { - error.InvalidProof => return 0, + // A slot that is genuinely unset yields null from verifyStorageIndexed + // (valid non-inclusion), which becomes 0 without an error. Reaching + // InvalidProof means the storage node is missing from the witness, so + // returning 0 would fabricate a value — the slot's real contents are + // unknown and may be non-zero. + error.InvalidProof => return DbError.InvalidWitness, else => return DbError.InvalidWitness, }; return value; @@ -193,7 +213,12 @@ pub const WitnessDatabase = struct { // ── hasNonZeroStorageForAddress ───────────────────────────────────────── - pub fn hasNonZeroStorageForAddress(self: *const Self, address: primitives.Address) bool { + /// Fallible: this feeds the CREATE collision check, so "I cannot prove it" + /// must not collapse into "no storage here". Doing so would let a CREATE + /// succeed at an address the reference rejects — a consensus-level wrong + /// result from an incomplete witness. A genuinely absent account still + /// resolves to null (valid non-inclusion) and returns false without error. + pub fn hasNonZeroStorageForAddress(self: *const Self, address: primitives.Address) !bool { if (self.storage_root_cache.get(address)) |root| { return !std.mem.eql(u8, &root, &EMPTY_TRIE_HASH); } @@ -201,7 +226,7 @@ pub const WitnessDatabase = struct { self.pre_state_root, address, self.node_index, - ) catch return false; + ) catch return DbError.InvalidWitness; const as = account_state orelse return false; return !std.mem.eql(u8, &as.storage_root, &EMPTY_TRIE_HASH); } diff --git a/src/stateless/db/test.zig b/src/stateless/db/test.zig index d074146..3229a83 100644 --- a/src/stateless/db/test.zig +++ b/src/stateless/db/test.zig @@ -382,3 +382,260 @@ test "blockHash returns InvalidWitness for missing hash" { defer wdb.deinit(); try std.testing.expectError(error.InvalidWitness, wdb.blockHash(12345678)); } + +// ─── Regression: storage-root cache must not be silently dropped ────────────── + +// `storageRootFor()` is a bare `storage_root_cache.get()`, so a missing entry is +// indistinguishable from "this account was never loaded". The post-execution +// batch trie update relies on that distinction: for a null pre-state root, +// computeStorageRootBatch (src/stateless/executor/output.zig:188) rebuilds the +// storage trie from only the slots execution touched, as if the account had no +// pre-state storage. For an account that *does* have pre-state storage, that +// silently drops every untouched slot and yields a WRONG state root, reported as +// success. +// +// So an allocation failure while caching the root must never be swallowed: the +// three `storage_root_cache.put(...)` sites in db/main.zig must propagate, not +// `catch {}`. This test asserts the invariant that makes the downstream +// assumption safe: +// +// basic() succeeded => storageRootFor() knows the account's storage root +// +// It drives every allocation index in turn, so it does not depend on how many +// allocations basic() happens to make. A wrong state root returned as success is +// the worst failure mode for a proving system, hence pinning it here. +test "basic must not succeed while silently dropping the storage-root cache entry" { + var address: primitives.Address = @splat(0x00); + address[19] = 0x33; + const key_hash = mpt.keccak256(&address); + + // A non-empty storage root: this is what must not be lost. + const storage_root: primitives.Hash = @splat(0x5a); + + var account_rlp: [200]u8 = undefined; + const account_len = buildAccountRlp(&account_rlp, 1, 500, storage_root, KECCAK_EMPTY); + var leaf_node: [512]u8 = undefined; + const leaf_len = buildLeafNode(&leaf_node, key_hash, account_rlp[0..account_len]); + const leaf_bytes = leaf_node[0..leaf_len]; + const state_root = mpt.keccak256(leaf_bytes); + + const w = input.StateWitness{ + .state_root = state_root, + .nodes = &[_][]const u8{leaf_bytes}, + .codes = &.{}, + .keys = &.{}, + .headers = &.{}, + }; + + // The node index is built with the real allocator; only the database's own + // allocations (which include the storage-root cache) are made to fail. + var idx = try mpt.buildNodeIndex(ALLOC, w.nodes); + defer idx.deinit(); + + var fail_index: usize = 0; + while (fail_index < 16) : (fail_index += 1) { + var failing = std.testing.FailingAllocator.init(ALLOC, .{ .fail_index = fail_index }); + var wdb = db_mod.WitnessDatabase.init( + failing.allocator(), + &idx, + w.state_root, + w.codes, + &.{}, + ) catch continue; // init itself ran out of memory: nothing to check + defer wdb.deinit(); + + const info = wdb.basic(address) catch continue; // propagated: correct + if (info == null) continue; // account absent: no root to cache + + // basic() reported success, so the cached root must be present and right. + const cached = wdb.storageRootFor(address); + if (cached == null) { + std.debug.print( + "\nfail_index={d}: basic() succeeded but storageRootFor() is null;\n" ++ + "the batch trie update will rebuild storage from scratch and\n" ++ + "produce a wrong state root with no error raised.\n", + .{fail_index}, + ); + return error.StorageRootSilentlyDropped; + } + try std.testing.expectEqualSlices(u8, &storage_root, &cached.?); + } +} + +// ─── Regression: an unprovable slot must not read as zero ───────────────────── + +// The MPT layer distinguishes two outcomes precisely (src/stateless/mpt/main.zig): +// `null` means *proved absent* — the comments there read "valid non-inclusion" — +// whereas `error.InvalidProof` is returned by +// `findNodeInIndex(...) orelse return error.InvalidProof`, i.e. the witness does +// not contain the node needed to prove anything at all. +// +// So InvalidProof means "cannot verify", not "absent". basic() already treats it +// that way (it maps InvalidProof to InvalidWitness, with a documented +// SYSTEM_ADDRESS carve-out), but storage() maps it to `return 0` +// (db/main.zig:186) and the storage-root lookup maps it to EMPTY_TRIE_HASH +// (:175). An incomplete witness therefore reads as "slot is zero" instead of +// failing, which lets a wrong state transition be executed and proved. +// +// Here slot 3 genuinely holds 0xabcd, but its storage leaf is withheld from the +// witness, so a zero result is demonstrably wrong rather than merely unproven. +test "storage must fail, not return 0, when the witness cannot prove the slot" { + var address: primitives.Address = @splat(0x00); + address[19] = 0x77; + const slot_key: u256 = 3; + + var slot_hash: primitives.Hash = @splat(0); + { + var n = slot_key; + var si: usize = 32; + while (si > 0) { + si -= 1; + slot_hash[si] = @intCast(n & 0xff); + n >>= 8; + } + } + const storage_key_hash = mpt.keccak256(&slot_hash); + const rlp_value = &[_]u8{ 0x82, 0xab, 0xcd }; // slot 3 = 0xabcd + var storage_leaf: [256]u8 = undefined; + const storage_leaf_len = buildLeafNode(&storage_leaf, storage_key_hash, rlp_value); + const storage_root = mpt.keccak256(storage_leaf[0..storage_leaf_len]); + + const acc_key_hash = mpt.keccak256(&address); + var account_rlp: [200]u8 = undefined; + const account_len = buildAccountRlp(&account_rlp, 0, 0, storage_root, KECCAK_EMPTY); + var acc_leaf: [512]u8 = undefined; + const acc_leaf_len = buildLeafNode(&acc_leaf, acc_key_hash, account_rlp[0..account_len]); + const acc_leaf_bytes = acc_leaf[0..acc_leaf_len]; + const state_root = mpt.keccak256(acc_leaf_bytes); + + // The account leaf is present; the storage leaf is deliberately withheld, so + // the storage trie cannot be walked at all. + const w = input.StateWitness{ + .state_root = state_root, + .nodes = &[_][]const u8{acc_leaf_bytes}, + .codes = &.{}, + .keys = &.{}, + .headers = &.{}, + }; + var idx: mpt.NodeIndex = undefined; + var wdb = try makeWdb(w, &idx); + defer idx.deinit(); + defer wdb.deinit(); + + const result = wdb.storage(address, slot_key); + if (result) |value| { + std.debug.print( + "\nstorage() returned {d} for an unprovable slot whose true value is 0xabcd;\n" ++ + "an incomplete witness reads as 'slot is zero' instead of failing.\n", + .{value}, + ); + return error.UnprovableSlotReadAsZero; + } else |_| { + // Any error is acceptable here; the point is that it must not succeed. + } +} + +// ─── Regression: CREATE collision check must not guess ──────────────────────── + +// hasNonZeroStorageForAddress feeds the CREATE collision check +// (src/evm/interpreter/host.zig, setupCreateCore). It used to `catch return +// false`, so a witness that cannot prove the target account read as "no storage +// here" and allowed a CREATE that the reference rejects — a consensus-level wrong +// result. It is now fallible; the journal wrapper records the error so the block +// is rejected after execution, and fails the CREATE closed in the meantime. +// +// Note a genuinely absent account is NOT an error: it resolves via valid +// non-inclusion to false, which the second half of this test pins so the fix +// cannot be "just always return an error". +test "hasNonZeroStorageForAddress must fail when the account cannot be proven" { + var address: primitives.Address = @splat(0x00); + address[19] = 0x88; + + // A state root whose account node is absent from the witness: nothing can be + // proven about this address one way or the other. + const unprovable_root: primitives.Hash = @splat(0x9e); + const w = input.StateWitness{ + .state_root = unprovable_root, + .nodes = &.{}, + .codes = &.{}, + .keys = &.{}, + .headers = &.{}, + }; + var idx: mpt.NodeIndex = undefined; + var wdb = try makeWdb(w, &idx); + defer idx.deinit(); + defer wdb.deinit(); + + if (wdb.hasNonZeroStorageForAddress(address)) |answer| { + std.debug.print( + "\nhasNonZeroStorageForAddress returned {} for an unprovable account;\n" ++ + "the CREATE collision check would proceed on a fabricated answer.\n", + .{answer}, + ); + return error.UnprovableAccountAnsweredAnyway; + } else |_| { + // Correct: the caller is told we cannot determine this. + } +} + +test "hasNonZeroStorageForAddress reports false for a provably absent account" { + var address: primitives.Address = @splat(0x00); + address[19] = 0x99; + + // An empty branch root proves non-inclusion for every address, so the answer + // is knowable and must be `false`, not an error. + var branch: [18]u8 = undefined; + const branch_len = buildEmptyBranchNode(&branch); + const branch_bytes = branch[0..branch_len]; + const state_root = mpt.keccak256(branch_bytes); + + const w = input.StateWitness{ + .state_root = state_root, + .nodes = &[_][]const u8{branch_bytes}, + .codes = &.{}, + .keys = &.{}, + .headers = &.{}, + }; + var idx: mpt.NodeIndex = undefined; + var wdb = try makeWdb(w, &idx); + defer idx.deinit(); + defer wdb.deinit(); + + try std.testing.expect(!(try wdb.hasNonZeroStorageForAddress(address))); +} + +// Covers the OTHER InvalidProof site in storage(): the storage-root lookup +// (db/main.zig, the verifyAccountIndexed call inside storage()). Here the ACCOUNT +// node is missing, not the storage node, so the account's storage root cannot be +// determined. Defaulting to EMPTY_TRIE_HASH would make every slot on this account +// read as 0. Distinct from the test above, which withholds the storage leaf and +// exercises the verifyStorageIndexed site. +test "storage must fail when the account's storage root cannot be proven" { + var address: primitives.Address = @splat(0x00); + address[19] = 0xaa; + + // Empty node pool: the account node behind this root is absent, so nothing + // about the account (including its storage root) can be proven. + const unprovable_root: primitives.Hash = @splat(0x7c); + const w = input.StateWitness{ + .state_root = unprovable_root, + .nodes = &.{}, + .codes = &.{}, + .keys = &.{}, + .headers = &.{}, + }; + var idx: mpt.NodeIndex = undefined; + var wdb = try makeWdb(w, &idx); + defer idx.deinit(); + defer wdb.deinit(); + + // Cache is empty, so this goes through the storage-root lookup path. + if (wdb.storage(address, 3)) |value| { + std.debug.print( + "\nstorage() returned {d} for an account whose storage root is unprovable;\n" ++ + "every slot on this account would read as 0.\n", + .{value}, + ); + return error.UnprovableAccountStorageReadAsZero; + } else |_| {} +} diff --git a/src/stateless/executor/transition.zig b/src/stateless/executor/transition.zig index 06b09fc..c4bc9ee 100644 --- a/src/stateless/executor/transition.zig +++ b/src/stateless/executor/transition.zig @@ -1523,3 +1523,115 @@ fn extractPostState( return post; } + +// ─── Tests ──────────────────────────────────────────────────────────────────── + +const TEST_SENDER: input.Address = @splat(0x11); +const TEST_COINBASE: input.Address = @splat(0x22); +const TEST_RECIPIENT: input.Address = @splat(0x33); + +// A database that resolves only the accounts a block legitimately needs and fails +// for anything else, standing in for a stateless witness that is missing the +// CREATE target account. Mirrors WitnessDatabase's duck-typed surface. +const PartialWitnessDb = struct { + fn known(address: input.Address) bool { + return std.mem.eql(u8, &address, &TEST_SENDER) or + std.mem.eql(u8, &address, &TEST_COINBASE) or + std.mem.eql(u8, &address, &TEST_RECIPIENT); + } + + pub fn basic(_: *@This(), address: input.Address) !?state_mod.AccountInfo { + if (!known(address)) return error.InvalidWitness; + var info = state_mod.AccountInfo.default(); + info.balance = 1_000_000_000_000; + return info; + } + + 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 0; + } + + pub fn blockHash(_: *@This(), _: u64) !primitives.Hash { + return @splat(0); + } +}; + +fn testEnv() input.Env { + return .{ .coinbase = TEST_COINBASE, .gas_limit = 30_000_000, .base_fee = 0 }; +} + +fn makeTestCtx() context_mod.Context(PartialWitnessDb) { + return context_mod.Context(PartialWitnessDb).new(.{}, primitives.SpecId.shanghai); +} + +fn runTestBlock(arena: std.mem.Allocator, ctx: anytype, txs: []input.TxInput) !TransitionResult { + const pre_alloc: std.AutoHashMapUnmanaged(input.Address, input.AllocAccount) = .{}; + return transitionWithContext( + arena, + ctx, + pre_alloc, + testEnv(), + txs, + primitives.SpecId.shanghai, + 1, + 0, + &.{}, + ); +} + +// Integration counterpart to the unit tests in db/test.zig and host.zig: those +// prove the database error is raised and that setupCreateCore marks ctx_error, +// this proves it survives a whole block execution. transitionWithContext does not +// itself reject -- the block-level driver (executor/main.zig) turns a non-ok +// ctx_error into InvalidWitness -- so the contract asserted here is that the block +// is *marked* invalid rather than completing as if the CREATE legitimately failed. +test "a CREATE whose target is absent from the witness marks the block invalid" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + + // to == null makes this a CREATE. The target address is derived from the + // sender and nonce, so it is not one of the addresses the stub DB can resolve. + var txs = [_]input.TxInput{.{ + .from = TEST_SENDER, + .to = null, + .nonce = 0, + .gas = 200_000, + .gas_price = 1, + .data = &[_]u8{ 0x60, 0x00, 0x60, 0x00, 0xf3 }, // RETURN empty + }}; + + var ctx = makeTestCtx(); + // Whether this returns a value or an error is not the contract; the mark is. + _ = runTestBlock(arena_state.allocator(), &ctx, &txs) catch {}; + + try std.testing.expectEqual( + context_mod.ContextError.database_error, + ctx.ctx_error, + ); +} + +// Control: the same stub DB and block shape, but a plain transfer to an address +// the witness *can* prove. Without this, the test above would also pass if every +// block were marked invalid for an unrelated reason. +test "a block whose accounts are all provable is not marked invalid" { + var arena_state = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena_state.deinit(); + + var txs = [_]input.TxInput{.{ + .from = TEST_SENDER, + .to = TEST_RECIPIENT, + .nonce = 0, + .gas = 100_000, + .gas_price = 1, + .value = 1, + }}; + + var ctx = makeTestCtx(); + _ = try runTestBlock(arena_state.allocator(), &ctx, &txs); + + try std.testing.expectEqual(context_mod.ContextError.ok, ctx.ctx_error); +}