Sponsored Content
Skip to content

fix: Set default peering port to 2459 - #6848

Merged
bthomee merged 2 commits into
XRPLF:developfrom
shortthefomo:fix/default-peering-stanza-2459
May 19, 2026
Merged

fix: Set default peering port to 2459#6848
bthomee merged 2 commits into
XRPLF:developfrom
shortthefomo:fix/default-peering-stanza-2459

Conversation

@shortthefomo

@shortthefomo shortthefomo commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

only leave the correct default in place 2459 and not 51235

force the default port, the backwards compatibility is still present higher up in the config default file. This just forces lazy operators across.

default port was giving back in 2019 #3037

the definition is still provided higher up in the file

#   One address or domain name per line is allowed. A port may be specified
#   after adding a space to the address. If a port is not specified, the default
#   port of 2459 will be used. Many servers still use the legacy port of 51235.
#   To connect to such servers, you must specify the port number. The ordering
#   of entries does not generally matter.

fixes issue #6847

@github-actions

Copy link
Copy Markdown

⚠️ This PR contains unsigned commits. To get your PR merged, please sign them. ⚠️

If only the most recent commit is unsigned, you can run:

  1. Amend the commit: git commit --amend --no-edit -n -S
  2. Overwrite the commit: git push --force-with-lease

If multiple commits are unsigned, you can run:

  1. Go into interactive rebase mode: git rebase --interactive HEAD~<NUM_OF_COMMITS>, where NUM_OF_COMMITS is the number of most recent commits that will be available to edit.
  2. Change "pick" to "edit" for the commits you need to sign, and then save and exit.
  3. For each commit, run: git commit --amend --no-edit -n -S
  4. Continue the rebase: git rebase --continue
  5. Overwrite the commit(s): git push --force-with-lease

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

  1. Generate a GPG key
  2. Add the GPG key to your GitHub account
  3. Configure git to use your GPG key for commit signing
Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

  1. Generate an SSH key and add it to ssh-agent
  2. Add the SSH key to your GitHub account
  3. Configure git to use your SSH key for commit signing
Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.
See use 1Password to sign your commits.

…higher up in the config default file. This just forces lazy operators accross.
@shortthefomo
shortthefomo force-pushed the fix/default-peering-stanza-2459 branch from 6cacf0f to 8f9d718 Compare April 10, 2026 01:29
@shortthefomo shortthefomo changed the title fix: issue 6847 default peering port to 2459 (Version: [xrpld version: 3.1.0]) fix: issue 6847 default peering port to 2459 Apr 10, 2026
@legleux

legleux commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

This is long overdue.

@legleux
legleux self-requested a review April 10, 2026 16:39
@legleux legleux added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Apr 10, 2026
@legleux

legleux commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

@bthomee how do you feel about the convention of leaving default items in config files commented out?

@bthomee

bthomee commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator

@bthomee how do you feel about the convention of leaving default items in config files commented out?

That sounds reasonable to me, in principle. However, do we have any mechanism in place to guarantee that the values we think are the default (as stated in the config file) are actually the default (as executed by the code)?

@shortthefomo

shortthefomo commented Apr 17, 2026

Copy link
Copy Markdown
Contributor Author

That sounds reasonable to me, in principle. However, do we have any mechanism in place to guarantee that the values we think are the default (as stated in the config file) are actually the default (as executed by the code)?

The [port_peer] section is purely user-configured — there's no fallback to DEFAULT_PEER_PORT if [port] is omitted (it would just fail to parse). The 2459 default only kicks in for [ips]/[ips_fixed] entries and the connect RPC when no port is specified.

By that view as well then one should default the whole stanza for [port_peer] ?

@legleux

legleux commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

It seems it is totally required to be specified. I guess I thought it might not be since it does get used automatically elsewhere.

Let's just leave it as is until the config gets a deeper refactor I guess.

@legleux legleux reopened this Apr 27, 2026
@bthomee bthomee changed the title fix: issue 6847 default peering port to 2459 fix: Set default peering port to 2459 Apr 29, 2026
Copilot AI review requested due to automatic review settings April 29, 2026 20:49
@bthomee

bthomee commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

@shortthefomo ready to merge?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the example xrpld configuration to use the IANA-registered XRPL peering port (2459) as the default listening port, aligning the sample config with current network expectations while leaving legacy-port guidance elsewhere in the file.

Changes:

  • Change [port_peer] default port from 51235 to 2459.
  • Remove the local backward-compatibility comment block from the [port_peer] stanza (legacy guidance remains earlier in the file under [ips]).

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

@shortthefomo

Copy link
Copy Markdown
Contributor Author

@bthomee there is no blocker from me it is good to go.

I had closed this previously due to this comment

Let's just leave it as is until the config gets a deeper refactor I guess.

@mvadari mvadari added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 19, 2026
@bthomee
bthomee added this pull request to the merge queue May 19, 2026
Merged via the queue into XRPLF:develop with commit 1e45d36 May 19, 2026
1 check passed
@legleux

legleux commented May 19, 2026

Copy link
Copy Markdown
Contributor

@bthomee there is no blocker from me it is good to go.

I had closed this previously due to this comment

Let's just leave it as is until the config gets a deeper refactor I guess.

@shortthefomo Sorry for the confusion. I meant to not try to change the config to be showing the defaults as commented out. I think multiple people are working on config refactors.

Definitely wanted the IANA port as the default in the config, thanks!

Tapanito pushed a commit that referenced this pull request May 19, 2026
Co-authored-by: Bart <bthomee@users.noreply.github.com>
@mvadari mvadari added this to the 3.2.0 milestone May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants