chore: Produce an error if bootnode multiaddr do not have peer ID (#106)

celestia/src/native.rs

@@ -14,19 +14,19 @@ use tracing::info;
 
 #[derive(Debug, Parser)]
 struct Args {
-    /// Network to connect
+    /// Network to connect.
     #[arg(short, long, value_enum, default_value_t)]
     network: Network,
 
-    /// Listening addresses
+    /// Listening addresses. Can be used multiple times.
     #[arg(short, long = "listen")]
     listen_addrs: Vec<Multiaddr>,
 
-    /// Bootnode
+    /// Bootnode multiaddr, including peer id. Can be used multiple times.
     #[arg(short, long = "bootnode")]
     bootnodes: Vec<Multiaddr>,
 
-    /// Persistent header store path
+    /// Persistent header store path.
     #[arg(short, long = "store")]
     store: Option<PathBuf>,
 }
@@ -46,7 +46,7 @@ pub async fn run() -> Result<()> {
 
     let p2p_local_keypair = identity::Keypair::generate_ed25519();
 
-    let p2p_bootstrap_peers = if args.bootnodes.is_empty() {
+    let p2p_bootnodes = if args.bootnodes.is_empty() {
         network_bootnodes(args.network).await?
     } else {
         args.bootnodes
@@ -69,7 +69,7 @@ pub async fn run() -> Result<()> {
         network_id,
         genesis_hash,
         p2p_local_keypair,
-        p2p_bootstrap_peers,
+        p2p_bootnodes,
         p2p_listen_on: args.listen_addrs,
         store,
     })

node/src/node.rs

@@ -32,7 +32,7 @@ where
     pub network_id: String,
     pub genesis_hash: Option<Hash>,
     pub p2p_local_keypair: Keypair,
-    pub p2p_bootstrap_peers: Vec<Multiaddr>,
+    pub p2p_bootnodes: Vec<Multiaddr>,
     pub p2p_listen_on: Vec<Multiaddr>,
     pub store: S,
 }
@@ -55,7 +55,7 @@ where
         let p2p = Arc::new(P2p::start(P2pArgs {
             network_id: config.network_id,
             local_keypair: config.p2p_local_keypair,
-            bootstrap_peers: config.p2p_bootstrap_peers,
+            bootnodes: config.p2p_bootnodes,
             listen_on: config.p2p_listen_on,
             store: store.clone(),
         })?);

node/src/p2p.rs

@@ -76,6 +76,9 @@ pub enum P2pError {
 
     #[error("HeaderSub already initialized")]
     HeaderSubAlreadyInitialized,
+
+    #[error("Bootnode multiaddrs without peer ID: {0:?}")]
+    BootnodeAddrsWithoutPeerId(Vec<Multiaddr>),
 }
 
 impl From<oneshot::error::RecvError> for P2pError {
@@ -102,7 +105,7 @@ where
 {
     pub network_id: String,
     pub local_keypair: Keypair,
-    pub bootstrap_peers: Vec<Multiaddr>,
+    pub bootnodes: Vec<Multiaddr>,
     pub listen_on: Vec<Multiaddr>,
     pub store: Arc<S>,
 }
@@ -136,7 +139,9 @@ impl<S> P2p<S>
 where
     S: Store,
 {
-    pub fn start(args: P2pArgs<S>) -> Result<Self, P2pError> {
+    pub fn start(args: P2pArgs<S>) -> Result<Self> {
+        validate_bootnode_addrs(&args.bootnodes)?;
+
         let local_peer_id = PeerId::from(args.local_keypair.public());
         let transport = new_transport(&args.local_keypair)?;
 
@@ -408,7 +413,7 @@ where
             swarm.listen_on(addr)?;
         }
 
-        for addr in args.bootstrap_peers {
+        for addr in args.bootnodes {
             // Bootstrap peers are always trusted
             if let Some(peer_id) = addr.peer_id() {
                 peer_tracker.set_trusted(peer_id, true);
@@ -721,6 +726,22 @@ where
     }
 }
 
+fn validate_bootnode_addrs(addrs: &[Multiaddr]) -> Result<(), P2pError> {
+    let mut invalid_addrs = Vec::new();
+
+    for addr in addrs {
+        if addr.peer_id().is_none() {
+            invalid_addrs.push(addr.to_owned());
+        }
+    }
+
+    if invalid_addrs.is_empty() {
+        Ok(())
+    } else {
+        Err(P2pError::BootnodeAddrsWithoutPeerId(invalid_addrs))
+    }
+}
+
 fn init_gossipsub<'a, S>(
     args: &'a P2pArgs<S>,
     topics: impl IntoIterator<Item = &'a gossipsub::IdentTopic>,
@@ -762,7 +783,7 @@ where
 
     let mut kad = Kademlia::with_config(local_peer_id, MemoryStore::new(local_peer_id), config);
 
-    for addr in &args.bootstrap_peers {
+    for addr in &args.bootnodes {
         if let Some(peer_id) = addr.peer_id() {
             kad.add_address(&peer_id, addr.to_owned());
         }

node/src/test_utils.rs

@@ -37,7 +37,7 @@ pub fn test_node_config() -> NodeConfig<InMemoryStore> {
         network_id: "private".to_string(),
         genesis_hash: None,
         p2p_local_keypair: node_keypair,
-        p2p_bootstrap_peers: vec![],
+        p2p_bootnodes: vec![],
         p2p_listen_on: vec![],
         store: InMemoryStore::new(),
     }

node/tests/exchange.rs

@@ -33,7 +33,7 @@ async fn client_server() {
 
     // Client node
     let client = Node::new(NodeConfig {
-        p2p_bootstrap_peers: server_addrs.clone(),
+        p2p_bootnodes: server_addrs.clone(),
         ..test_node_config()
     })
     .await
@@ -119,7 +119,7 @@ async fn client_server_invalid_requests() {
 
     // Client node
     let client = Node::new(NodeConfig {
-        p2p_bootstrap_peers: server_addrs.clone(),
+        p2p_bootnodes: server_addrs.clone(),
         ..test_node_config()
     })
     .await
@@ -224,7 +224,7 @@ async fn head_selection_with_multiple_peers() {
 
     // Client Node
     let client = Node::new(NodeConfig {
-        p2p_bootstrap_peers: server_addrs,
+        p2p_bootnodes: server_addrs,
         ..listening_test_node_config()
     })
     .await
@@ -239,7 +239,7 @@ async fn head_selection_with_multiple_peers() {
     // Rogue node, connects to client so isn't trusted
     let rogue_node = Node::new(NodeConfig {
         store: gen_filled_store(26).0,
-        p2p_bootstrap_peers: client_addr.clone(),
+        p2p_bootnodes: client_addr.clone(),
         ..listening_test_node_config()
     })
     .await
@@ -256,7 +256,7 @@ async fn head_selection_with_multiple_peers() {
     // new node from group B joins, head should go up
     let new_b_node = Node::new(NodeConfig {
         store: server_store.clone(),
-        p2p_bootstrap_peers: client_addr,
+        p2p_bootnodes: client_addr,
         ..test_node_config()
     })
     .await
@@ -306,7 +306,7 @@ async fn replaced_header_server_store() {
     let server_addrs = server.p2p().listeners().await.unwrap();
 
     let client = Node::new(NodeConfig {
-        p2p_bootstrap_peers: server_addrs,
+        p2p_bootnodes: server_addrs,
         ..listening_test_node_config()
     })
     .await
@@ -380,7 +380,7 @@ async fn invalidated_header_server_store() {
     let server_addrs = server.p2p().listeners().await.unwrap();
 
     let client = Node::new(NodeConfig {
-        p2p_bootstrap_peers: server_addrs,
+        p2p_bootnodes: server_addrs,
         ..listening_test_node_config()
     })
     .await
@@ -461,7 +461,7 @@ async fn unverified_header_server_store() {
     let server_addrs = server.p2p().listeners().await.unwrap();
 
     let client = Node::new(NodeConfig {
-        p2p_bootstrap_peers: server_addrs,
+        p2p_bootnodes: server_addrs,
         ..listening_test_node_config()
     })
     .await

node/tests/node.rs

@@ -38,7 +38,7 @@ async fn new_connected_node() -> Node<InMemoryStore> {
     let (_, bridge_ma) = fetch_bridge_info().await;
 
     let node = Node::new(NodeConfig {
-        p2p_bootstrap_peers: vec![bridge_ma],
+        p2p_bootnodes: vec![bridge_ma],
         ..test_node_config()
     })
     .await
@@ -122,7 +122,7 @@ async fn peer_discovery() {
     let node1_keypair = identity::Keypair::generate_ed25519();
     let node1_peer_id = PeerId::from(node1_keypair.public());
     let node1 = Node::new(NodeConfig {
-        p2p_bootstrap_peers: vec![bridge_ma],
+        p2p_bootnodes: vec![bridge_ma],
         p2p_listen_on: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()],
         ..test_node_config_with_keypair(node1_keypair)
     })
@@ -139,7 +139,7 @@ async fn peer_discovery() {
     let node2_keypair = identity::Keypair::generate_ed25519();
     let node2_peer_id = PeerId::from(node2_keypair.public());
     let node2 = Node::new(NodeConfig {
-        p2p_bootstrap_peers: node1_addrs.clone(),
+        p2p_bootnodes: node1_addrs.clone(),
         p2p_listen_on: vec!["/ip4/127.0.0.1/tcp/0".parse().unwrap()],
         ..test_node_config_with_keypair(node2_keypair)
     })
@@ -154,7 +154,7 @@ async fn peer_discovery() {
     let node3_keypair = identity::Keypair::generate_ed25519();
     let node3_peer_id = PeerId::from(node3_keypair.public());
     let node3 = Node::new(NodeConfig {
-        p2p_bootstrap_peers: node1_addrs.clone(),
+        p2p_bootnodes: node1_addrs.clone(),
         ..test_node_config_with_keypair(node3_keypair)
     })
     .await