Skip to content

Conversation

@cdiielsi
Copy link
Contributor

@cdiielsi cdiielsi commented Nov 7, 2025

Motivation

It can be useful to have the INITIAL_LOOKUP_INTERVAL and LOOKUP_INTERVAL constants as parameters in the command line to test different configurations.

Description

This pr introduces initial_lookup_interval and lookup_interval as new cli parameters.

Closes #5148

@github-actions github-actions bot added the L1 Ethereum client label Nov 7, 2025
@github-actions
Copy link

github-actions bot commented Nov 7, 2025

Lines of code report

Total lines added: 47
Total lines removed: 9
Total lines changed: 56

Detailed view
+------------------------------------------------+-------+------+
| File                                           | Lines | Diff |
+------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs                       | 655   | +23  |
+------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/initializers.rs              | 446   | +2   |
+------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/initializers.rs           | 306   | +2   |
+------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/discv4/server.rs  | 501   | +6   |
+------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/network.rs        | 370   | +8   |
+------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/initiator.rs | 102   | -9   |
+------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/test_utils.rs     | 303   | +6   |
+------------------------------------------------+-------+------+

@cdiielsi cdiielsi marked this pull request as ready for review November 10, 2025 14:41
@cdiielsi cdiielsi requested a review from a team as a code owner November 10, 2025 14:41
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Nov 10, 2025
@mpaulucci mpaulucci requested a review from Copilot November 11, 2025 14:28
Copilot finished reviewing on behalf of mpaulucci November 11, 2025 14:31
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 makes the peer lookup intervals configurable via CLI arguments instead of hardcoded constants, enabling easier testing of different P2P network configurations.

Key changes:

  • Converts INITIAL_LOOKUP_INTERVAL and LOOKUP_INTERVAL from Duration constants to u64 millisecond constants
  • Adds two new CLI parameters: --p2p.initial-lookup-interval and --p2p.lookup-interval
  • Threads the configuration values through P2PContext to both DiscoveryServer and RLPxInitiator

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/CLI.md Documents the new CLI parameters with default values and descriptions; also updates version string
crates/networking/p2p/discv4/server.rs Converts constants to u64 and adds interval fields to DiscoveryServer struct
crates/networking/p2p/network.rs Adds lookup interval fields to P2PContext and accepts them as constructor parameters
crates/networking/p2p/rlpx/initiator.rs Removes local interval fields and uses values from P2PContext instead
cmd/ethrex/cli.rs Adds CLI argument definitions with default values from constants
cmd/ethrex/initializers.rs Passes new parameters when creating L1 P2P context
cmd/ethrex/l2/initializers.rs Passes new parameters when creating L2 P2P context
crates/networking/rpc/test_utils.rs Updates test helper to pass the new parameters using constant values

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

#[cfg(feature = "l2")]
pub based_context: Option<P2PBasedContext>,
pub tx_broadcaster: GenServerHandle<TxBroadcaster>,
// The initial interval between peer lookups, until the number of peers reaches [target_peers](RLPxInitiatorState::target_peers).
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

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

The documentation reference RLPxInitiatorState::target_peers is incorrect. The struct RLPxInitiatorState does not exist in the codebase. This should refer to the actual field or remove the broken documentation link.

Suggested change
// The initial interval between peer lookups, until the number of peers reaches [target_peers](RLPxInitiatorState::target_peers).
// The initial interval between peer lookups, until the number of peers reaches the target number.

Copilot uses AI. Check for mistakes.
@mpaulucci
Copy link
Collaborator

@copilot suggest better names for the cli params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

Create a CLI argument to set up the Discovery and Connetion initiation Lookup Rate

3 participants