[rpc] eth_simulateV1

[Baltoli]

Depends on:

• Implement block overrides for eth_simulatev1 #2057
• Native transfer tracer for eth_simulateV1 #1926

(commits should be rebased out when those PRs are merged)

[github-actions]

Claude finished @Baltoli's task in 6m 44s —— View job

---

Final re-review of eth_simulateV1 PR

Re-reviewed the latest tip (d454ec1) against the prior review feedback. All seven previously-flagged issues are addressed; I'm flagging four smaller follow-ups below.

Status of prior review items

#	Severity	Issue	Status
1	P1	HiveNet null-deref via dynamic_cast<MonadChain*>	✅ Fixed (93a1144) — monad_executor.cpp:1966-1967 now routes CHAIN_CONFIG_HIVE_NET through the EVM branch and adds the HiveNet case to the chain factory at line 1957-1958.
2	P1	Validator's gap > 1 else branch advanced timestamp by 1 * increment instead of gap * increment	✅ Fixed (30905ca) — monad_executor.cpp:564 now uses gap * default_timestamp_increment, matching eth_simulate_impl's synthetic-block progression.
3	P1	Inner senders[i].size() == txns[i].size() not validated → potential OOB	✅ Fixed (30905ca) — eth_simulate_impl now asserts inner-dimension equality at monad_executor.cpp:670-673.
4	P2	Rust block_overrides leaked on recv.await Err branch	✅ Fixed (cba34af) — lib.rs:815-819 now destroys block_overrides symmetrically with state_overrides.
5	P2	LOG_INFO("res: ...") debug leftover	⚠️ Commented out (f7ab929), not deleted. The commit message says "to be removed entirely later" — please just remove it before merge; carrying commented-out logs is exactly the rot the original comment was warning about.
6	P3	Withdrawals ternary collapsed to one branch	✅ Fixed (0c7ba20) — monad_executor.cpp:828 is now just block_overrides[block_idx]->withdrawals.
7	P3	Dead (void)emit_native_transfer_logs;	✅ Fixed (a2c12f9).

New observations

1. [P2] User block following a synthetic gap inherits gas_limit = 0. At monad_executor.cpp:709-713, synthetic blocks deliberately don't inherit the previous header (just number and timestamp), so synthetic_block.header.gas_limit is zero. After the synthetic loop, previous_header = synthetic_block.header (line 748). The next user block's gas_limit then defaults to that zero at line 767: bo->gas_limit.value_or(previous_header.gas_limit). The Monad-only fix-up at line 855 only triggers when the original input header.gas_limit == 0, so it doesn't catch this case, and there's no fix-up at all for EVM traits. The existing eth_simulate_v1_block_override_synthetic_gap test sidesteps this by always overriding gas_limit on the user block (monad_executor_test.cpp:4646). Suggest either (a) having synthetic blocks inherit previous_header.gas_limit (the comment at line 764-766 already promises "irrespective of whether it is a real block, a synthetic block, or a user-defined block"), or (b) generalizing the line 855 fix-up to handle the gap case. Add a regression test where the first user block follows a synthetic gap and doesn't override gas_limit. Fix this →

2. [P3] n_block_overrides is not validated against txns.size() at the C entry point. monad_executor.cpp:2266-2267 checks senders.size() == txns.size() and n_state_overrides == txns.size(), but skips the equivalent assertion for n_block_overrides. The mismatch will eventually be caught by MONAD_ASSERT(calls.size() == block_overrides.size()) inside eth_simulate_impl at line 668, so this is just a missing symmetric guard at the boundary. Fix this →

3. [P3] Mutually-exclusive state-override fields aren't enforced. In apply_state_overrides (monad_executor.cpp:300-311), if a caller sets both state_diff and state for the same address, the state_diff writes happen first, then set_to_state_incarnation(address) clears them, then state is applied — silently dropping the state_diff overrides. Per the geth spec, state and stateDiff are mutually exclusive; either reject both being set in the C API, or assert in apply_state_overrides. Today the Rust side enforces this via the StorageOverride enum, but the C API is reachable from anything else.

4. [Nit] monad_executor.cpp:546-548 — the comment "I am not sure how to interpret the 'or remain constant' part" reads as an unresolved question in production code. The <= chosen here matches the spec; consider deleting the comment or replacing it with the rationale ("we follow the spec literally rather than geth's stricter implementation").

Other notes

• The lifetime fix in eth_simulate_v1_stress_queue_rejection (d454ec1) is correct — taking &subs[i]->so after emplace_back is safe because submission is heap-allocated through unique_ptr, so vector reallocation doesn't invalidate the inner pointers.
• The apply_state_overrides pattern of merging into block_state per user block means overrides persist across subsequent blocks in the simulation. That matches what most callers want, but note it's a semantic choice worth a one-line comment if it isn't documented elsewhere.
• The MONAD_ASSERT(false) after each SWITCH_*_TRAITS macro at lines 1986/2009 is correct given the macros' return f<...>(...) expansion in every case label.
• ChainContextBuffer<traits> is well-structured: the EVM specialization is empty, the Monad specialization manages the K=3 circular buffer, and advance() correctly returns a ChainContext whose backing arrays outlive the call (held by the buffer + the function-local senders/authorities vectors).

Verdict

The P1s and the leak P2 are properly resolved. None of the new observations are blocking. I'd push for #1 (synthetic-gap gas_limit) and removing rather than commenting out the LOG_INFO before merge, but the rest can be follow-ups.
· branch bruce/simulate-2

[Baltoli]

From Codex 5.5.

Codex Adversarial Review — PR #2060

Findings

🔴 [critical] Simulated block hashes are not chained into later blocks

Location: category/rpc/monad_executor.cpp:759-775

Each simulated header is built without a parent_hash, and the same LazyBlockHash anchored at the original block_number is reused for every simulated block. A later simulated transaction that calls BLOCKHASH for an earlier simulated block can fall through to LazyBlockHash::get for a block number newer than its anchor, hitting an assertion. Even when it does not crash, parentHash and block-hash history for simulated blocks is wrong.

Recommendation: Maintain a per-simulation block-hash chain — compute each simulated block hash, set it as the next header.parent_hash, and serve simulated hashes to BLOCKHASH / EIP-2935 before falling back to the DB. Add a multi-block BLOCKHASH test.

---

🟠 [high] First simulated blocks ignore real parent reserve context

Location: category/rpc/monad_executor.cpp:687-689

eth_simulate starts ChainContextBuffer empty, so the first simulated block sees empty parent and grandparent sender / authority sets. Monad reserve checks explicitly reject reserve dipping when the sender appeared in the parent or grandparent; this path can therefore report success for a simulation that production block execution would reject.

Recommendation: Seed the context buffer from the real base block and its parent before simulating, mirroring the trace path's DB recovery. If the needed block ids are unavailable, extend the API to pass them or reject the simulation instead of using empty context.

---

🟡 [medium] baseFeePerGas is emitted as decimal with a hex prefix

Location: category/rpc/monad_executor.cpp:472-473

store_output_header formats baseFeePerGas with intx::to_string(value) but prefixes it with 0x. intx defaults to decimal, so a base fee like 1000000000 is returned as 0x1000000000, which clients parse as a much larger hex value.

Recommendation: Format uint256 JSON quantities with base 16, e.g. intx::to_string(value, 16), and add coverage for base fees above 9.

---

🟡 [medium] Log indexes are duplicated within a transaction

Location: category/rpc/monad_executor.cpp:607-620

The log serialization loop uses tx_idx as logIndex for every log in that transaction. Any transaction that emits multiple logs — including the new native-transfer logging path — returns duplicate logIndex values and non-cumulative indexes across the block, breaking clients that identify logs by blockHash / logIndex.

Recommendation: Track a cumulative block log index and increment it for every emitted log. Add a test with multiple logs from one transaction and logs across multiple transactions.

---

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dhil]

I don't think this suggestion is correct. The templated invocation in monad_executor_eth_simulate_submit is parametrized by rlp::decode_transaction which internally dispatches to either rlp::decode_transaction_legacy or rlp::decode_transaction_eip2718. I've added a test that submits legacy transactions and checks they execute successfully.

[dhil]

@claude Please do another detailed re-review of this PR, taking into account unresolved and resolved comments from previous reviews and responses. Please consider the PR commit by commit; we want each commit be sound and coherent.

[github-actions]

Claude finished @dhil's task in 7m 44s —— View job

---

Re-review of eth_simulateV1 PR — commit by commit

Re-reviewed at d954180 against prior review rounds. Each commit builds and is internally coherent. The Codex critical (BLOCKHASH chaining) and high (parent reserve context) are both properly fixed via the new EthSimulateBlockHashBuffer and the DB-loaded grandparent/parent context. Three older issues remain unresolved.

Per-commit assessment

#	Commit	Verdict
1	8ec3efc overrides FFI vectors	✅ Self-contained. Removes blob_base_fee from monad_block_override (resolves Copilot finding). Adds withdrawals field with corresponding add_block_override_withdrawal[_at] setters.
2	e363370 ChainContextBuffer	✅ Clean K=3 circular buffer. Empty EVM specialization. EXPLICIT_*_TRAITS_CLASS instantiations are needed only for this commit to link in isolation; comment correctly flags they are temporary.
3	08283b9 EthSimulateBlockHashBuffer	✅ Solves Codex critical: simulated block hashes are now appended via advance() and served back to BLOCKHASH via get(). The visibility flip of LazyBlockHash::N from private → public is the only collateral; reasonable since it's a derived class need.
4	afbafe5 C API	✅ Pure plumbing. Endpoint asserts false until commit 5 wires it up — fine, the commit message is explicit about that.
5	f01e282 implementation	✅ Removes the explicit instantiations from commit 2, adds eth_simulate_impl, store_output_header, eth_simulate_validate_inputs, save_eth_simulate_log_entry, and submit_eth_simulate_to_pool. Loads grandparent + parent context from DB (resolves Codex high), routes HiveNet through EVM branch (resolves prior P1). MONAD_ASSERT(false) after each SWITCH_*_TRAITS is correct (every case label returns).
6	842f860 enable trace transfers	✅ Threads bool trace_transfers through dispatch_transaction → ExecuteTransaction → EvmcHost. Minor inconsistency: ethereum/dispatch_transaction.hpp adds = false default, but monad/dispatch_transaction.hpp does not. Both call sites in execute_block_transactions pass it explicitly, so harmless, but worth aligning the signatures.
7	787b708 tests	✅ 16 GTest cases, including dedicated eth_simulate_v1_blockhash_reads (asserts BLOCKHASH chaining across user + synthetic blocks at block #511), eth_simulate_v1_reserve_balance_chain_context_buffer, eth_simulate_v1_block_override_synthetic_gap, eth_simulate_v1_legacy_transactions.
8	d954180 Rust interop	✅ Symmetric destroy of both state_overrides and block_overrides on recv.await Err. Validates inner senders length up front.

Status of all flagged comments

Source	Issue	Status
Prior P1	HiveNet null-deref via dynamic_cast<MonadChain*>	✅ Fixed (monad_executor.cpp:2082-2083)
Prior P1	Validator gap > 1 timestamp progression	✅ Fixed; also corrected per Copilot to (gap - 1) * default_timestamp_increment (monad_executor.cpp:799)
Prior P1	Inner senders[i].size() == txns[i].size() not validated	✅ Fixed (monad_executor.cpp:917-920 + Rust-side at lib.rs:678-680)
Prior P2	Rust block_overrides leaked on Err	✅ Fixed (lib.rs:877-883)
Prior P2	LOG_INFO("res: ...") debug leftover	⚠️ Still commented out, not removed (monad_executor.cpp:1172)
Prior P3	Withdrawals collapsing ternary	✅ Fixed (monad_executor.cpp:1123)
Prior P3	Dead (void)emit_native_transfer_logs;	✅ Fixed in commit 6
Prior P3	Mutually-exclusive state-override fields not enforced	❌ Still present (monad_executor.cpp:230-241)
Prior P2	Synthetic-gap user-block gas_limit = 0	✅ Fixed — synthetic blocks now inherit previous_header.gas_limit (monad_executor.cpp:984)
Prior P3	n_block_overrides not validated at C entry	✅ Fixed (monad_executor.cpp:2397)
Codex 🔴	Simulated block hashes not chained	✅ Fixed via EthSimulateBlockHashBuffer + eth_simulate_v1_blockhash_reads test
Codex 🟠	First simulated blocks ignore real parent reserve context	✅ Fixed (monad_executor.cpp:937-960)
Codex 🟡	baseFeePerGas decimal-with-hex-prefix	✅ Fixed — now intx::to_string(value, 16) (monad_executor.cpp:781)
Codex 🟡	Log indexes duplicated within a block	⚠️ Partially fixed. log_index resets to 0 per tx (monad_executor.cpp:851); per geth's reference, Index should be cumulative across the block. With one tx per block all tests pass; multi-tx blocks would emit 0,1,0,1 instead of 0,1,2,3.
Copilot	gap > 1 over-advance by one block	✅ Fixed
Copilot	"strictly increasing" message wording	✅ Fixed — now "monotonically increasing" (monad_executor.cpp:804)
Copilot	Rust input length validation	✅ Fixed
Copilot	RLP encoding mismatch for legacy txs Rust→C++	⚠️ Mitigated, not eliminated. A C++-side legacy test was added (eth_simulate_v1_legacy_transactions at monad_executor_test.cpp:6632) which wraps each tx in rlp::encode_string2(...). The Rust side uses calls.encode(...) from alloy's Encodable for TxEnvelope, which emits typed txs as RLP strings (type byte + payload) and legacy txs as RLP lists. Since the C++ decode_nested_items<rlp::decode_transaction, true> calls parse_string_metadata first, a legacy tx encoded as a list by alloy would fail to decode. There is no Rust → C++ round-trip test for legacy txs. Recommend adding one (or asserting in Rust that the encoding is wrapped) to avoid a latent ABI mismatch.
Copilot	MONAD_ASSERT for required pointers in submit	✅ Fixed (monad_executor.cpp:2374-2381)
Copilot	blob_base_fee dead field	✅ Removed from overrides.hpp

Remaining issues worth fixing before merge

1. [P2] Block-cumulative logIndex. monad_executor.cpp:851 resets log_index = 0 per transaction. Geth's simulate.go sets log.Index = uint(i) over the flattened block log list. A block where two transactions each emit logs will currently report duplicate indexes. Suggest hoisting log_index to the block scope (i.e., outside the for tx_idx loop in save_eth_simulate_log_entry). Add a multi-tx-with-logs regression test. Fix this →

2. [P2] Mutually-exclusive state/stateDiff not enforced. apply_state_overrides at monad_executor.cpp:230-241 runs state_diff first, then set_to_state_incarnation(address) (which wipes everything just written), then state. So setting both for the same address silently drops the state_diff writes. Geth rejects this combination. The Rust side enforces it via the StorageOverride enum, but the C API is reachable from elsewhere. Either swap the order so state_diff becomes additive on top of state, or assert that they're not both populated for the same address. Fix this →

3. [Nit] LOG_INFO("res: ...") left commented out at monad_executor.cpp:1172. The earlier review and dhil's own commit message both said this would be removed; carrying commented-out logs in production code is the rot the original comment was warning about. Just delete the line.

4. [Nit] RLP-string vs RLP-list mismatch for legacy txs Rust→C++. Either add a test that exercises the full Rust-encode → C-decode path with a legacy tx, or wrap each TxEnvelope in an explicit RLP-string layer in eth_simulate_v1 (Rust). Today there is no automated check that what Rust sends, C++ accepts, for legacy types.

5. [Nit] monad/dispatch_transaction.hpp declaration of trace_transfers lacks the = false default that the ethereum side gained. Either add the default for symmetry or document the asymmetry. (category/execution/monad/dispatch_transaction.hpp:32)

Verdict

The PR is in good shape. The critical and high-severity findings from prior rounds are properly resolved with regression tests. Remaining items are P2/Nit and individually small. I'd push for #1 (block-cumulative logIndex) and #2 (state/stateDiff mutual exclusion) before merge, plus the trivial cleanup of #3. Items #4 and #5 can be follow-ups.
· branch bruce/simulate-2

[dhil]

P3 Mutually-exclusive state-override fields not enforced. Wont fix.

[dhil]

@claude please review this PR again. This time focus on identifying opportunities for data races, deadlocks, livelocks, or other parallel and concurrency-related bugs introduced by this PR.