Conversation
- Use --port as --discovery-port if and only if explicitly given - Respect --port6
Member
Author
|
Added some docs to highlight the default ports and sneaked in some more details 👀 |
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the port selection logic so that TCP, discovery UDP, and IPv6 ports all respect the appropriate flags and default to Go-SSV values, and updates documentation to match.
- Export default port constants and use them when no CLI flag is provided
- Ensure
--portapplies to both TCP and discovery UDP,--discovery-portoverrides only discovery, and--port6is respected - Update docs to reflect new defaults and directory layout
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| book/src/running_node.md | Added Golang migration note, updated default data directory and port docs |
| book/src/cli.md | Clarified default behavior of --discovery-port |
| book/src/advanced_networking.md | Updated default UDP/TCP port list to match new defaults |
| anchor/network/src/lib.rs | Export DEFAULT_DISC_PORT, DEFAULT_QUIC_PORT, and DEFAULT_TCP_PORT constants |
| anchor/client/src/config.rs | Revised port-parsing logic to use Option<u16> flags and default constants |
| anchor/client/src/cli.rs | Made --port and --discovery-port optional and updated help text |
| .github/wordlist.txt | Added “Golang” to the wordlist |
Comments suppressed due to low confidence (4)
anchor/client/src/config.rs:461
- For the IPv6 discovery port fallback, this uses
cli_args.portinstead ofcli_args.port6. It should.or(cli_args.port6)before falling back to the default.
.use_zero_ports
book/src/running_node.md:35
- [nitpick] Consider changing “Per default” to “By default” for idiomatic English.
Create a directory for Anchor-related data and move the generated private key into the directory. Per default, Anchor
book/src/cli.md:76
- The backtick formatting is mismatched (
--port`). It should be--port`` for correct Markdown rendering.
| `--discovery-port <PORT>` | UDP port for discovery | Same as ``--port` if specified, otherwise `12001` |
anchor/client/src/cli.rs:263
- [nitpick] Use consistent branding: elsewhere it’s referred to as “Go-SSV”, so consider capitalizing here to “Go-SSV”.
matching go-ssv's default values.",
Zacholme7
previously approved these changes
Jul 10, 2025
anchor/client/src/cli.rs
Outdated
| will apply to the IPv4 address and --port6 to the IPv6 address.", | ||
| default_value = "13001", | ||
| will apply to the IPv4 address and --port6 to the IPv6 address. If this flag is not set, the default values will be 12001 for discovery and 13001 for TCP, \ | ||
| matching go-ssv's default values.", |
Member
There was a problem hiding this comment.
IMO, I wouldnt mention this is matching go-ssv.
anchor/client/src/cli.rs
Outdated
| value_name = "PORT", | ||
| help = "The UDP port that discovery will listen on. Defaults to `12001`", | ||
| default_value = "12001", | ||
| help = "The UDP port that discovery will listen on. Defaults to --port if explicitly specified, and `12001` otherwise.", |
Member
There was a problem hiding this comment.
I think this description is a little confusing. I would just add in "--port if --port is explicitly specified". Otherwise it makes it sound like this will default to --port if --discovery-port is specified.
Member
There was a problem hiding this comment.
or maybe "It has the value passed in --port, if --port is explicitly specified"
Co-authored-by: Zacholme7 <zacholme@gmail.com>
Comment on lines
259
to
262
| help = "The TCP/UDP ports to listen on. There are two UDP ports. \ | ||
| The discovery UDP port will be set to this value and the Quic UDP port will be set to this value + 1. The discovery port can be modified by the \ | ||
| --discovery-port flag and the quic port can be modified by the --quic-port flag. If listening over both IPv4 and IPv6 the --port flag \ | ||
| will apply to the IPv4 address and --port6 to the IPv6 address.", | ||
| default_value = "13001", | ||
| will apply to the IPv4 address and --port6 to the IPv6 address. If this flag is not set, the default values will be 12001 for discovery and 13001 for TCP.", |
Member
There was a problem hiding this comment.
It's a bit confusing We mention TCP two times, but the text is mostly about discovery UDP and Quic UDP.
Member
|
Small nits, otherwise looks great. |
Co-authored-by: diegomrsantos <diego@sigmaprime.io>
28 tasks
mergify bot
pushed a commit
that referenced
this pull request
Jul 10, 2025
My previous PR introduced a regression, as `--port` was only respected for the TCP port, but not the discovery port. Also I noticed that `--port6` was not correctly set at all. Ensure that: - ports are set as in Go-SSV when `--port` is not specified. - if only `--port` is specified, that is used for both TCP and discovery UDP. - if `--discovery-port` is specified, that value is used for discovery UDP regardless of whether `--port` is specified. - `--port6` is respected
diegomrsantos
pushed a commit
to diegomrsantos/anchor
that referenced
this pull request
Sep 17, 2025
My previous PR introduced a regression, as `--port` was only respected for the TCP port, but not the discovery port. Also I noticed that `--port6` was not correctly set at all. Ensure that: - ports are set as in Go-SSV when `--port` is not specified. - if only `--port` is specified, that is used for both TCP and discovery UDP. - if `--discovery-port` is specified, that value is used for discovery UDP regardless of whether `--port` is specified. - `--port6` is respected
jking-aus
pushed a commit
to jking-aus/anchor
that referenced
this pull request
Oct 8, 2025
My previous PR introduced a regression, as `--port` was only respected for the TCP port, but not the discovery port. Also I noticed that `--port6` was not correctly set at all. Ensure that: - ports are set as in Go-SSV when `--port` is not specified. - if only `--port` is specified, that is used for both TCP and discovery UDP. - if `--discovery-port` is specified, that value is used for discovery UDP regardless of whether `--port` is specified. - `--port6` is respected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
My previous PR introduced a regression, as
--portwas only respected for the TCP port, but not the discovery port.Also I noticed that
--port6was not correctly set at all.Proposed Changes
Ensure that:
--portis not specified.--portis specified, that is used for both TCP and discovery UDP.--discovery-portis specified, that value is used for discovery UDP regardless of whether--portis specified.--port6is respected