feat: upgrade reth sdk to v2.4.0 - #277
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
| gas_limit: POL_TX_GAS_LIMIT, // this is the env value used in revm for system calls | ||
| gas_price: base_fee.into(), /* gas price is set to the base fee for RPC | ||
| * compatibility reasons */ | ||
| gas_limit, // the block gas limit (36M per the Berachain genesis configurations) | ||
| gas_price: base_fee.into(), /* gas price is set to the base fee for RPC | ||
| * compatibility reasons */ |
There was a problem hiding this comment.
As mentioned on our last sync, I would prefer we try to keep reth 2.0 state compatible so we can roll this out without an EL hard fork. We can consider updating the gaslimit in future more lightweight el hf.
|
@calbera update on this? |
Still testing.
@camembera found an issue with taking snapshots of a v2 node (or rather downloading that snapshot) but its not bera-reth specific. filed an upstream bug here. Next steps are running this live on a devnet and migrating validator nodes from v1 to v2 incrementally and ensure network stays live. Would definitely help to start code review.. I'll open this PR once these tests are done. |
fridrik01
left a comment
There was a problem hiding this comment.
Quick review, looks good, will wait for devnet testing and verification for a more detailed review.
Upgrade to Reth SDK v2.4.0 (Storage V2)
Reviewer Outline
Suggested review order, from highest to lowest risk.
Cargo.lock(~4,300 lines) can be skimmed — the hand-written diff is roughly 1,700 lines.Area | Code Files | What to scrutinize
src/engine/payload.rs| The sha256 payload-ID derivation moved into PayloadAttributes::payload_id. Must produce byte-identical IDs; check the preserved unit-test vectors.src/primitives/header.rs,src/transaction/mod.rs| Golden byte-vector tests (mod db_format) pin the Compact encoding. Confirm no codec change slipped in — existing datadirs depend on it.src/engine/rpc.rs,src/engine/mod.rs,src/engine/builder.rs| beacon-kit ≥ v1.4.1 treats EL HTTP 4xx as fatal; the custom getPayloadV4P11 / pre-Osaka getPayloadV5 → UnsupportedFork behavior must survive.src/node/evm/*,src/pool/,src/rpc/, remainingsrc/| Trait-signature churn dictated by upstream; verify against reth v2.4.0 patterns rather than line-by-line.Cargo.toml,build.rs,deny.toml| default-features = false on reth — confirm the explicit feature list loses nothing vs v1.11.4.tests/e2e/storage_v2_test.rs,tests/e2e/gas_limit_regression_test.rs,tests/e2e/*| New coverage; check assertions actually pin the behaviors above.README.md,docs/storage-v2.md,.github/,Dockerfiles,CLAUDE.md| Operator guidance accuracy; hive assets flagged for re-tuning.Points worth reviewer attention:
rethdependency is nowdefault-features = falsewith an explicit feature list. Reth v2.4.0's defaults includejit(revmc, requires a system LLVM toolchain) andgmp; the explicit list reproduces the v1.11.4-equivalent set (otlp,otlp-logs,js-tracer,keccak-cache-global,asm-keccak,min-debug-logs) plusreth-revmwithportable.rust-version = "1.95"(reth v2.4.0's MSRV).[profile.dev]gainsdebug = "line-tables-only"+split-debuginfo = "unpacked", matching upstream reth — debug test binaries otherwise exceed multiple GB each.Storage V2
No node wiring was needed: the v2.4.0 launcher defaults handle everything.
--storage.v2flag (which only applies at datadir creation).bera-reth db migrate-v2 --chain <genesis>, confirmed generic over Berachain's custom primitives.docs/storage-v2.mdsays so explicitly.Operator-facing docs:
docs/storage-v2.md(what changed, the three operator situations, migration procedure,--storage.v2semantics, node modes) plus a new "Storage" section inREADME.md.API migration, file by file
src/primitives/header.rs— upstream removedRlpBincode/SerdeBincodeCompat(impls deleted);HeaderMutgainedset_mix_hash/set_extra_data/set_parent_beacon_block_root;Decompressnow returnsreth_codecs::DecompressError;BlockHeadergainedblock_access_list_hash()/slot_number(), bothNone(Berachain headers don't adopt EIP-7928 BAL fields). The on-diskCompactencoding is intentionally untouched, locked by golden testHEADER_V2_4_0_GOLDENplus a pre-Prague1 vector.src/transaction/mod.rs,txtype.rs—SignedTransactionis now blanket-implemented upstream, so the manual impl is deleted; sameDecompressErrorchange; golden vectorPOL_ENVELOPE_V2_4_0_GOLDENlocks the PoL envelope's on-disk format.src/engine/payload.rs— the structural pivot of this PR.PayloadTypeslosttype PayloadBuilderAttributes, soBerachainPayloadBuilderAttributesis deleted and the custom sha256 payload-ID derivation moved into the new requiredPayloadAttributes::payload_id(&self, parent_hash)(src/engine/payload.rs:37). Consensus-critical; the pre-existingberachain_payload_idunit-test vectors are preserved and pass. Also adds the now-requiredFrom<BerachainBuiltPayload> for BerachainExecutionData.src/engine/mod.rs—block_to_payloadgained abal: Option<Bytes>parameter;ExecutionPayloadgainedgas_limit()/slot_number().src/engine/builder.rs—PayloadConfignow carriespayload_id;BuildArgumentsgainedexecution_cache/state_root_handle(unused here);execute_transactionreturnsGasOutput;finish(state_provider, None)takes the new precomputed-state-root argument;mark_invalidtakes owned errors.src/engine/rpc.rs—EngineApi::newtakesreth_tasks::Runtimeinstead of a boxedTaskSpawner. v2.4.0's default engine capabilities flow through; the existingengine_getPayloadV5removal (pre-OsakaUnsupportedFork) is retained because beacon-kit ≥ v1.4.1 treats EL HTTP 4xx as fatal.src/evm/mod.rs— revm 41:Evmgained the requiredcfg_env(); the system-call path usesMainnetHandler::run_system_call/inspect_run_system_call;ExecutionResult::Successnow carriesResultGas(EIP-8037 gas split), and the PoLtransact_rawpath zeroes it viaResultGas::default().src/node/evm/executor.rs,config.rs,builder.rs—commit_transactionreturnsGasOutput;BlockExecutorFactorygainedtype TxExecutionResultand theExecutorGAT with aStateDBbound;BlockEnvgainedslot_num;BuildPendingEnvgained aBlockOverridesparameter.src/consensus/mod.rs—FullConsensus::validate_block_post_executiongained a fourth parameter (block_access_list_hash, forwarded to the inner Ethereum validator);ConsensusError::Otheris nowArc<dyn Error>, so all string sites useConsensusError::msg(...).src/pool/transaction.rs— implements the newPoolTransaction::consensus_ref(); the pool tx now storesRecovered<BerachainTxEnvelope>.src/rpc/api.rs— alloy 2.x orphan rules: thealloy_network::Network/TransactionBuilderimpls are replaced byimpl reth_rpc_convert::RpcTypes for BerachainNetwork(src/rpc/api.rs:101);TaskSpawner→Runtime; addssend_pool_transactionand the fully-defaultedGetBlockAccessList/EthSubscriptionsmarker impls.Tests
169 tests, all passing. New and changed coverage:
tests/e2e/storage_v2_test.rs(new) — five binary-driven tests viaCARGO_BIN_EXE:test_fresh_datadir_defaults_to_storage_v2test_storage_v2_flag_opts_new_datadir_into_v1test_v1_datadir_remains_readabletest_migrate_v2_converts_v1_datadir_in_placetest_migrate_v2_is_idempotentsrc/primitives/header.rsandsrc/transaction/mod.rspin the exact on-disk bytes, so any future codec drift fails loudly.tests/e2e/mod.rs,osaka_engine_api_test.rs— migrated to the v2 e2e harness (BerachainPayloadAttributesgenerator,Runtime::test(),BuildNewPayload/next_attributes()).CI / ops
deny.toml— drops five ignores resolved by the upgrade (hickory-proto ×2, git2 ×2 via vergen-git2 10, core2); adds dev-onlyRUSTSEC-2023-0089(atomic-polyfill via test-fuzz)..github/workflows/sync.yml— beacon-kit genesis/peer URLs pinned to v1.4.2-rc.0; adds adb settingsstep, making the nightly fresh-datadir sync a Storage V2 canary..github/assets/hive/) — re-synced from reth v2.4.0 with the Berachain deltas re-applied;expected_failures.yaml/ignored_tests.yamlheaders note they were last tuned against v1.11.4 and need re-validation once a v2.4.0-based nightly image exists.Cross.toml—cargo-chefpinned to a Rust ≥ 1.95 tag with an MSRV comment.Documentation
README.md— Rust 1.95+ prerequisite; new "Running with BeaconKit" section (v1.4.2 two-terminal flow, JWT/genesis wiring, why--engine.persistence-threshold 0and--engine.memory-block-buffer-target 0); new "Storage" section.docs/storage-v2.md(new) — full operator guide for the V1 → V2 transition.CLAUDE.md— refreshed reference versions (reth v2.4.0, beacon-kit v1.4.2), corrected file paths, Storage V2 note.TODOs
Still needs operator validation outside this environment:
migrate-v2run.scripts/test-block-progression.sh.