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

feat: Add enable-p2p flag to fuel-core CLI #1268

Merged
merged 6 commits into from
Aug 3, 2023

Conversation

bvrooman
Copy link
Contributor

@bvrooman bvrooman commented Jul 29, 2023

Related issues:

This PR disables P2P by default, and adds a new flag to the fuel-core CLI to allow users to expressly enable P2P:

--enable-p2p

Currently, the P2P service is only enabled when the user provides CLI arguments for keypair and network. If either of these values are missing, P2P is implicitly disabled without warning to the user. There is no way to tell if the user accidentally omitted these values, or deliberately omitted them in order to disable P2P.

A better user experience is to enable P2P explicitly using --enable-p2p and to make keypair and network mandatory. This will raise an error if either of these values is missing. P2P should be disabled by default. This flag replaces having to add or remove the network argument to enable or disable P2P implicitly.

If a user wants to run fuel-core with P2P, and supplies --enable-p2p, but forgets to include keypair and/or network, they will encounter an error:

error: the following required arguments were not provided:
  --keypair <KEYPAIR>
  --network <NETWORK>

@bvrooman bvrooman self-assigned this Jul 29, 2023
@bvrooman bvrooman marked this pull request as ready for review July 29, 2023 01:45
@bvrooman bvrooman requested a review from a team July 29, 2023 01:45
@bvrooman bvrooman marked this pull request as draft July 31, 2023 20:58
@bvrooman
Copy link
Contributor Author

As discussed in the sync today, we will change the approach to have P2P disabled by default, and instead use a flag to enable it. I will rework this PR with that feedback. In the meantime, I am putting it back in draft.

@bvrooman bvrooman changed the title feat: Add disable-p2p flag to fuel-core CLI feat: Add enable-p2p flag to fuel-core CLI Aug 2, 2023
@bvrooman bvrooman marked this pull request as ready for review August 2, 2023 22:49
@bvrooman bvrooman merged commit 9235cae into master Aug 3, 2023
21 checks passed
@bvrooman bvrooman deleted the bvrooman/feat/cli-disable-p2p-flag branch August 3, 2023 13:49
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