-
Notifications
You must be signed in to change notification settings - Fork 144
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(daemon): add --p2p-listen-address
arg to forest
#3995
Conversation
src/cli_shared/cli/mod.rs
Outdated
@@ -57,6 +58,9 @@ pub struct CliOpts { | |||
/// Address used for RPC. By defaults binds on localhost on port 2345. | |||
#[arg(long)] | |||
pub rpc_address: Option<SocketAddr>, | |||
/// P2P addresses, e.g. `--p2p-address /ip4/0.0.0.0/tcp/12345 --p2p-address /ip4/0.0.0.0/tcp/12346` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use the word listening
somewhere. I tend to grep for such keywords, and it's ultimately what it does. Perhaps --p2p-listening-address
? Otherwise, at first glance, I'd assume it's an address to which Forest should connect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Co-authored-by: Hubert <hubert@chainsafe.io>
--p2p-adderss
arg to forest
--p2p-listening-address
arg to forest
@@ -174,6 +178,10 @@ impl CliOpts { | |||
} | |||
} | |||
|
|||
if let Some(addresses) = &self.p2p_listening_address { | |||
cfg.network.listening_multiaddrs = addresses.clone(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: not a fan of "listening". "listen" is better. And is actually the way it's put for in e.g. ssh configs. Otherwise good to go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LesnyRumcajs How about using listen instead of listening?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ruseinov fixed.
--p2p-listening-address
arg to forest
--p2p-listen-address
arg to forest
Summary of changes
As part of #3990
This PR allows overriding p2p listening addresses via CLI options, which is easier than using a config file. The next step is updating
forest-iac
to publicly expose the p2p addresses of forest nodes to allow accepting connections from new peers, to improve discoverability and connectivity.Changes introduced in this pull request:
--p2p-listen-address
CLI option toforest
daemon--p2p-listen-address A --p2p-listen-address B
)Reference issue to close (if applicable)
Closes
Other information and links
Change checklist