chore: bump reth dependency to v1.9.3 #200
Conversation
📝 WalkthroughWalkthroughUpgrade to reth v1.9.3: dependency bumps and CI/simulator migrations from eest→eels with expanded EIP-7610 test coverage; EVM API exposes Changes
Sequence Diagram(s)sequenceDiagram
participant BlockEnv as Block Environment
participant Executor as BerachainBlockExecutor
participant EVM as EVM
participant POLTx as POL Transaction
participant BlobTracker as Blob Gas Tracker
participant Result as BlockExecutionResult
BlockEnv->>Executor: provide block environment
Executor->>BlockEnv: call timestamp(), basefee(), number(), gas_limit(), prevrandao()
BlockEnv-->>Executor: return values
alt Prague1 active
Executor->>POLTx: construct POL tx (uses basefee(), number())
POLTx->>EVM: execute POL tx
EVM-->>Executor: POL receipt/result
Executor->>Result: prepend POL receipt
end
loop each transaction
Executor->>EVM: execute tx
EVM-->>Executor: receipt & execution result
alt Cancun active and tx has blobs
Executor->>BlobTracker: add tx blob_gas_used
BlobTracker-->>Executor: update block blob_gas_used
end
end
Executor->>Result: finish() -> assemble receipts, state, blob_gas_used
Result-->>Executor: return BlockExecutionResult (includes blob_gas_used)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
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 |
fridrik01
force-pushed
the
bump-reth-1-9-3
branch
from
1d934b6 to
ba5e844
Compare
|
looks like |
|
certain tests in are in both |
|
Similar for |
rezzmah
left a comment
rezzmah
left a comment
There was a problem hiding this comment.
some minor changes as commented
|
|
src/node/evm/executor.rs
Outdated
|
|
||
| // append gas used | ||
| self.gas_used += gas_used; | ||
| self.blob_gas_used += tx.tx().blob_gas_used().unwrap_or(0); |
There was a problem hiding this comment.
eth equivalent is
// only determine cancun fields when active
if self.spec.is_cancun_active_at_timestamp(self.evm.block().timestamp().saturating_to()) {
let tx_blob_gas_used = tx.tx().blob_gas_used().unwrap_or_default();
self.blob_gas_used = self.blob_gas_used.saturating_add(tx_blob_gas_used);
}On berachain cancun is always active and the default will handle not active case but for consistency it'd be better to use same as upstream
| receipts: self.receipts, | ||
| requests, | ||
| gas_used: self.gas_used, | ||
| blob_gas_used: self.blob_gas_used, |
There was a problem hiding this comment.
This blob_gas_used is passed to the assembler which no longer needs to re-calculate as noted in other comment
depends on bump berachain/hive#19 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/assets/hive/build_simulators.sh (1)
2-2: Update the version reference in the comment.The comment still references reth v1.8.1, but this PR upgrades to v1.9.3. Please update the comment to reflect the current version.
🔎 Proposed fix
-# Copied from reth v1.8.1 +# Copied from reth v1.9.3
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/assets/hive/build_simulators.sh.github/assets/hive/expected_failures.yamldeny.tomlsrc/node/evm/assembler.rssrc/node/evm/executor.rs
💤 Files with no reviewable changes (1)
- deny.toml
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.rs: No comments in code unless explicitly requested
Use custom error types over generic errors
Extract magic numbers into documented constants
Write succinct documentation focusing on 'why' rather than 'what'
Files:
src/node/evm/executor.rssrc/node/evm/assembler.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: rezbera
Repo: berachain/bera-reth PR: 152
File: src/pool/transaction.rs:3-4
Timestamp: 2025-09-10T05:49:07.298Z
Learning: In reth v1.7.0 upgrade, BerachainPooledTransactionVariant type alias should use PooledTransactionVariant directly instead of EthereumTxEnvelope<TxEip4844WithSidecar<BlobTransactionSidecarVariant>>.
📚 Learning: 2025-12-01T21:16:41.509Z
Learnt from: CR
Repo: berachain/bera-reth PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T21:16:41.509Z
Learning: Applies to src/genesis/**/*.rs : Prague1 hardfork must be activated at genesis (time: 0) with 1 gwei minimum base fee
Applied to files:
src/node/evm/executor.rssrc/node/evm/assembler.rs
📚 Learning: 2025-09-10T05:49:07.298Z
Learnt from: rezbera
Repo: berachain/bera-reth PR: 152
File: src/pool/transaction.rs:3-4
Timestamp: 2025-09-10T05:49:07.298Z
Learning: In reth v1.7.0 upgrade, BerachainPooledTransactionVariant type alias should use PooledTransactionVariant directly instead of EthereumTxEnvelope<TxEip4844WithSidecar<BlobTransactionSidecarVariant>>.
Applied to files:
.github/assets/hive/expected_failures.yamlsrc/node/evm/assembler.rs
🧬 Code graph analysis (2)
src/node/evm/executor.rs (3)
src/engine/payload.rs (2)
timestamp(38-40)timestamp(112-114)src/engine/mod.rs (1)
timestamp(252-254)src/primitives/header.rs (1)
timestamp(332-334)
src/node/evm/assembler.rs (2)
src/primitives/header.rs (1)
timestamp(332-334)src/transaction/pol.rs (1)
create_pol_transaction(16-53)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Security audit
- GitHub Check: Run tests
- GitHub Check: Check documentation
- GitHub Check: Clippy lint
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (15)
src/node/evm/executor.rs (4)
58-59: LGTM! Blob gas tracking added correctly.The
blob_gas_usedfield is properly initialized and will be accumulated during transaction execution, addressing the optimization concern mentioned in the PR objectives about avoiding redundant blob gas calculations in the assembler.Also applies to: 75-75
87-87: LGTM! Accessor methods correctly migrated.All
BlockEnvfield accesses have been correctly migrated to use the new trait-based accessor methods (timestamp(),basefee(),number(),gas_limit()), aligning with the reth v1.9.3 breaking SDK changes mentioned in the PR objectives.Also applies to: 101-101, 105-105, 175-175, 208-208, 242-242, 270-270, 299-299
242-245: LGTM! Blob gas accumulation follows upstream pattern.The Cancun activation check and blob gas accumulation logic correctly follows the upstream pattern, as suggested in previous review feedback. The implementation properly handles the case when Cancun is not active by skipping the accumulation.
332-337: LGTM! BlockExecutionResult now includes blob_gas_used.The
blob_gas_usedfield is correctly included in the finalBlockExecutionResult, which allows the assembler to use this computed value instead of recalculating it. This addresses the optimization concern raised in the PR objectives about redundant calculations in the assembler.src/node/evm/assembler.rs (4)
9-9: LGTM! Imports updated correctly.The new imports align with the reth v1.9.3 API changes, including the
Blocktrait for accessor methods and the updatedBlockExecutionResultstructure.Also applies to: 12-14
57-57: LGTM! Blob gas now sourced from executor, eliminating redundant calculation.The assembler now receives
blob_gas_usedfrom theBlockExecutionResult(computed in the executor) instead of recalculating it, directly addressing the optimization concern raised in the PR objectives. The local variable is appropriately renamed toblock_blob_gas_usedto avoid shadowing.Also applies to: 114-114, 118-118
62-62: LGTM! BlockEnv accessor methods correctly applied.All block environment field accesses have been properly migrated to use the new accessor methods (
timestamp(),basefee(),number(),beneficiary(),prevrandao(),gas_limit(),difficulty()), aligning with the reth v1.9.3 breaking SDK changes.Also applies to: 72-72, 76-76, 136-136, 143-143, 145-148, 152-152
67-95: LGTM! Prague1 POL transaction injection implemented correctly.The POL transaction injection logic for Prague1 is well-structured:
- Validates proposer pubkey presence
- Synthesizes the POL transaction using the correct base fee and block number from accessors
- Prepends it to the transactions list
- Validates receipt presence and transaction ordering
The implementation correctly ensures the POL transaction is the first transaction when Prague1 is active.
.github/assets/hive/build_simulators.sh (2)
32-33: Artifact naming is consistent with simulator change.The artifact names correctly reflect the simulator name change from eest to eels. Note that per the PR comments, the expected_failures lists for
eels/consume-rlpandeels/consume-engineshould be made consistent with upstream reth to ensure these artifacts are properly tested.
15-15: The simulator invocation and fixtures configuration are consistent with upstream reth v1.9.3. Both configurations use the same simulator name (ethereum/eels) and fixtures URL (v5.3.0). The docker image paths referenced (hive/simulators/ethereum/eels/consume-engine:latestandhive/simulators/ethereum/eels/consume-rlp:latest) match those in the docker save commands. The use ofbera-rethclient and the inclusion of theberachain/berachain-rpc-compatsimulator are intentional fork-specific customizations, not discrepancies with upstream.Likely an incorrect or invalid review comment.
.github/assets/hive/expected_failures.yaml (5)
1-1: Version header updated correctly.The version reference now accurately reflects the reth v1.9.3 source.
47-70: Excellent documentation of test failure rationale.The added comments provide clear technical justification for why EIP-7610 collision tests and system contract tests are expected to fail, including references to the mathematical impossibility of certain scenarios on mainnet.
152-157: Clear explanation of blob limit test behavior.The comment effectively explains why the blob limit test fails in
eels/consume-rlp(pipeline import approach) but passes ineels/consume-engine, and clarifies that the failure actually indicates correct block rejection behavior.
195-244: EIP-7610 test entries added to eels/consume-rlp.These test entries expand the EIP-7610 collision test coverage. Ensure these are consistent with upstream reth v1.9.3 as noted in the verification script above.
71-151: Test list consistency already matches upstream reth v1.9.3.The eels/consume-engine section in the current PR is already aligned with upstream reth v1.9.3. Both contain 173 test entries with identical test type distributions. The eels/consume-rlp section (86 entries) is also consistent. The unique test distribution is preserved:
test_engine_from_state_testappears only in consume-engine, andtest_max_blobs_per_tx_fork_transitionappears only in consume-rlp, matching the upstream configuration.
Question, should we not just copy the upstream reth |
open to it. whichever is more restrictive is better imo. for a diff PR tho |
fridrik01
force-pushed
the
bump-reth-1-9-3
branch
from
ae06a76 to
9b5b585
Compare
fridrik01
force-pushed
the
bump-reth-1-9-3
branch
from
9b5b585 to
29635d1
Compare
There was a problem hiding this comment.
Pull request overview
This PR bumps the reth dependency from v1.8.4 to v1.9.3, adapting the codebase to handle several breaking SDK changes introduced in the new version. The update also aligns alloy dependencies with upstream reth v1.9.3 and synchronizes test configurations.
Changes:
- Updated all reth and alloy dependencies to align with v1.9.3
- Refactored BlockEnv field access to use trait methods instead of direct field access
- Modified ConfigureEngineEvm trait methods to return Result types
- Added BlockEnv associated type to Evm and EvmFactory implementations
- Simplified RpcConvert trait bounds by replacing TxEnv and Spec with Evm parameter
- Enhanced blob gas tracking with per-block accounting
- Updated hive test configurations (eest → eels simulator, fixtures v5.1.0 → v5.3.0)
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | Bumped reth dependencies from v1.8.4 to v1.9.3, updated alloy packages from 1.0.37 to 1.0.41, and aligned versions with upstream |
| src/rpc/mod.rs | Updated RpcConvert trait bounds to use Evm instead of TxEnv and Spec parameters |
| src/rpc/api.rs | Removed AddDevSigners trait implementation, simplified RpcConvert bounds, added evm_memory_limit method, removed EthApiSpec associated types |
| src/node/evm/executor.rs | Converted BlockEnv field access to method calls, added blob_gas_used tracking field and logic |
| src/node/evm/config.rs | Updated ConfigureEngineEvm methods to return Result with Infallible error type |
| src/node/evm/assembler.rs | Converted BlockEnv field access to method calls, updated blob_gas_used handling to use executor-computed value |
| src/evm/mod.rs | Added BlockEnv associated type to Evm and EvmFactory trait implementations |
| src/engine/builder.rs | Changed InvalidPoolTransactionError::OversizedData from tuple to struct variant |
| .github/workflows/hive.yml | Renamed simulator from ethereum/eest to ethereum/eels across all test scenarios |
| .github/assets/hive/ignored_tests.yaml | Updated header comment to reference reth v1.9.3, added new ignored test case |
| .github/assets/hive/expected_failures.yaml | Updated header to v1.9.3, enhanced documentation for EIP-7610 and system contract tests, added extensive EIP-7610 test cases |
| .github/assets/hive/build_simulators.sh | Updated simulator name from eest to eels, bumped fixtures from v5.1.0 to v5.3.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/assets/hive/build_simulators.sh (1)
15-46:⚠️ Potential issue | 🔴 CriticalCritical: Naming mismatch will cause image loading to fail.
Lines 33-34 save
eels_engine.tarandeels_rlp.tar, butload_images.sh(lines 15-16) expects/tmp/eest_engine.tarand/tmp/eest_rlp.tar. The prefix divergence (eels_vseest_) means the test job will fail to load these images. Fix the naming to match betweenbuild_simulators.shandload_images.sh.Additionally, line 15's TODO about pinning the eels branch remains open and should be tracked to prevent simulator source drift.
🧹 Nitpick comments (1)
src/node/evm/executor.rs (1)
242-246: Remove the inline comment to comply with the Rust style guidelines.As per coding guidelines: No comments in code unless explicitly requested.🧹 Proposed change
- // only determine cancun fields when active if self.spec.is_cancun_active_at_timestamp(self.evm.block().timestamp().saturating_to()) {
This PR bumps reth dependency from
v1.8.4tov1.9.3. Note that it required bumping alloy dependency as well.Reth v1.9.x contain several breaking SDK changes, most notable being:
BlockEnvfields are now accessed via trait methods instead of struct fieldsConfigureEngineEvmmethods now returnResult.EvmandEvmFactorynow requireBlockEnvassociated typeRpcConverttrait changedEthApiSpectrait was simplifiedThis PR also syncs the expected_failures/ignored_tests/hive configs with upstream reth v1.9.3. Note that removed few passing
EIP-6110 fork_Praguetests from expected failures as they are currently working, don't know why though.Note: The alloy dependencies should align exactly with upstream reth v1.9.3 release , see here
Test plan
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.