refactor: rename MPTFork hardfork to Jade#33
Conversation
📝 WalkthroughWalkthroughThis PR systematically renames the MPTFork hardfork to Jade throughout the codebase. The refactoring updates struct field names (mpt_fork_time → jade_time), enum variants (MorphHardfork::MPTFork → MorphHardfork::Jade), method names (is_mpt_fork → is_jade), JSON serialization keys, and associated documentation and tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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). Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/chainspec/src/spec.rs (1)
568-636: Consider adding test coverage for Jade hardfork activation.The existing
test_morph_hardfork_attest covers hardforks up to Emerald but doesn't include Jade. Since Jade is the new latest hardfork, consider adding a test case to verifymorph_hardfork_atreturnsMorphHardfork::Jadewhen Jade is active.💡 Suggested test extension
Add
"jadeTime": 6000to the genesis JSON and include:// At Jade time (block 600, timestamp 6000) assert_eq!( chainspec.morph_hardfork_at(600, 6000), MorphHardfork::Jade ); // After Jade (block 700, timestamp 7000) assert_eq!( chainspec.morph_hardfork_at(700, 7000), MorphHardfork::Jade );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/chainspec/src/spec.rs` around lines 568 - 636, The test test_morph_hardfork_at misses coverage for the new Jade hardfork; update the genesis JSON used to create the MorphChainSpec (via MorphChainSpec::from(genesis)) by adding "jadeTime": 6000 in the config and extend the assertions calling chainspec.morph_hardfork_at(...) to check that morph_hardfork_at(600, 6000) returns MorphHardfork::Jade and that subsequent calls (e.g., morph_hardfork_at(700, 7000)) also return MorphHardfork::Jade so Jade activation is validated.
🤖 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/chainspec/src/spec.rs`:
- Around line 568-636: The test test_morph_hardfork_at misses coverage for the
new Jade hardfork; update the genesis JSON used to create the MorphChainSpec
(via MorphChainSpec::from(genesis)) by adding "jadeTime": 6000 in the config and
extend the assertions calling chainspec.morph_hardfork_at(...) to check that
morph_hardfork_at(600, 6000) returns MorphHardfork::Jade and that subsequent
calls (e.g., morph_hardfork_at(700, 7000)) also return MorphHardfork::Jade so
Jade activation is validated.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
crates/chainspec/src/genesis.rscrates/chainspec/src/hardfork.rscrates/chainspec/src/spec.rscrates/engine-api/src/builder.rscrates/engine-api/src/lib.rscrates/engine-api/src/validator.rs
Co-authored-by: chengwenxi <22697326+chengwenxi@users.noreply.github.com>
Co-authored-by: chengwenxi <22697326+chengwenxi@users.noreply.github.com>
…#35) * refactor: add receipt buulder in block executor * refactor: add ethereum primitives features for evm crate tests * refactor: add morph block executor factory * docs: add some comments * feat: add morph-reth bin and node wire * fix: fixed eth_call l1 fee rpl encode * fix: remove redundant ReceiptBuilder impl * test * test: add local test script * refactor(engine-api): reuse reth engine pipeline for custom L2 api * refactor: add legacy state root flag for block sync * docs: add CLAUDE.md project instructions for AI-assisted development * chore: switch reth deps to morph-l2 fork and update docs * refactor(local-test): migrate from nohup to pm2 for process management Replace nohup-based process management with pm2 to prevent processes from being killed when user sessions timeout. Also set file log level to info instead of debug. * fix(node): avoid DB read in withdraw trie root validation to reduce lock contention * fix(revm): add disabled precompile stubs for ripemd160/blake2f in Bernoulli In go-ethereum's PrecompiledContractsBernoulli, addresses 0x03 (ripemd160) and 0x09 (blake2f) are kept as disabled stubs. This means their addresses are warmed in the access list (100 gas CALL) and any call to them consumes all sub-call gas (error → gas=0 in go-ethereum). morph-reth's bernoulli() was completely removing 0x03 and 0x09, causing: - Cold access cost (2600 gas) instead of warm (100 gas) - Empty account behavior returning all sub-call gas to parent instead of consuming it This produced a 2301 gas undercount per disabled-precompile call, directly causing the "block gas used mismatch: got 823153, expected 825454" error on mainnet block 2662438. Fix: add PrecompileError stubs for 0x03 and 0x09 in bernoulli() so their addresses are warmed and calls consume all sub-call gas, matching go-ethereum behavior. * Revert "fix(revm): add disabled precompile stubs for ripemd160/blake2f in Bernoulli" This reverts commit 2bb0743. * fix(revm): add disabled precompile stubs for ripemd160/blake2f in Bernoulli go-ethereum's PrecompiledContractsBernoulli includes disabled stubs for 0x03 (ripemd160) and 0x09 (blake2f). This causes two gas effects: 1. Both addresses are in PrecompiledAddressesBernoulli so they get warmed via StateDB.Prepare (EIP-2929: 100 gas warm vs 2600 cold). 2. When called, go-eth's CALL handler sets gas=0 for non-revert errors, burning all forwarded gas. revm's PrecompileError result (not ok-or-revert) also causes all forwarded gas to be consumed by the parent. So disabled stubs in revm match go-eth's behavior exactly. Without stubs, morph-reth treats 0x03/0x09 as cold empty accounts: 2600 base CALL cost but forwarded gas is returned (no code). For a call with ~4801 forwarded gas: go-eth costs 4901 (100+4801), morph-reth costs 2600. Diff = 2301, matching the observed gas mismatch for mainnet block 2662438 tx1. The fix: replace the working Berlin 0x03/0x09 in bernoulli() with disabled stubs that return PrecompileError. morph203() already correctly overrides these stubs with working implementations via extend(). * fix(chainspec): add missing hardfork timestamps to mainnet and hoodi genesis go-ethereum hardcodes Morph203/Viridian/Emerald activation times in MorphMainnetChainConfig and MorphHoodiChainConfig (params/config.go). morph-reth parses these from genesis JSON extra_fields, but the JSON files were missing these fields, causing all timestamp-based hardforks to silently not activate. Mainnet: morph203Time=1747029600, viridianTime=1762149600, emeraldTime=1767765600 Hoodi: viridianTime=1761544800, emeraldTime=1766988000 (morph203Time=0 was already present) Without this fix, morph-reth mainnet ran permanently in Curie hardfork with no Morph203/Viridian/Emerald activation. * fix(local-test): disable NAT external IP probing to suppress security alerts Add --nat none to reth startup args so reth does not query icanhazip.com, ifconfig.me, or ipinfo.io for public IP detection. In Engine API-driven mode morphnode handles all block sync, so P2P peer discovery is not needed. * feat(node): add geth diskRoot RPC cross-validation for MPT state root Before MPTFork activates, reth cannot validate ZK-trie state roots from block headers. This adds an optional `--morph.geth-rpc-url` CLI flag that enables cross-validation by calling geth's `morph_diskRoot` RPC after each block execution, comparing the returned MPT root against reth's computed root to catch state divergences early. If the RPC call fails (e.g. geth hasn't synced that block yet), validation is skipped gracefully rather than halting the node. * fix(node): use rustls-tls for reqwest to avoid system OpenSSL dependency * fix(node): promote diskRoot cross-validation log from debug to info * fix(node): delete morph-reth when stop * fix(node): add log * fix(node): pass configured PVB to BasicEngineValidatorBuilder BasicEngineValidatorBuilder::default() was creating its own default MorphEngineValidatorBuilder internally, discarding the geth_rpc_url that was configured via CLI args. Fix by passing the configured PVB to BasicEngineValidatorBuilder::new() so the geth diskRoot cross-validation actually receives the URL. * docs: add engine API block tag and FCU optimization design Design for adding engine_setBlockTags RPC (aligned with geth PR #277) and simplifying per-block FCU calls to improve sync performance. * docs: add engine API block tag implementation plan 5-task plan for adding engine_setBlockTags RPC and simplifying per-block FCU to align with geth PR #277. * feat(engine-api): add engine_setBlockTags RPC and decouple tag updates from block import Add `set_block_tags(safe, finalized)` to the Engine API trait, RPC server, and implementation. This aligns with geth PR #277's `engine_setBlockTags` design. Simplify `import_l2_block_via_engine` to pass B256::ZERO for safe/finalized in every-block FCU, eliminating per-block DB writes for those tags. Safe tag is now updated separately in `new_safe_l2_block` via `set_block_tags`. Remove unused `current_forkchoice_state` method and `mark_safe` parameter. * chore: commit local node and sync test updates * chore: remove unused reth engine env defaults * refactor: rename MPTFork hardfork to Jade (#33) Co-authored-by: chengwenxi <22697326+chengwenxi@users.noreply.github.com> * style: fmt all * fix(node): pass configured PVB to MorphTreeEngineValidatorBuilder MorphTreeEngineValidatorBuilder::default() was creating its own default MorphEngineValidatorBuilder, discarding the geth_rpc_url configured via CLI args. Use ::new(pvb) to propagate the configured builder. * fix: correct three bugs found in code review - hardfork: restore correct MorphHardfork→SpecId mapping Commit 538f7d4 (rename MPTFork→Jade) accidentally reverted the fix from 6200cbc, making all six variants map to OSAKA. Restored the correct mapping: Bernoulli/Curie/Morph203→CANCUN, Viridian→PRAGUE, Emerald/Jade→OSAKA. Also fixed From<SpecId> for MorphHardfork to use literal SpecId::OSAKA/PRAGUE instead of SpecId::from(Self::Jade) which resolved to OSAKA and made the Emerald/Viridian branches unreachable. - evm: use execution_ctx.extra_data in MorphBlockAssembler assemble_block was hardcoding extra_data to Default::default(), dropping any extra_data from payload attributes and causing block hash mismatches against go-ethereum. - engine-api: replace std::sync::RwLock with parking_lot::RwLock parking_lot::RwLock never poisons, eliminating the silent-discard if-let-Ok pattern in record_local_head/record_local_forkchoice and the .ok().and_then() pattern in current_head/current_forkchoice_state. * fix(node): batch Finish checkpoint updates for engine imports Align startup canonical head with the DB tip before spawning the engine tree, and move StageId::Finish updates out of the engine API hot path into the node engine-event loop with a 128-block flush interval. This keeps restart head recovery correct while reducing per-block checkpoint write overhead during sync. * debug(node): add startup head diagnostics for replay mismatch Add targeted startup and current-head fallback logs to compare best_block_number vs last_block_number and trace canonical-head correction outcomes, so replay-time "wrong block number" failures can be diagnosed from server logs without code changes. * revert: remove recent head diagnostics and checkpoint batching Undo recent startup/head diagnostic instrumentation and StageId::Finish batching changes to restore the previous engine-import path while isolating sync regression causes. * refactor(code-quality): simplify code based on review findings - Replace std::sync::Mutex with parking_lot::Mutex in MorphEngineValidator to eliminate poison risk; simplify lock call sites accordingly - Remove .unwrap() on SystemTime::duration_since(UNIX_EPOCH) in assemble_l2_block timestamp fallback; use .unwrap_or_default() instead - Split consume_sender_budget (9 params, #[allow(too_many_arguments)]) into consume_eth_budget and consume_token_budget; update call sites and tests - Remove redundant .clone() on execution_ctx.extra_data in MorphBlockAssembler Closes #26 Closes #27 Closes #32 * fix(engine-api): restore EngineStateTracker and set_block_tags after rebase * fix: hardfork SpecId mapping, fee vault routing, and cleanup - Fix From<MorphHardfork> for SpecId: Bernoulli/Curie/Morph203 → CANCUN, Viridian → PRAGUE, Emerald/Jade → OSAKA (was all OSAKA incorrectly) - Fix From<SpecId> for MorphHardfork: use literal SpecId values to avoid circular self-referential From calls - Add resolve_fee_recipient() to MorphEvmConfig to route EL fees to fee vault when configured, matching Morph's fee accounting model - Remove local-test/ and CLAUDE.md from tracking (add to .gitignore) - Remove docs/plans temporary files * simplify(evm): inline fee vault recipient logic directly at call sites * docs(engine-api): add set_block_tags to trait method overview * fix(chainspec): align Prague activation with Viridian Set Ethereum Prague activation to viridianTime so EIP-7702 timing stays consistent with geth and avoids fork-rule divergence across execution and validation paths. * style: cargo fmt * chore: add local test scripts * refactor(engine-api): remove unused forkchoice tracking from EngineStateTracker The forkchoice state (last_valid_forkchoice, record_local_forkchoice, current_forkchoice_state, head_snapshot, forkchoice_snapshot) was only written to but never read. Remove the entire dead-code chain including the ForkchoiceUpdated event branch and related test. * fix(engine-api): correct swapped error messages in build_l2_payload The two map_err calls on send_new_payload had their messages transposed: outer error (channel send failure) said "receive" and inner error (builder error response) said "send". Swap to match actual failure paths. * fix(local-test): bind HTTP RPC to 0.0.0.0 for external access Default RETH_HTTP_ADDR was 127.0.0.1, blocking external RPC connections. Auth RPC (8551) remains on 127.0.0.1 as it is internal-only. --------- Co-authored-by: vincent <vincent.ch.cn@gmail.com> Co-authored-by: chengwenxi <22697326+chengwenxi@users.noreply.github.com>
Resolves: #30
Summary by CodeRabbit