feat: add a hook channel into keyless sandbox execution

[RealiCZ]

Summary

KeylessDeploy (0x6342…0003) runs the wrapped deployment in a nested sandbox EVM, which is invisible to the parent EVM's inspector: tracers stop at the intercepted CALL. This PR adds a hook channel into that sandbox, SandboxInspector, whose signatures match revm's Inspector on the sandbox EVM, plus the tracing glue that lets mega-evme and node tracing RPCs render sandbox frames under the KeylessDeploy CALL.

The no-hook path is unchanged: with nothing attached, sandbox execution is byte-for-byte the same as before. Attaching a non-intervening hook leaves result, state, gas, and resource usage identical to the unattached run. Interventions are node-local and non-consensus.

What changed

• sandbox::inspector — SandboxInspector<E>: object-safe, all hooks default-empty / None, nine hooks that mirror revm-inspector 8.1.0's Inspector exactly (&mut inputs, override return values) so CALL/CREATE can be short-circuited and outcomes rewritten inside the sandbox as on a top-level EVM, plus a paired sandbox_start / sandbox_end lifecycle carrying SandboxStartInfo (signer, deploy address, gas limit override / effective / tx, and OuterCallInfo: the intercepted call's depth, caller, gas limit, remaining gas, value, and calldata) and SandboxEndOutcome (Applied { Deployed | EmptyCode | ExecutionFailed, gas_used } or NotApplied { Rejected | PostAccountingHalt | ApplyFailed | AddressMismatch }). A hook that returns None from call/create and leaves interpreter and context state alone observes without intervening, the same read-only notion the outer EVM's inspector has. EmptyCode is reported on every spec; the wire shape stays the frozen one (pre-REX5 drops the constructor's logs). Lifecycle events are delivered on the slot that also receives the sandbox's opcode-level hooks (EmptyExternalEnv pre-REX4, the parent env from REX4 on).
• Blanket impl: any type that implements Inspector for every sandbox context lifetime is a SandboxInspector. Local types that are not inspectors implement the trait by hand; coherence is pinned by unit tests.
• One type-erased slot on MegaContext (Rc<RefCell<dyn SandboxInspector<E>>>, held twice: parent env type and EmptyExternalEnv). Setting a hook replaces the previous one.
• Public API, forwarded identically from MegaContext, MegaEvm, and MegaBlockExecutor: set_keyless_sandbox_hook(Rc<RefCell<I>>) and clear_keyless_sandbox_hook (the only detach path). No take/drain API: recorded data stays in the caller's handle.
• External-env invariance: pre-REX4 sandboxes keep EmptyExternalEnv, REX4+ keep sharing the parent env, whether or not a hook is attached. with_external_envs resets the slot and debug-asserts it was empty.
• AddressMismatch is now checked before apply_sandbox_state. Unreachable on the no-hook path (the deploy address derives from the signer and the sandbox nonce is fixed); on the inspector channel a create short-circuit to another address must not leave applied state in the parent journal. A create short-circuit that returns no address is charged the same way (crate-private SandboxRun::NoContractCreated: REX5+ books the sandbox's gas and usage, nothing is applied, the outer call reverts with NoContractCreated); the no-hook path never produces it, and the public SandboxOutcome and execute_keyless_deploy_call keep their shapes.
• inspectors feature (optional revm-inspectors dependency): sandbox::trace ships SharedTracingInspector (outer EVM inspector), SandboxTracer (a hand-written SandboxInspector that never intervenes and records each sandbox execution into an arena of its own, keyed by the intercepted call from OuterCallInfo), paired to build both, and splice_sandbox_traces, which grafts every recorded sandbox under the outer KeylessDeploy CALL that started it and leaves the tracer empty. Splicing is idempotent; a sandbox whose call is not in the outer arena is dropped; a sandbox aborted by a database error inside a nested frame is closed as FatalExternalError; pre-REX5 empty-code deployments carry no logs (the receipt has none). The synthetic CREATE step on the intercepted CALL reports the outer frame's remaining gas and the sandbox reservation as gasCost. mega-evme trace/replay use it, and mega-reth's tracing RPCs use the same implementation.
• test_utils::keyless: shared corner-case init codes (CREATE then REVERT, three nesting levels with reverting calls and a log, sandbox out of gas, a call back into KeylessDeploy) used by the in-process tests, the offline state-test corpus, and the e2e suite.
• benches/mega_bench.rs: keyless_sandbox_hook rows (rex5/no_hook, rex5/hook) put the InspectorBridge → hook forwarding path under CodSpeed.

Contract (main)

1. No hook attached: the sandbox path is unchanged.
2. Hook attached, not intervening: result, state, gas, and usage identical to the unattached path.
3. Interventions take effect inside the sandbox per revm's top-level semantics; the parent frame records the post-intervention gas and usage as-is and does not check conservation.
4. The hook is node-local and non-consensus.
5. Later specs may measure interventions and refuse some shapes; integrators must not depend on this base being permissive.

Tests

• tests/rex2/keyless_sandbox_hook.rs: parity matrix REX2–REX6 × success/revert, REX5/REX6 resource halt, pre-REX4 crowded parent env, split-CREATE through the real interceptor; event ordering, reverted frames, short-circuited precompile frames balanced, lifecycle exactly once; all SandboxEndOutcome variants incl. ApplyFailed via an error-injecting database; a defaults-only hook attached to a logging, calling, creating, and self-destructing deployment leaves result, state, and usage identical to the no-hook run; EmptyCode is pinned on every spec together with the EmptyCodeDeployed wire shape; a hook with one impl per env sees a sandbox's lifecycle and opcode hooks on the same impl on every spec.
• tests/rex2/keyless_sandbox_inspector.rs: no-intervention parity (local no-op hook and TracingInspector) across specs and outcomes; interventions each with an unintervened control arm: call short-circuit, create short-circuit to AddressMismatch and to Rejected, call_end rewrite, step gas surcharge (sandbox gas_used exactly +1000; outer delta 0 pre-REX5, +1000 REX5+), journal writes, caller prank (inputs.caller rewritten on call, observed by the callee's SSTORE(CALLER)), creator prank (nested CREATE lands at pranked.create(0), parent nonce untouched); setting a hook replaces the previous one and clear restores parity; seven outcomes reached through the hook; a defaults-only hook is inert on a self-destructing constructor; a create short-circuit without an address books the same outer gas and usage as the address-mismatch arm.
• tests/rex2/keyless_sandbox_extreme.rs: the four corner-case shapes, each run with a recording hook and with no hook, with byte-identical results.
• tests/rex2/keyless_sandbox_trace.rs (feature inspectors): the paired tracer grafts the three-level tree with consistent arena links, step depths one level below their frame, and a synthetic CREATE step carrying the outer remaining gas and the reservation, with result parity against the no-hook run; splicing is idempotent and fuse resets the outer; one tracer pair across two transactions grafts only the transaction just traced; a recorded sandbox is dropped when spliced into an empty outer, an unrelated transaction, or after the outer was fused; a database error inside a nested sandbox frame grafts the CREATE and the helper CALL as FatalExternalError (both tracer configs); empty-code logs follow the receipt (stripped REX2–REX4, kept REX5+); a parity-config leaf CREATE is still grafted with the synthetic step; no extra step when the parent already pairs one call-like step per child; a sandbox SELFDESTRUCT reaches the tracer; the MegaBlockExecutor forwarders attach a plain TracingInspector, detach, and attach a SandboxTracer across a three-transaction block; a validate-rejected sandbox claims its frame and grafts nothing; the outer adapter forwards create/log/selfdestruct and the blanket impl forwards log/selfdestruct to a plain TracingInspector.
• bin/mega-evme: geth call / opcode / prestate rendering with sandbox frames, parity / flatCallTracer trace addresses ([] [0] [0,0] [0,0,0] [0,0,1] [0,1]), splicing into an outer inspector that recorded no frames drops the sandbox (the arena's default root is never used as a parent), three offline replay tests over captured mainnet REX2 transactions (the synthetic CREATE step pins depth, the remaining gas, and the reservation).
• Corpus: bench/replay/fixtures/keyless_deploy_rex2_{small,medium,large}.json (mainnet blocks 7487457 / 7560053 / 8064149, manifest case keyless_deploy) and eight keyless_sandbox_*_rex{5,6}.json corner-case fixtures filled with state-test --fill.
• Verification: cargo test -p mega-evm --all-features (1275 passed), cargo test -p mega-evme (83 passed), cargo test -p mega-state-test --test replay_corpus, cargo fmt --all --check, cargo clippy --workspace --lib --examples --tests --benches --all-features --locked, cargo sort --check …, cargo check -p mega-evm --target riscv64imac-unknown-none-elf --no-default-features, cargo doc -p mega-evm --no-deps (no new broken links).
• Real chain: three mainnet REX2 keyless deployments replayed on a mainnet RPC node with identical results and sandbox frames rendered; devnet end-to-end (mega-reth + mega-e2e branches) renders the sandbox in every tracing entry point across 13 sub-scenarios.

Docs

crates/mega-evm/src/sandbox/mod.rs and sandbox::inspector module docs and crates/mega-evm/src/evm/AGENTS.md ("Keyless sandbox hook") describe the hook, the contract, the env invariant, and the lifecycle slot rule. docs/mega-evme/tracing/ documents the synthetic CREATE entry and the log rule for pre-REX5 empty-code deployments. docs/spec/ is untouched: nothing here changes protocol behavior.

Review follow-ups

The first review round (15 findings at ce43e5b) is addressed as follows:

• Fixed: stale / duplicated sandbox trees across transactions and non-idempotent splicing (per-execution arenas keyed by the intercepted call, consuming splice); as_sandbox_observer aliasing the outer handle (removed; paired builds the two halves); unclosed sandbox root rendered as a success (closed as FatalExternalError); pre-REX5 empty-code logs shown but absent from the receipt (stripped); lifecycle hooks landing on a different env slot than the opcode hooks (same slot); SandboxCompletionKind docs wrong pre-REX5 (uniform EmptyCode, pinned on every spec); create override with address: None refunding the whole reservation (charged like AddressMismatch); undocumented synthetic CREATE gas placeholder (real remaining gas and reservation, documented, depth pinned); deep clone of the sandbox arena (moved); no benchmark for the hooked arm (keyless_sandbox_hook); Option setters (by value, clear_keyless_sandbox_hook detaches).
• Deferred to follow-ups: the AddressMismatch hoist relying on ordering rather than a journal checkpoint (design note); notify_sandbox_end pairing enforced at seven sites (labeled-block refactor); test helper duplication across the keyless test files and mega-evme.

The second round (maintainer review at f511680) asked to drop the read-only SandboxObserver channel and keep SandboxInspector only: the read-only guarantee was partial (step and the context were mutable on both traits), the outer EVM's inspector has no read-only variant and can rewrite every frame of canonical execution, and enforcing read-only-ness cost a clone of every CALL/CREATE input and outcome on the tracing path. Done: one trait, one setter (set_keyless_sandbox_hook), ReadOnlyHook and the blanket observer impl deleted, the lifecycle types live in sandbox::inspector, and SandboxTracer forwards without copies.

Notes for reviewers

• inspectors is off by default and does not affect no_std builds.
• Downstream: mega-reth (cz/feat/keyless-sandbox-trace) and mega-e2e (cz/feat/keyless-sandbox-trace) consume this branch; mega-reth pins a git rev and switches to the released tag once this lands.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-08T13:15:52.363292Z	7041fec	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[mega-maxwell]

Claude review status

Living comment — rewritten in place. The review workflow keeps this single comment up to date instead of posting a new one each round, so it always describes the latest reviewed commit and the earlier text is intentionally gone. No reply is needed here; reply to a finding in its own review thread, and answer an open question in a reply on this PR. The next review round reconciles your answer.

✅ Review clean

Last reviewed: ab11c3bb..7041fecf · updated 2026-09-08T13:15:39+00:00

New this round: 0 finding(s), 0 question(s) · Resolved this round: 0 · Open questions: 0

[github-actions]

🧬 Mutation testing — ✅ PASS

Diff mutation score: 100.0% (6/6 viable mutants killed)

• caught: 6
• survived (real gaps): 0
• timed out (inconclusive): 0
• suppressed (equivalent/dead-code): 0
• unviable: 0 · timeout total: 0

No new test gaps introduced by this change. 🎉

[codecov]

Codecov Report

❌ Patch coverage is 97.15262% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.9%. Comparing base (a15728d) to head (7041fec).

Files with missing lines	Patch %	Lines
crates/mega-evm/src/sandbox/execution.rs	92.8%	17 Missing and 2 partials ⚠️
crates/mega-evm/src/evm/context.rs	95.8%	2 Missing and 1 partial ⚠️
crates/mega-evm/src/sandbox/trace.rs	98.8%	1 Missing and 2 partials ⚠️

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

Merging this PR will not alter performance

⚠️ 16 benchmarks measured no execution time

Nothing ran under measurement, usually because the compiler removed the code under test. These results are not comparable, so they count as unchanged.

Preventing compiler optimizations

✅ 385 untouched benchmarks
🆕 2 new benchmarks
🗄️ 44 archived benchmarks run¹

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
🆕	rex5/no_hook	N/A	1.1 ms	N/A
🆕	rex5/hook	N/A	1.1 ms	N/A
⚠️	estimated_da_size[0]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[1000]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[180]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[68]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[0]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[1000]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[180]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[68]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[0]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[1000]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[180]	< 1 ns	< 1 ns	N/A
⚠️	estimated_da_size[68]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[0]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[1000]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[180]	< 1 ns	< 1 ns	N/A
⚠️	tx_size[68]	< 1 ns	< 1 ns	N/A

---

_{Comparing cz/feat/keyless-sandbox-observer (7041fec) with main (a15728d)}

Footnotes

1. 44 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them. ↩

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce43e5b22c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

🧬 Mutation testing — ✅ PASS

Diff mutation score: 100.0% (54/54 viable mutants killed)

• caught: 54
• survived (real gaps): 0
• timed out (inconclusive): 0
• suppressed (equivalent/dead-code): 0
• unviable: 97 · timeout total: 0

No new test gaps introduced by this change. 🎉

[flyq]

Review — observer/inspector channels and sandbox trace splicing

Reviewed at ce43e5b22c20 · xhigh effort · 15 findings

Reviewed the full diff against the branch checkout at ce43e5b. Locally: cargo fmt --all --check, cargo clippy --workspace --lib --examples --tests --benches --all-features --locked, cargo sort --check, Prettier on docs/, and the riscv no_std check all clean. Every correctness finding below was reproduced with a throwaway test, since reverted. Line numbers are from the post-change files at that commit.

Verdict: the core is sound; the tracing glue is not merge-ready yet. With no hook attached the sandbox path is semantically unchanged, the AddressMismatch hoist is unreachable on that path, and every wrapper forwards all nine revm Inspector hooks. The defects are in sandbox/trace.rs, which is the piece the PR advertises to mega-reth's tracing RPCs, and on exactly that shared-handle, multi-transaction usage it produces wrong output: stale sandbox trees under unrelated transactions (1), a silent root overwrite plus a BorrowMutError panic when the observer aliases the outer handle (2), a never-closed CREATE rendered as a success under a reverted CALL (3), and receipt-inconsistent logs on REX2–REX4 (4). None of these are reachable from mega-evme's own CLI, which builds fresh handles and splices once, so the PR's own tests are green. Findings 5–7 and 12 are public-contract mismatches an integrator will trip on; the rest is cleanup. Specifics are inline on the relevant lines.

What I verified

1. The AddressMismatch hoist does not change frozen-spec behavior. The sandbox pins the signer nonce to 0 via with_nonce_override, so on the no-hook path the CREATE address is a pure function of the signer and the pre-apply_sandbox_state check is structurally unreachable; only an inspector create short-circuit to another address can reach it. The no-hook path was restructured into the None arm of run_sandbox_ctx with the same body, matching contract item 1 and the spec:stable / spec:unchanged labels.
2. Every wrapper forwards all nine revm Inspector hooks. The blanket SandboxObserver/SandboxInspector impls, ReadOnlyHook, InspectorBridge, and SharedTracingInspector each cover initialize_interp, step, step_end, log, call, call_end, create, create_end, selfdestruct.
3. Two candidates were refuted and are not reported. The manifest note's 25.5M gas figure is correct once the 10,000-gas/byte code-deposit charge is included; the state-test runner's inability to trace sandbox frames predates this PR.
4. Lint and portability gates are clean, --locked metadata resolves, all new #[test] functions carry the test_ prefix, and docs/ passes Prettier.

Ties to existing signal

The cargo-mutants gate failure has two survivors in sandbox/trace.rs that map directly onto findings here: fuse replaced with () at trace.rs:48 (nothing observes the reset, finding 1) and the + 1 on create_depth at trace.rs:196 (the synthetic step's depth is unpinned, finding 8). The tests for those two findings should also turn the gate green. Codex's P2 on trace.rs:159 covers only the rposition → None fallback slice of finding 1; the missing per-transaction reset, the non-idempotence, and the aliasing footgun in finding 2 are new.

Inline findings

#	Where	Finding
1	crates/mega-evm/src/sandbox/trace.rs:159	splice grafts stale/duplicate sandbox trees across txs
2	crates/mega-evm/src/sandbox/trace.rs:62	as_sandbox_observer on outer handle clobbers root, panics
3	crates/mega-evm/src/sandbox/trace.rs:169	Unclosed sandbox root CREATE grafted and rendered as success
4	crates/mega-evm/src/sandbox/trace.rs:147	REX2-REX4 empty-code deploy trace shows logs receipt dropped
5	crates/mega-evm/src/sandbox/execution.rs:446	Lifecycle and opcode hooks dispatched on different env slots
6	crates/mega-evm/src/sandbox/observer.rs:125	SandboxCompletionKind docs wrong pre-REX5 for empty code
7	crates/mega-evm/src/sandbox/execution.rs:563	create override with address None refunds whole reservation
8	crates/mega-evm/src/sandbox/trace.rs:198	Synthetic CREATE step gas placeholder is undocumented
9	crates/mega-evm/src/sandbox/execution.rs:494	AddressMismatch hoist relies on ordering, not a checkpoint
10	crates/mega-evm/src/sandbox/trace.rs:167	Splice deep-clones the whole sandbox arena including stacks
11	crates/mega-evm/src/sandbox/execution.rs:719	No Criterion/CodSpeed bench for the hooked sandbox arm
12	crates/mega-evm/src/sandbox/observer.rs:324	Observer read-only guarantee structural only for call/create
13	crates/mega-evm/src/sandbox/execution.rs:444	notify_sandbox_end pairing enforced by convention at 7 sites
14	crates/mega-evm/src/evm/context.rs:515	Option setter keeps a dead None arm on 3 public types
15	crates/mega-evm/tests/rex2/keyless_sandbox_support.rs:185	Test helpers re-derived instead of using test_utils::keyless

Suggested path to merge

1. Fix findings 1 and 2 together in sandbox/trace.rs: early-return on None, consume-and-fuse the sandbox arena in the splice (finding 10's mem::take shape gives idempotence for free), a paired() constructor or ptr_eq guard, and a two-transaction test.
2. Close dangling sandbox roots in the remap closure (3) and strip receipt-inconsistent logs for the REX2–REX4 empty-code arm (4) — both tracing-layer only, no frozen-spec risk.
3. Pin the synthetic CREATE step's depth in replay_keyless.rs and either surface real remaining gas or document the placeholder (8). Steps 1 and 3 together should clear the mutation gate.
4. Settle the three public-contract items before mega-reth pins a release: lifecycle-slot dispatch rule (5), SandboxCompletionKind docs or uniform EmptyCode (6), and the address: None refund carve-out (7). Finding 14's signature change is cheapest in the same pass.
5. Findings 9, 11, 12, 13, 15 can follow up separately; 12 deserves at least the negative test now so the read-only promise is not purely documentary.

Not reported inline (lower value, all confirmed): unused PartialEq derive on LimitUsage (crates/mega-evm/tests/rex2/keyless_sandbox_support.rs:339); the Some/None arms of run_sandbox_ctx duplicate the sandbox construction (crates/mega-evm/src/sandbox/execution.rs:718); in bin/mega-evme/src/replay/cmd.rs:475 the outer SharedTracingInspector is built unconditionally while the sandbox handle is gated on is_tracing_enabled().

[flyq]

[MEDIUM] api-contract · CONFIRMED

The observer channel's read-only guarantee is structural only for call/create; step-family hooks hand &mut through, and nothing pins that.

The observer channel's structural read-only guarantee covers only CALL/CREATE inputs and overrides (clone-and-discard at 324/336/342/353 plus ReadOnlyHook::call/create returning None at 431/451); step/step_end/initialize_interp/log forward &mut Interpreter and &mut MegaContext verbatim, so a step-mutating type is accepted by set_keyless_sandbox_observer and rewrites sandbox state, no negative test pins that, and the tracing path pays a three-deep RefCell/dyn chain per hook that as_sandbox_observer could shorten by one layer.

Suggested fix: At minimum a negative test that a step-mutating observer is rejected or documented as unsupported; ideally one trait with a read-only mode on InspectorBridge. Related: as_sandbox_observer could return Rc::clone(&self.0) directly, saving one RefCell/dyn layer per hook.

Failure scenario

A caller attaches through the 'read-only' channel a type whose step calls interp.gas.record_cost(..) or context.journal_mut().sstore(..) (a one-token trait rename SandboxInspector -> SandboxObserver on the test-side StepGasSurcharge/JournalWriter yields a compiling mutating observer): ReadOnlyHook::step (observer.rs:402) hands the real &mut through, the mutation lands in the sandbox journal, is merged into the parent, and the node diverges from consensus; the only guard is the doc sentence at observer.rs:24-31, and test_observer_channel_drops_call_overrides (keyless_sandbox_observer.rs:702) only proves a call override is dropped. Per sandbox opcode the observer channel runs three borrow_mut/dyn hops, twice (step + step_end); as_sandbox_observer (trace.rs:62-64) could return Rc::clone(&self.0) since TracingInspector: Inspector already satisfies the blanket bound.

[flyq]

[MINOR] robustness · CONFIRMED

notify_sandbox_end pairing is enforced by convention at seven sites.

notify_sandbox_end is hand-placed at seven terminal sites (472, 488, 499, 515, 528, 545, 565) with three early returns after sandbox_start (478, 494, 502), so the 'fires exactly once' INVARIANT comment at 444-445 is enforced only by convention; all four frame-result macros evaluate to FrameResult values with no return and the match at 458 is the tail expression, so a labeled block yielding (SandboxEndOutcome, FrameResult) once would make the pairing compiler-enforced.

Suggested fix: A labeled block yielding (SandboxEndOutcome, FrameResult) with one notify_sandbox_end call after it makes the pairing compiler-enforced.

Failure scenario

A future terminal arm (e.g. a REX7 post-sandbox rejection per contract item 5) added without its own notify_sandbox_end silently breaks the sandbox_start/sandbox_end pairing for every hook; it is caught only if a test drives that arm (the pairing is pinned solely by assert_single_start_end_pair, keyless_sandbox_observer.rs:316-322, on the paths tests happen to exercise). Ordering is unobservable to hooks: sandbox_end receives only &SandboxEndOutcome and ctx.log at 511/524 is a journal push that fires no Inspector::log hook.

[flyq]

[MINOR] reuse · CONFIRMED

Test-side duplication the PR's own test_utils::keyless was created to end.

The PR's new test code re-derives helpers the PR itself created a shared home for: the r=s=0x2222 pre-EIP-155 signer / keylessDeployCall builder / funded_db / TEST_CALLER and LARGE_* constants are copied again into keyless_sandbox_support.rs and bin/mega-evme/src/common/trace.rs:319 (now 11 signer copies in 9 files) instead of test_utils/keyless.rs; the keyless run harness is spelled three times (support.rs:271-386, keyless_sandbox_inspector.rs:106-169, keyless_sandbox_extreme.rs:156) differing only in the setter; three recorders with overlapping event enums exist (RecordingObserver is defined twice with different bodies); and test_utils/keyless.rs keeps call_and_pop / return_stop_runtime / INTERNAL_CALL_GAS private so tests re-spell them with a bare 50_000_u32 that push_number emits as PUSH4 instead of the u16 helper's PUSH2.

Suggested fix: Move the signer/call-tx/funded_db helpers and constants into test_utils::keyless, make the bytecode fragments pub, and keep one harness taking attach: impl FnOnce(&mut MegaContext) with one recorder type.

Failure scenario

A change to the keyless signing scheme, the keylessDeployCall ABI, or default gas constants must be edited in 11 signer copies and ~10 call-tx builders; a run-config field change touches 3 harness bodies and 32 seven-field struct literals; retuning INTERNAL_CALL_GAS leaves five 50_000 literals silently diverging, and the copies are not byte-identical to the shared helper (PUSH4 vs PUSH2); hex!("60006000fd") is still spelled at support.rs:66 / observer.rs:610 / inspector.rs:657 despite the new REVERTING_RUNTIME.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cbe58f0437

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Troublor]

Requesting a change before merge: drop SandboxObserver entirely and keep only SandboxInspector.

The two traits declare the same hook set; the only difference is that the observer removes the write channels (&CallInputs instead of &mut CallInputs, no Option<CallOutcome> return). What that buys is that a hand-written read-only hook cannot intervene. Three things make that thinner than it looks:

1. The guarantee is already partial. step, step_end, and the &mut MegaContext argument stay mutable on both traits — as the module docs say, that half is contractual. An observer that mutates interpreter or context state changes results regardless.

2. The outer EVM already takes the opposite position. MegaEvm::with_inspector / create_executor_with_inspector accept any Inspector with no read-only variant, and inspect_frame_init honours an override outright ("Inspector intercepted — frame_init() is skipped entirely"). That inspector can rewrite every frame of every transaction in canonical block execution. A read-only channel only for the sandbox applies a stricter policy to the inner EVM than to the outer one, which is the wrong way round — the outer hook can do strictly more damage.

3. It costs per-frame work on the path the feature exists for. The blanket impl enforces read-only-ness by cloning CallInputs / CallOutcome and discarding the copy (observer.rs:367,379,385,396), and SandboxTracer repeats the same dance (trace.rs:279-311) because the observer hands it & while TracingInspector wants &mut. On one trait it forwards directly and both sets of clones disappear.

Concretely:

• Delete the trait and its blanket impl (observer.rs:209-411) and ReadOnlyHook (412-522).
• Keep SandboxStartInfo, OuterCallInfo, SandboxEndOutcome, SandboxCompletionKind, SandboxRejectKind — fold them into inspector.rs and drop observer.rs.
• Collapse to one setter, which can then be named for what it is: set_keyless_sandbox_hook / clear_keyless_sandbox_hook. The ExtEnvs: 'static bound that only the observer setter carries (context.rs:517) goes with it, so that "Notes for reviewers" item disappears too.
• SandboxTracer implements SandboxInspector and returns None from call / create.

Read-only then means what it means in revm: your hooks return None, which is the trait's default body anyway. The exclusivity rule, the "attaching one replaces the other" semantics, and the aliasing hazard between the two channels all stop being things a reader has to learn.

This is public API that mega-reth and mega-e2e already track, so collapsing it after a released tag is a breaking change for them — I'd rather it not ship in this shape. The change is mechanical (about 310 lines deleted, and the ~34 set_keyless_sandbox_observer call sites are almost all tests), so I'm happy to turn a follow-up push around quickly.