Conversation
khordadi
left a comment
There was a problem hiding this comment.
Notes for the reviewer on the non-obvious lines.
| }; | ||
|
|
||
| // Mirrors evmc_flags 1:1; values asserted equal in message.cpp. | ||
| enum monad_call_flags |
There was a problem hiding this comment.
Not in the issue title, but without it every reader of flags still needs evmc.h.
| monad::Address code_address; | ||
| uint8_t *memory_handle; | ||
| uint8_t *memory; | ||
| uint32_t memory_capacity; |
There was a problem hiding this comment.
Fork layout: evmc 663a1c2 dropped code/code_size, 8533622 added the memory-pool fields. A mirror of upstream evmc_message fails the asserts in message.cpp.
| static_assert(sizeof(monad_message) == sizeof(evmc_message)); | ||
| static_assert(alignof(monad_message) == alignof(evmc_message)); | ||
|
|
||
| #define MONAD_ASSERT_MESSAGE_FIELD_EQ(field) \ |
There was a problem hiding this comment.
Offsets only: sizeof on the pointer members trips bugprone-sizeof-expression. The type asserts below cover the narrowed-field case that offsets alone would miss.
There was a problem hiding this comment.
🔵 Needs a closer look
It changes core VM/execution message plumbing across many call sites and relies on layout-dependent bit_cast adapters, warranting final human verification.
Pull request overview
This PR continues the execution/vm “evmc leaf types” migration by introducing an in-tree monad_message (plus monad_call_kind / monad_call_flags) and switching the execution layer + VM entry points to use it, keeping EVMC types only at the ABI seams via std::bit_cast adapters with compile-time layout verification.
Changes:
- Add
category/vm/evm/message.{hpp,cpp}definingmonad_messageandto_evmc_message/from_evmc_message, with static assertions for enum values and struct layout. - Replace
evmc_messagewithmonad_messagein VM APIs (VM::execute,VM::execute_bytecode) and in execution-layer message plumbing (precompiles, execute_{message,transaction}, tracing). - Update tests, fuzzing, and benchmarking tooling to construct and pass
monad_message, bridging back to EVMC only where required.
Verdict: CORRECT
Generated with Claude Code
File summaries
| File | Description |
|---|---|
category/vm/evm/message.hpp |
Introduces monad_message + call kind/flags mirrors and conversion function declarations. |
category/vm/evm/message.cpp |
Proves enum/value + layout equivalence vs evmc_message and implements bit_cast conversions. |
category/vm/evm/CMakeLists.txt |
Adds the new message mirror sources to the EVM VM target. |
category/vm/vm.hpp |
Updates VM public template entrypoints to accept monad_message. |
category/vm/vm.cpp |
Adapts execute-override path to convert monad_message to evmc_message for the EVMC ABI hook. |
category/vm/runtime/types.hpp |
Changes runtime::Context::from to accept monad_message. |
category/vm/runtime/context.cpp |
Wires monad_message fields into runtime context construction. |
category/vm/fuzzing/generator/generator.hpp |
Switches fuzz-message generation and flags/kind selection to monad_message types. |
category/execution/ethereum/execute_transaction.hpp |
Updates transaction→message construction to return monad_message. |
category/execution/ethereum/execute_transaction.cpp |
Replaces message kinds/flags and value encoding to populate monad_message. |
category/execution/ethereum/execute_message.hpp |
Updates execute_{call,create}_message signatures to take monad_message. |
category/execution/ethereum/execute_message.cpp |
Ports runtime message handling logic from evmc_message to monad_message. |
category/execution/ethereum/precompiles.hpp |
Ports precompile checks to monad_message. |
category/execution/ethereum/precompiles.cpp |
Updates flag checks (delegated/static) and signatures to monad_message. |
category/execution/ethereum/evmc_host.hpp |
Adds call(monad_message const&) and makes the EVMC vtable slot an adapter. |
category/execution/ethereum/trace/call_tracer.hpp |
Updates tracer interface on_enter to accept monad_message. |
category/execution/ethereum/trace/call_tracer.cpp |
Ports call-kind handling to MONAD_* enumerators. |
category/execution/ethereum/trace/call_frame.cpp |
Ports static-call flag checks to MONAD_STATIC. |
category/execution/ethereum/process_requests.cpp |
Constructs system-call messages as monad_message. |
category/execution/ethereum/block_hash_history_test.cpp |
Updates test message construction to monad_message. |
category/execution/ethereum/trace/state_tracer_test.cpp |
Updates message construction to monad_message for tracing tests. |
category/execution/ethereum/test/test_call_trace.cpp |
Updates trace test message type to monad_message. |
category/execution/ethereum/precompiles_test.cpp |
Updates precompile tests to use monad_message. |
category/execution/ethereum/execute_message_test.cpp |
Updates execution tests to use monad_message and bytes32_t values directly. |
category/execution/monad/monad_precompiles.hpp |
Ports Monad precompile entrypoint to accept monad_message. |
category/execution/monad/monad_precompiles.cpp |
Ports kind checks to MONAD_CALL and signature to monad_message. |
category/execution/monad/execute_system_transaction.hpp |
Changes system-tx to_message() to return monad_message. |
category/execution/monad/execute_system_transaction.cpp |
Constructs monad_message for system tx execution and encodes value into bytes32_t. |
category/execution/monad/reserve_balance/reserve_balance_contract_test.cpp |
Updates reserve-balance tests and helper signatures to monad_message. |
cmd/vm/mce/src/instrumentable_vm.hpp |
Switches microbenchmark message allocation and kinds to monad_message. |
test/vm/utils/test_message.hpp |
Updates TestMessage helper to store a monad_message. |
test/vm/vm/test_vm.cpp |
Adapts EVMC ABI BlockchainTestVM::execute to convert EVMC→Monad message before Context::from. |
test/vm/unit/runtime/memory_tests.cpp |
Uses monad_message in runtime memory unit tests. |
test/vm/unit/monad_vm_interface_tests.cpp |
Updates host mock plumbing and test lambdas to accept monad_message. |
test/vm/unit/evm_fixture.hpp |
Keeps EVMC ABI boundary by converting monad_message back to evmc_message for evmone. |
test/vm/fuzzer/fuzzer.cpp |
Ports fuzz transition helpers to accept monad_message. |
Review details
- Files reviewed: 36/36 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.
There was a problem hiding this comment.
Reviewed per REVIEW.md: traits usage, FFI/ABI boundaries, C++ correctness (lifetimes, bit_cast legality, overload/name-hiding, temporary binding at every conversion seam), testing, style, and hygiene.
What I verified:
- Layout proof is sound.
message.cpppins size (I independently recomputed the 192-byte layout: 4+4+4+pad+8+20+20+8+8+32+32+20+pad+8+8+4+pad = 192), alignment, all 14 field offsets, exact types of the 8 integer/pointer fields, and the enumerator values. Combined withis_standard_layout/is_trivially_copyable, this makes the twostd::bit_casts well-defined; a mirror written from upstream evmc (or a swapped enumerator) fails to compile, as the PR description demonstrates. - No overload/dispatch hazards.
EvmcHostisfinal, so the non-virtual in-treecall(monad_message const &)alongside the virtual evmc adapter cannot be bypassed by a derived override, and the adapter'sfrom_evmc_messageargument selects the monad overload with no recursion risk. - Temporary lifetimes are correct at every seam:
to_evmc_messagelocals inVM::execute/execute_bytecodeoutlive the synchronous override call; the temporaries bound to const-ref parameters inevm_fixture.hpp,monad_vm_interface_tests.cpp, andtest_vm.cpplive through the full expression / enclosing scope of their synchronous use. - Remaining
evmc_messagesites are the intended ABI boundaries (runtime/call.cpp/create.cppfeeding the evmc host table,evmc_vmcallbacks, benchmark harnesses,ExecuteOverride), matching the EXE-173/EXE-161/EXE-174 scoping in the description. - Traits/instantiation rules respected: no macro changes, no new
is_monad_trait_vbranching inethereum/, andcheck-trait-instantiationspasses. - FFI:
monad_messageis not exposed to the Rust bindgen headers; conversions happen before crossing the evmone C ABI.
Testing via the compile-time assertion suite is appropriate for a pure layout mirror. CI builds/tests were still pending at review time; the PR's own verification record (full ctest, ASAN+UBSAN with compiler testing, fuzzer, spec tests) is thorough.
One non-blocking nit posted inline (P3: noexcept on the conversion functions).
Verdict: CORRECT
🤖 Generated with Claude Code
9c4b807 to
1184605
Compare
EXE-172, part of EXE-162. monad_message, with monad_call_kind and monad_call_flags, mirrors the fork's evmc_message field-for-field (no code/code_size, evmc 663a1c2; memory_handle/memory/memory_capacity, evmc 8533622). message.cpp asserts standard layout, sizeof, alignof, the offsetof of all 14 fields, the exact type of the 8 integer/pointer fields and the enumerator values. Purely additive; nothing references the type yet. The call-site migration goes with the vm::Host port (EXE-173), where the runtime builds the message itself and no per-call evmc conversion is needed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1184605 to
8fe6bf2
Compare
evmc_message and evmc_call_kind to in-tree Monad typesmonad_message type
Part of EXE-172 (EXE-162), reduced to the type.
monad_message, withmonad_call_kindandmonad_call_flags, mirrors the fork'sevmc_message;message.cppasserts layout, field offsets, field types and enumerator values. Purely additive, nothing references the type yet.The call-site migration (formerly commit 2 here) moves into the
vm::Hostport, EXE-173. With the vtable slot and the runtime onmonad_messagethere is no per-callevmc_messageconversion, which the adapter inEvmcHost::callneeded (one out-of-line call and a 192-byte copy per nested call).Verification. gcc-15 RelWithDebInfo build clean;
clang-format-19andclang-tidy-19on the new files clean.message.cppincludes<evmc/evmc.h>exactly asrevision.cppandstatus_code.cppalready do, so the zkVM guest sees nothing new.Conflicts trivially with #2539 on
category/vm/evm/CMakeLists.txt; whichever lands first, the other rebases.🤖 Generated with Claude Code