diff --git a/node/src/args.rs b/node/src/args.rs index 0edc8aa9..37e8ce85 100644 --- a/node/src/args.rs +++ b/node/src/args.rs @@ -200,9 +200,15 @@ pub struct RunFlags { #[arg(long, requires = "checkpoint_path")] pub unsafe_skip_checkpoint_verification: bool, - /// IP address for this node (optional, will use genesis if not provided) + /// Public IP address this node advertises to its peers, paired with + /// `--port` to form the address they dial. Optional: a node in the genesis + /// committee takes the address recorded there, and one that is not resolves + /// its public IP at startup. Set it whenever the node's own view of its + /// address is the authoritative one — a node joining an existing network + /// has no committee entry to read, and an operator who knows the address + /// need not have it discovered. #[arg(long)] - pub ip: Option, + pub ip: Option, /// Path to a TOML file containing bootstrapper nodes (pubkey and address) for syncing #[arg(long)] @@ -412,6 +418,42 @@ mod genesis_path_tests { assert!(listen.is_ipv4()); } + /// Parse `summit run ` and return the run flags. + fn run_flags(args: &[&str]) -> RunFlags { + let argv = [&["summit", "run"], args].concat(); + match CliArgs::try_parse_from(argv) + .expect("flags should parse") + .cmd + { + Command::Run { flags } => *flags, + other => panic!("expected the run subcommand, got {other:?}"), + } + } + + #[test] + fn advertised_ip_is_a_bare_address() { + // --ip carries the host only; --port supplies the port the peers dial, + // the same one the node listens on, so the two cannot disagree. + let flags = run_flags(&["--ip", "203.0.113.7", "--port", "26000"]); + assert_eq!(flags.ip, Some("203.0.113.7".parse::().unwrap())); + assert_eq!(flags.port, 26000); + } + + #[test] + fn advertised_ip_accepts_ipv6_unbracketed() { + let flags = run_flags(&["--ip", "2001:db8::7"]); + assert_eq!(flags.ip, Some("2001:db8::7".parse::().unwrap())); + } + + #[test] + fn advertised_ip_rejects_a_socket_address() { + // A port here would be a second, silently authoritative copy of --port. + // Rejecting it at parse time keeps the failure a usage error the + // operator sees immediately. + let argv = ["summit", "run", "--ip", "203.0.113.7:18551"]; + assert!(CliArgs::try_parse_from(argv).is_err()); + } + #[test] fn listener_family_follows_ipv6_dialable() { // Regression: an IPv6 advertised address (genesis ip_address, --ip, or an @@ -1225,10 +1267,8 @@ async fn get_node_ip( key_store: &KeyStore, committee: &[Validator], ) -> SocketAddr { - if let Some(ref ip_str) = flags.ip { - ip_str - .parse::() - .expect("Invalid IP address format") + if let Some(ip) = flags.ip { + SocketAddr::new(ip, flags.port) } else if let Some(addr) = committee.iter().find_map(|v| { if v.node_public_key == key_store.node_key.public_key() { Some(v.ip_address) diff --git a/node/src/bin/observer.rs b/node/src/bin/observer.rs index 875d21eb..275a76d1 100644 --- a/node/src/bin/observer.rs +++ b/node/src/bin/observer.rs @@ -285,7 +285,7 @@ fn main() -> Result<(), Box> { observer_flags.observer = Some(OBSERVER_DERIVE_IDX); // Pin the observer's advertised IP so it does NOT inherit the master // validator's genesis IP (which is already bound by validator 1). - observer_flags.ip = Some(format!("127.0.0.1:{}", 26600 + OBSERVER_SLOT * 10)); + observer_flags.ip = Some(std::net::Ipv4Addr::LOCALHOST.into()); println!( "Starting observer consensus engine (master = node{}, derive index = {})", diff --git a/node/src/bin/stake_and_checkpoint.rs b/node/src/bin/stake_and_checkpoint.rs index c1cd74e2..e3c9ad05 100644 --- a/node/src/bin/stake_and_checkpoint.rs +++ b/node/src/bin/stake_and_checkpoint.rs @@ -517,7 +517,7 @@ fn main() -> Result<(), Box> { fs::write(&consensus_key_path, encoded_consensus_key).expect("Unable to write consensus key to disk"); flags.key_store_path = format!("{}/node{}/data", args.data_dir, x); - flags.ip = Some("127.0.0.1:26640".to_string()); + flags.ip = Some(std::net::Ipv4Addr::LOCALHOST.into()); println!( "Starting consensus engine for node {} with checkpoint", diff --git a/node/src/bin/stake_and_join_with_outdated_ckpt.rs b/node/src/bin/stake_and_join_with_outdated_ckpt.rs index 4decf3bc..40934ee5 100644 --- a/node/src/bin/stake_and_join_with_outdated_ckpt.rs +++ b/node/src/bin/stake_and_join_with_outdated_ckpt.rs @@ -561,7 +561,7 @@ fn main() -> Result<(), Box> { fs::write(&consensus_key_path, encoded_consensus_key).expect("Unable to write consensus key to disk"); flags.key_store_path = format!("{}/node{}/data", args.data_dir, x); - flags.ip = Some("127.0.0.1:26640".to_string()); + flags.ip = Some(std::net::Ipv4Addr::LOCALHOST.into()); println!( "Starting consensus engine for node {} with checkpoint", diff --git a/node/src/bin/sync_from_genesis.rs b/node/src/bin/sync_from_genesis.rs index 29b0c122..59c07d59 100644 --- a/node/src/bin/sync_from_genesis.rs +++ b/node/src/bin/sync_from_genesis.rs @@ -512,7 +512,7 @@ fn main() -> Result<(), Box> { } flags.key_store_path = format!("{}/node{}/data", args.data_dir, x); - flags.ip = Some("127.0.0.1:26640".to_string()); + flags.ip = Some(std::net::Ipv4Addr::LOCALHOST.into()); // Create a bootstrappers.toml file with one of the genesis validators // Read the genesis to get a validator's public key