fix(revm): align BLOCKHASH with Morph geth semantics#38
Conversation
Override BLOCKHASH to return keccak(chain_id||block_number) within the 256-block window, matching Morph geth and removing the gas divergence at block 2662438.
📝 WalkthroughWalkthroughImplements Morph-specific BLOCKHASH opcode behavior in the EVM, including helper functions to compute keccak256-hashed block references and enforce the 256-block history window, with corresponding test coverage and instruction registration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/revm/src/evm.rs (1)
285-289: Strengthen the lower-bound test to assert exact value, not just non-zero.Line 286 currently only proves “some non-zero value” was returned. Asserting equality against
morph_blockhash_value(chain_id, current - 256)will catch hashing/input regressions too.Suggested test tightening
- // lower bound is inclusive (current - 256 is valid) - assert_ne!( - morph_blockhash_result(chain_id, current, current - 256), - U256::ZERO - ); + // lower bound is inclusive (current - 256 is valid) + assert_eq!( + morph_blockhash_result(chain_id, current, current - 256), + morph_blockhash_value(chain_id, current - 256) + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/revm/src/evm.rs` around lines 285 - 289, The test currently only asserts the blockhash is non-zero using morph_blockhash_result(chain_id, current, current - 256); replace that non-zero check with an exact equality assertion against the expected value by calling morph_blockhash_value(chain_id, current - 256) and asserting morph_blockhash_result(chain_id, current, current - 256) == morph_blockhash_value(chain_id, current - 256) so the test fails on any hashing/input regression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/revm/src/evm.rs`:
- Around line 285-289: The test currently only asserts the blockhash is non-zero
using morph_blockhash_result(chain_id, current, current - 256); replace that
non-zero check with an exact equality assertion against the expected value by
calling morph_blockhash_value(chain_id, current - 256) and asserting
morph_blockhash_result(chain_id, current, current - 256) ==
morph_blockhash_value(chain_id, current - 256) so the test fails on any
hashing/input regression.
Summary
BLOCKHASHopcode inmorph-revmwith Morph geth semantics: returnkeccak(chain_id || block_number)in the last-256-block window0for current/future/out-of-window requests)chain_id=2818,number=2662437) and 256-window boundary behaviorWhy
2662438failed import withblock gas used mismatch(got 823153,expected 825454)BLOCKHASH, which changed downstream execution path and gas accountingTest plan
cargo check -p morph-revmcargo test -p morph-revm morph_blockhashCloses #37