Conversation
This patch hardens `eth_simulateV1` to gracefully exit in the event of a block merge failure in `execute_block_header`.
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved issues were identified, and regression coverage is included.
Pull request overview
Hardens eth_simulateV1 by gracefully handling block-header merge failures.
Changes:
- Converts merge assertions into catchable errors.
- Adds regression coverage for conflicting state overrides.
File summaries
| File | Summary |
|---|---|
category/rpc/monad_executor_test.cpp |
Tests graceful error reporting. |
category/execution/ethereum/execute_block_header.cpp |
Makes merge failures catchable. |
Review details
- Files reviewed: 2/2 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}
| MONAD_ASSERT_THROW( | ||
| block_state.can_merge(state), "block state cannot be merged"); |
There was a problem hiding this comment.
[P2] This hardens the block-prologue merge, but the same user-controlled trigger still aborts the process via two sibling asserts on the eth_simulateV1 path: MONAD_ASSERT(block_state.can_merge(state)) in execute_block.cpp:344 (epilogue merge) and execute_transaction.cpp:490 (post-retry transaction merge). The failure mechanism your test exercises — create_contract resets storage, so the State's recorded original diverges from a storage override already sitting in block_state — is reproducible per-transaction: a simulated call that CREATEs at an address carrying a state_diff override fails can_merge at execute_transaction.cpp:470, and the retry deterministically re-derives the same mismatch and trips the hard assert at line 490. Consider converting those two sites to MONAD_ASSERT_THROW as well (they're inside the try/catch that maps MonadException to EVMC_INTERNAL_ERROR), or noting why they're unreachable from the simulate path.
There was a problem hiding this comment.
I think the execute_block.cpp:344 is theoretically triggerable via an Ethereum Prague request by overriding an EIP-7002/7251 deploy with code that recreates an overridden account and touches its storage. However, simulated headers leave requests_hash unset, and block overrides cannot set it. Execution will therefore fail at the earlier requests_hash.has_value() check. Nonetheless, this configuration is currently unsupported.
I will think a bit about execute_transaction.cpp:470.
This patch hardens
eth_simulateV1to gracefully exit in the event of a block merge failure inexecute_block_header.