Skip to content

Conversation

@avilagaston9
Copy link
Contributor

@avilagaston9 avilagaston9 commented Nov 10, 2025

Motivation

Currently, we initialize the peer_handler and SyncManager in L2 only because the RpcApiContext struct requires them.

Description

Makes peer_handler and SyncManager fields optional.

Closes #5222

@github-actions
Copy link

Lines of code report

Total lines added: 26
Total lines removed: 0
Total lines changed: 26

Detailed view
+----------------------------------------------------+-------+------+
| File                                               | Lines | Diff |
+----------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/initializers.rs               | 306   | +2   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/admin/peers.rs        | 153   | +5   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/fork_choice.rs | 359   | +3   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/payload.rs     | 718   | +10  |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/eth/client.rs         | 158   | +4   |
+----------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/net/mod.rs            | 20    | +2   |
+----------------------------------------------------+-------+------+

@avilagaston9 avilagaston9 marked this pull request as ready for review November 10, 2025 18:29
@avilagaston9 avilagaston9 requested a review from a team as a code owner November 10, 2025 18:29
Copilot AI review requested due to automatic review settings November 10, 2025 18:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR refactors the RPC API context to make syncer and peer_handler optional, enabling conditional initialization based on the "based" mode configuration in L2. The version is also bumped from 5.0.0 to 6.0.0.

Key changes:

  • Modified RpcApiContext to use Option<Arc<SyncManager>> and Option<PeerHandler> instead of required fields
  • Refactored L2 initializer to conditionally create networking components only when based mode is enabled
  • Updated all RPC endpoints to handle optional syncer and peer_handler with proper error messages

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
docs/CLI.md Updated default builder extra data version from 5.0.0 to 6.0.0
crates/networking/rpc/rpc.rs Changed syncer and peer_handler fields to Option types in RpcApiContext
crates/networking/rpc/test_utils.rs Updated test context construction to wrap values in Some()
crates/networking/rpc/net/mod.rs Added Option unwrapping with error handling for peer_handler
crates/networking/rpc/eth/client.rs Added Option unwrapping with error handling for syncer in syncing endpoint
crates/networking/rpc/engine/payload.rs Added Option unwrapping with error handling for syncer in payload handlers
crates/networking/rpc/engine/fork_choice.rs Added Option unwrapping with error handling for syncer in fork choice handler
crates/networking/rpc/admin/peers.rs Added Option unwrapping with error handling for peer_handler in admin endpoints
crates/l2/networking/rpc/rpc.rs Updated start_api signature to accept optional syncer and peer_handler
cmd/ethrex/l2/initializers.rs Refactored to conditionally initialize networking components based on based mode flag

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

@avilagaston9 avilagaston9 moved this to In Review in ethrex_l2 Nov 10, 2025
@ilitteri ilitteri added this pull request to the merge queue Nov 10, 2025
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L2 Rollup client

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

L2: review P2P initialization

4 participants