-
Notifications
You must be signed in to change notification settings - Fork 69
cmd/utils: fix disabling discovery through config file #33279 #2003
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-upgrade
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
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 |
There was a problem hiding this 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 pull request aims to fix issue ethereum#33279 by enabling V4 and V5 discovery to be disabled through the configuration file. The changes introduce a new DiscoveryV4 configuration field alongside the existing DiscoveryV5 field, add corresponding command-line flags, and modify the flag handling logic to respect configuration file settings.
Changes:
- Added
DiscoveryV4boolean field top2p.Configstruct for controlling V4 discovery - Set default values for both
DiscoveryV4andDiscoveryV5totrueinnode.DefaultConfig - Introduced new
DiscoveryV4Flagand updatedDiscoveryV5Flagwith new names and aliases - Modified
SetP2PConfigto only update discovery settings when flags are explicitly set, allowing config file values to be respected
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| p2p/server.go | Adds DiscoveryV4 configuration field to the Config struct |
| node/defaults.go | Sets default values for DiscoveryV4 and DiscoveryV5 to true for backward compatibility |
| cmd/utils/flags.go | Adds DiscoveryV4Flag, renames DiscoveryV5Flag, and updates SetP2PConfig to respect config file settings |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
2317891 to
f3539b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes
Ref: ethereum#33279
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which part of the codebase this PR will touch base on,
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that