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

cli/net_params: Warn on empty public-addr when starting a validator node #5240

Merged
merged 6 commits into from
Aug 7, 2024

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Aug 5, 2024

This PR shows a warning when the --public-addr is not provided for validators.

In the future, we'll transform this warning into a hard failure. Validators are encouraged to provide this parameter for better availability over the network.

cc @paritytech/networking

@lexnv lexnv self-assigned this Aug 5, 2024
Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv lexnv added the T0-node This PR/Issue is related to the topic “node”. label Aug 5, 2024
substrate/client/cli/src/config.rs Outdated Show resolved Hide resolved
lexnv added 2 commits August 5, 2024 17:15
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6903270

Signed-off-by: Alexandru Vasile <[email protected]>
})
};

// TODO: Return error here in the next release.
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have an issue for this and link the place where to add IMO. Already too many dead TODOs in the code 😬

Signed-off-by: Alexandru Vasile <[email protected]>
@lexnv lexnv added this pull request to the merge queue Aug 7, 2024
Merged via the queue into master with commit 7a6e91c Aug 7, 2024
160 of 161 checks passed
@lexnv lexnv deleted the lexnv/warn-dht-pub-addr branch August 7, 2024 09:08
@@ -638,6 +644,14 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {

logger.init()?;

if config.role.is_authority() && config.network.public_addresses.is_empty() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you check parachain collators(is_authorithy returns true) won't get this warning?

Copy link
Contributor

Choose a reason for hiding this comment

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

dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this pull request Aug 28, 2024
…ode (paritytech#5240)

This PR shows a warning when the `--public-addr` is not provided for
validators.

In the future, we'll transform this warning into a hard failure.
Validators are encouraged to provide this parameter for better
availability over the network.

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants