Skip to content
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

Change p2p default port to 8084 #21162

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Change p2p default port to 8084 #21162

merged 1 commit into from
Feb 10, 2025

Conversation

mwtian
Copy link
Contributor

@mwtian mwtian commented Feb 10, 2025

Description

8084 is mentioned to be the default p2p port in docs: https://docs.sui.io/guides/operator/validator-tasks#connectivity
And validator / transaction RPC port defaults to 8080 already.
So changing the default p2p port from 8080 to 8084 in P2pConfig.
I believe the change is backward compatible but lmk otherwise.

Test plan

CI


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • gRPC:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

Copy link

vercel bot commented Feb 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Feb 10, 2025 6:39pm
sui-kiosk ⬜️ Ignored (Inspect) Feb 10, 2025 6:39pm

@mwtian mwtian temporarily deployed to sui-typescript-aws-kms-test-env February 10, 2025 18:39 — with GitHub Actions Inactive
@@ -40,7 +40,7 @@ pub struct P2pConfig {
}

fn default_listen_address() -> SocketAddr {
"0.0.0.0:8080".parse().unwrap()
"0.0.0.0:8084".parse().unwrap()
Copy link
Contributor

Choose a reason for hiding this comment

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

This came up recently:
#20971
#20973

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great context. Do you have opinions on how this should be updated?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't think of any issues this would cause for an operator who isn't explicitly setting this value

Copy link
Contributor

@johnjmartin johnjmartin left a comment

Choose a reason for hiding this comment

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

And Sui node RPC port uses the 8080 port as well.

sui node rpc uses 9000, right? or is 8080 used for something else

@mwtian
Copy link
Contributor Author

mwtian commented Feb 10, 2025

And Sui node RPC port uses the 8080 port as well.

sui node rpc uses 9000, right? or is 8080 used for something else

Updated the description. I meant the validator/transaction interface port, default set here:

"/ip4/0.0.0.0/tcp/8080".parse().unwrap()

@mwtian mwtian merged commit 43dd574 into main Feb 10, 2025
53 checks passed
@mwtian mwtian deleted the tmw/port branch February 10, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants