Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified, and all assessments indicate readiness.
Pull request overview
Replaces EVMC’s mocked host with an in-tree vm::Host implementation for VM tests.
Changes:
- Adds page-aware storage and exhaustive EIP-2200 status tests.
- Migrates fixtures and interface tests to the shared mock host.
- Removes the obsolete wrapper and EVMC mock linkage.
File summaries
| File | Description |
|---|---|
test/vm/utils/mocked_host.hpp |
Adds the in-tree mock host. |
test/vm/utils/CMakeLists.txt |
Registers the shared mock header. |
test/vm/unit/runtime/mocked_host.hpp |
Removes the obsolete wrapper. |
test/vm/unit/runtime/fixture.hpp |
Uses the shared mock host. |
test/vm/unit/runtime/fixture.cpp |
Updates account construction. |
test/vm/unit/monad_vm_interface_tests.cpp |
Migrates interface tests. |
test/vm/unit/mocked_host_tests.cpp |
Adds storage-status transition tests. |
test/vm/unit/evm_fixture.hpp |
Uses the shared mock host. |
test/vm/unit/CMakeLists.txt |
Adds tests and removes EVMC mock linkage. |
Review details
- Files reviewed: 9/9 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.
Contributor
There was a problem hiding this comment.
Reviewed this replacement of evmc::MockedHost with the in-tree monad::vm::test::MockedHost.
What was verified:
- The new
storage_statusimplementation matches evmc's EIP-2200 logic exactly; the newMockedHost.StorageStatustest covers all 15 (original, current, value) transition classes and passes (verified by building and running it standalone). access_account,selfdestruct,get_storage,set_storage,account_exists,get_balance,copy_code, transient storage, and block-hash recording all reproduce the vendoredevmc::MockedHostsemantics, including the EIP-2929 precompile warm range (0x01–0x09) and the record-then-check warmth behavior.- The page-based
access_storage/update_pagebehavior (compute_page_keyfromstorage_page.hpp,PAGE_KEY_SHIFT == 7) is identical to the deletedtest/vm/unit/runtime/mocked_host.hppwrapper, so the runtime fixture tests keep their previous semantics. The tests migrated off pureevmc::MockedHost(evm_fixture,monad_vm_interface_tests) don't exercise storage warmth, so no behavior-sensitive test is affected. - No remaining references to
evmc/mocked_host.hppanywhere in the tree, so dropping theevmc::mocked_hostlink fromvm-unit-testsis safe. The new header compiles standalone, and thefixture.cppdesignated initializer matches the newMockedAccountfield order.
One non-blocking note posted inline: the dangling input_data/code pointers retained in recorded_calls (documented, but worth hardening).
Verdict: CORRECT
🤖 Generated with Claude Code
khordadi
force-pushed
the
akhordadi/exe-161-replace-evmcmockedhost-with-an-in-tree-mock-host-for-vm-unit
branch
from
September 15, 2026 16:10
748244a to
fa5c51b
Compare
Closes EXE-161, part of EXE-162. The VM unit tests no longer depend on evmc::MockedHost, so the vm::Host port (EXE-173) has one test host to move instead of two, and evmc::mocked_host leaves the test link line. test/vm/utils/mocked_host.hpp replaces the test/vm/unit/runtime wrapper, which layered MIP-8 page tracking over evmc's mock. It implements vm::Host directly, including update_page, carries the page tracking over on the production compute_page_key, and keys its state on Address/bytes32_t; evmc types remain only where the vm::Host virtuals require them. It sits in monad-vm-test-utils so the fuzzer and benchmark targets can pick it up later without a new target. The EIP-2200 storage status is the one piece of logic rewritten rather than moved. mocked_host_tests.cpp checks the fifteen original -> current -> new transitions listed on evmc_storage_status; they partition the three values by zero-ness and pairwise equality, so the table is exhaustive. The runtime storage tests reach only the six clean-slot cases, and the spec comparison drives both VMs through the same mock, so neither would notice a wrong dirty-slot status. No test-observable behaviour change. evm_fixture.hpp and monad_vm_interface_tests.cpp used evmc::MockedHost directly and now get the wrapper's semantics: access_storage warmth per page rather than per slot, and update_page from the page set rather than from the storage status; the only SSTORE either reaches runs under execute_and_compare, pinned to MONAD_EIGHT where MIP-8 is inactive. Dropped because no test uses them: the 100/200 caps on recorded calls and account accesses, the copy of call input bytes behind recorded_calls (its pointer fields are nulled on record), MockedAccount::nonce, the per-slot access flag the wrapper never returned, access_storage's side effect of creating the account and slot (account_exists after SLOAD on an absent account is now false), the second recorded_account_accesses entry per SSTORE from the wrapper's extra get_storage, and evmc's always-warm rule for 0x01..0x09 in access_account (production decides that per fork in EvmcHost; tests that need a warm address pre-warm it). vm-unit-tests: gcc 1292 passed / 34 skipped (main 1291 / 34); clang ASAN+UBSAN with the spec comparison 1786 / 196 (main 1785 / 196). The difference is the new test. monad-compiler-fuzzer, execution-benchmarks, vm-micro-benchmarks and mce build. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
khordadi
force-pushed
the
akhordadi/exe-161-replace-evmcmockedhost-with-an-in-tree-mock-host-for-vm-unit
branch
from
September 15, 2026 18:07
fa5c51b to
3fc0faa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes EXE-161, part of EXE-162. The VM unit tests no longer depend on
evmc::MockedHost, so thevm::Hostport (EXE-173) has one test host to move instead of two, andevmc::mocked_hostleaves the test link line.What changes.
test/vm/utils/mocked_host.hppreplaces thetest/vm/unit/runtimewrapper, which layered MIP-8 page tracking over evmc's mock. It implementsvm::Hostdirectly, includingupdate_page, carries the page tracking over on the productioncompute_page_key, and keys its state onAddress/bytes32_t; evmc types remain only where thevm::Hostvirtuals require them. It sits inmonad-vm-test-utilsso the fuzzer and benchmark targets can pick it up later without a new target.Storage status. The EIP-2200 status is the one piece of logic rewritten rather than moved.
mocked_host_tests.cppchecks the fifteenoriginal -> current -> newtransitions listed onevmc_storage_status; they partition the three values by zero-ness and pairwise equality, so the table is exhaustive. The runtime storage tests reach only the six clean-slot cases, and the spec comparison drives both VMs through the same mock, so neither would notice a wrong dirty-slot status.Behaviour. No test-observable change.
evm_fixture.hppandmonad_vm_interface_tests.cppusedevmc::MockedHostdirectly and now get the wrapper's semantics:access_storagewarmth per page rather than per slot, andupdate_pagefrom the page set rather than from the storage status. The only SSTORE either reaches runs underexecute_and_compare, pinned to MONAD_EIGHT where MIP-8 is inactive.Dropped, nothing uses them:
recorded_calls(its pointer fields are nulled on record)MockedAccount::nonceaccess_storage's side effect of creating the account and slot (account_existsafter SLOAD on an absent account is now false)recorded_account_accessesentry per SSTORE from the wrapper's extraget_storage0x01..0x09inaccess_account(production decides that per fork inEvmcHost; tests that need a warm address pre-warm it)Verification.
vm-unit-testsThe difference is the new test.
monad-compiler-fuzzer,execution-benchmarks,vm-micro-benchmarksandmcebuild.clang-format-19andclang-tidy-19clean on the new files.Review map:
test/vm/utils/mocked_host.hpp(storage_status, then the virtuals) →test/vm/unit/mocked_host_tests.cpp→ the five include/type swaps → the deleted wrapper.🤖 Generated with Claude Code