Skip to content

Conversation

@cdiielsi
Copy link
Contributor

@cdiielsi cdiielsi commented Nov 6, 2025

Motivation

p2p_enabled is a no op since it's the default configuration, so there's no need for it.

Description

This pr replaces the p2p_enabled flag with a p2p_disabled flag in case someone wants to disable p2p.

Closes #4979

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

github-actions bot commented Nov 6, 2025

Lines of code report

Total lines added: 0
Total lines removed: 2
Total lines changed: 2

Detailed view
+--------------------------+-------+------+
| File                     | Lines | Diff |
+--------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs | 632   | -2   |
+--------------------------+-------+------+

authrpc_addr: "localhost".into(),
authrpc_port: "8551".into(),
authrpc_jwtsecret: "jwt.hex".into(),
p2p_enabled: true,
Copy link
Contributor Author

@cdiielsi cdiielsi Nov 6, 2025

Choose a reason for hiding this comment

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

I opted for implementing the opposite behaviour to this and have p2p_disable be set with the default value false, but isn't it that p2p isn't necessary for L2? Shouldn't I set p2p_disabled: true?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure L2 does require P2P, since the P2P module has an RLPX module related to L2 P2P networking

@cdiielsi cdiielsi marked this pull request as ready for review November 7, 2025 13:24
@cdiielsi cdiielsi requested a review from a team as a code owner November 7, 2025 13:24
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Nov 7, 2025
authrpc_addr: "localhost".into(),
authrpc_port: "8551".into(),
authrpc_jwtsecret: "jwt.hex".into(),
p2p_enabled: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure L2 does require P2P, since the P2P module has an RLPX module related to L2 P2P networking

@cdiielsi cdiielsi added this pull request to the merge queue Nov 7, 2025
Merged via the queue into main with commit 7ff0ff2 Nov 7, 2025
44 checks passed
@cdiielsi cdiielsi deleted the p2p_disabled_flag branch November 7, 2025 17:35
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 7, 2025
xqft pushed a commit that referenced this pull request Nov 11, 2025
**Motivation**

`p2p_enabled` is a no op since it's the default configuration, so
there's no need for it.

**Description**

This pr replaces the `p2p_enabled` flag with a `p2p_disabled` flag in
case someone wants to disable p2p.

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

Labels

L1 Ethereum client

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Replace --p2p.enabled flag with --p2p.disabled or similar

4 participants