Conversation
khordadi
left a comment
khordadi
left a comment
There was a problem hiding this comment.
Notes for the reviewer on the non-obvious lines.
There was a problem hiding this comment.
🟢 Approval recommended
The refactor keeps evmc confined to the host vtable boundary and includes strong compile-time layout checks plus updated call sites/tests with no evident correctness regressions.
Pull request overview
Ports transaction-context producers to an in-tree monad_tx_context while keeping evmc_tx_context at the vm::Host vtable boundary (via a single by-value copy in EvmcHostBase). This continues the repo-wide effort to push the evmc dependency toward leaf ABI boundaries without changing live execution behavior.
Changes:
- Added
monad_tx_contextand ato_evmc_tx_context()bridge, with compile-time layout/offset verification againstevmc_tx_context. - Updated
get_tx_context()/system_call()producers to build/returnmonad_tx_context, and updatedEvmcHostBaseto store an internalevmc_tx_contextcopy for the vtable slot. - Updated affected tests/utilities and a couple of comments to reference
monad_tx_context.
Verdict: CORRECT
File summaries
| File | Description |
|---|---|
| test/vm/utils/test_host.hpp | Switch test host’s stored tx context to monad_tx_context. |
| category/vm/evm/tx_context.hpp | Introduce monad_tx_context and declare to_evmc_tx_context bridge. |
| category/vm/evm/tx_context.cpp | Prove layout parity with evmc_tx_context and implement std::bit_cast bridge. |
| category/vm/evm/CMakeLists.txt | Add new tx-context sources to monad-vm-evm target. |
| category/rpc/monad_executor.cpp | Update comment to reference monad_tx_context.block_round. |
| category/execution/runloop/runloop_monad.cpp | Update comment to reference monad_tx_context.block_round. |
| category/execution/monad/reserve_balance/reserve_balance_contract_test.cpp | Use monad_tx_context in test setup. |
| category/execution/ethereum/tx_context.hpp | Change EMPTY_TX_CONTEXT and API to monad_tx_context. |
| category/execution/ethereum/tx_context.cpp | Produce monad_tx_context from get_tx_context(). |
| category/execution/ethereum/test/test_call_trace.cpp | Update tests to use monad_tx_context. |
| category/execution/ethereum/process_requests.cpp | Build monad_tx_context for system_call() execution path. |
| category/execution/ethereum/evmc_host.hpp | Store an evmc_tx_context by value; accept monad_tx_context in ctor. |
| category/execution/ethereum/evmc_host.cpp | Construct the stored evmc_tx_context via to_evmc_tx_context(). |
| category/execution/ethereum/evmc_host_test.cpp | Update comparison helper and expected context type to monad_tx_context. |
| category/execution/ethereum/block_hash_history_test.cpp | Update test to use monad_tx_context result type. |
🤖 Generated with Claude Code
Review details
- Files reviewed: 15/15 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed per REVIEW.md. The port is clean: layout compatibility between monad_tx_context and evmc_tx_context is pinned exhaustively at compile time (total size, per-field offsets, integer field types — a reorder or dropped field fails the build); the switch from a reference member to a by-value evmc_tx_context copy in EvmcHostBase is safe at every construction site (all callers build a local const context and never mutate it after constructing the host, and the copy removes a dangling-reference hazard rather than adding one); the updated sizeof static_asserts (328/392) match the 8-byte-reference-to-264-byte-value change; the VM consumer side is deliberately untouched per the staging described for EXE-173; and category/vm/evm → category/core is an established dependency (delegation.hpp already includes category/core/address.hpp, and the target links monad_core PUBLIC). Single-commit PR, rebased on main, no FFI or RLP surface touched.
One non-blocking inline comment: to_evmc_tx_context should be noexcept, since it is invoked from a noexcept constructor's member-init list.
Verdict: CORRECT
🤖 Generated with Claude Code
9608478 to
c1d9357
Compare
4b6a751 to
b097d3a
Compare
khordadi
changed the title
evmc_tx_context producers to in-tree monad_tx_contextmonad_tx_context type
b097d3a to
85d3732
Compare
EXE-171, part of EXE-162. monad_tx_context mirrors evmc_tx_context field-for-field with bytes32_t/Address members, including the fork's block_round. tx_context.cpp asserts standard layout, sizeof, alignof, the offsetof of all 15 fields and the exact type of the six integer fields. The header has no evmc include. Purely additive; nothing references the type yet. The producer migration goes with the vm::Host port (EXE-173), where get_tx_context returns the in-tree type and the host holds no evmc copy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
85d3732 to
fe4b98d
Compare
Part of EXE-171 (EXE-162), reduced to the type.
monad_tx_contextmirrorsevmc_tx_contextfield-for-field withbytes32_t/Addressmembers, including the fork'sblock_round;tx_context.cppasserts layout, field offsets and integer field types. Purely additive, nothing references the type yet.The producer migration moves into the
vm::Hostport, EXE-173. Withget_tx_contextreturning the in-tree type the host holds noevmc_tx_contextcopy, which this PR's earlier shape needed (256 bytes per host instance).Verification. gcc-15 RelWithDebInfo build clean;
clang-format-19andclang-tidy-19on the new files clean.tx_context.cppincludes<evmc/evmc.h>exactly asrevision.cppandstatus_code.cppalready do, so the zkVM guest sees nothing new.Conflicts trivially with #2540 on
category/vm/evm/CMakeLists.txt; whichever lands first, the other rebases.🤖 Generated with Claude Code