diff --git a/prdoc/pr_6109.prdoc b/prdoc/pr_6109.prdoc new file mode 100644 index 0000000000000..f9a8c48dcd064 --- /dev/null +++ b/prdoc/pr_6109.prdoc @@ -0,0 +1,14 @@ +# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0 +# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json + +title: Remove the legacy kademlia protocol support + +doc: + - audience: [ Node Dev, Node Operator ] + description: | + This PR removes the legacy /kad kademlia protocol. + Instead, nodes use the genesis based kademlia protocol for the discovery process. + +crates: + - name: sc-network + bump: patch diff --git a/substrate/client/network/src/discovery.rs b/substrate/client/network/src/discovery.rs index 86c66c22701cc..0afb4ede94079 100644 --- a/substrate/client/network/src/discovery.rs +++ b/substrate/client/network/src/discovery.rs @@ -46,7 +46,7 @@ //! active mechanism that asks nodes for the addresses they are listening on. Whenever we learn //! of a node's address, you must call `add_self_reported_address`. -use crate::{config::ProtocolId, utils::LruHashSet}; +use crate::utils::LruHashSet; use array_bytes::bytes2hex; use futures::prelude::*; @@ -111,7 +111,6 @@ pub struct DiscoveryConfig { enable_mdns: bool, kademlia_disjoint_query_paths: bool, kademlia_protocol: Option, - kademlia_legacy_protocol: Option, kademlia_replication_factor: NonZeroUsize, } @@ -128,7 +127,6 @@ impl DiscoveryConfig { enable_mdns: false, kademlia_disjoint_query_paths: false, kademlia_protocol: None, - kademlia_legacy_protocol: None, kademlia_replication_factor: NonZeroUsize::new(DEFAULT_KADEMLIA_REPLICATION_FACTOR) .expect("value is a constant; constant is non-zero; qed."), } @@ -182,10 +180,8 @@ impl DiscoveryConfig { &mut self, genesis_hash: Hash, fork_id: Option<&str>, - protocol_id: &ProtocolId, ) -> &mut Self { self.kademlia_protocol = Some(kademlia_protocol_name(genesis_hash, fork_id)); - self.kademlia_legacy_protocol = Some(legacy_kademlia_protocol_name(protocol_id)); self } @@ -214,7 +210,6 @@ impl DiscoveryConfig { enable_mdns, kademlia_disjoint_query_paths, kademlia_protocol, - kademlia_legacy_protocol, kademlia_replication_factor, } = self; @@ -222,15 +217,10 @@ impl DiscoveryConfig { let mut config = KademliaConfig::default(); config.set_replication_factor(kademlia_replication_factor); - // Populate kad with both the legacy and the new protocol names. - // Remove the legacy protocol: - // https://github.com/paritytech/polkadot-sdk/issues/504 - let kademlia_protocols = if let Some(legacy_protocol) = kademlia_legacy_protocol { - vec![kademlia_protocol.clone(), legacy_protocol] - } else { - vec![kademlia_protocol.clone()] - }; - config.set_protocol_names(kademlia_protocols.into_iter().map(Into::into).collect()); + + config.set_protocol_names( + std::iter::once(kademlia_protocol.clone()).map(Into::into).collect(), + ); config.set_record_filtering(libp2p::kad::StoreInserts::FilterBoth); @@ -329,9 +319,6 @@ pub struct DiscoveryBehaviour { /// to these peers. records_to_publish: HashMap, /// The chain based kademlia protocol name (including genesis hash and fork id). - /// - /// Remove when all nodes are upgraded to genesis hash and fork ID-based Kademlia: - /// . kademlia_protocol: Option, } @@ -391,10 +378,6 @@ impl DiscoveryBehaviour { } // The supported protocols must include the chain-based Kademlia protocol. - // - // Extract the chain-based Kademlia protocol from `kademlia.protocol_name()` - // when all nodes are upgraded to genesis hash and fork ID-based Kademlia: - // https://github.com/paritytech/polkadot-sdk/issues/504. if !supported_protocols.iter().any(|p| { p == self .kademlia_protocol @@ -492,7 +475,7 @@ impl DiscoveryBehaviour { /// Returns the number of nodes in each Kademlia kbucket for each Kademlia instance. /// - /// Identifies Kademlia instances by their [`ProtocolId`] and kbuckets by the base 2 logarithm + /// Identifies Kademlia instances by their `ProtocolId` and kbuckets by the base 2 logarithm /// of their lower bound. pub fn num_entries_per_kbucket(&mut self) -> Option> { self.kademlia.as_mut().map(|kad| { @@ -1070,12 +1053,6 @@ impl NetworkBehaviour for DiscoveryBehaviour { } } -/// Legacy (fallback) Kademlia protocol name based on `protocol_id`. -fn legacy_kademlia_protocol_name(id: &ProtocolId) -> StreamProtocol { - let name = format!("/{}/kad", id.as_ref()); - StreamProtocol::try_from_owned(name).expect("protocol name is valid. qed") -} - /// Kademlia protocol name based on `genesis_hash` and `fork_id`. fn kademlia_protocol_name>( genesis_hash: Hash, @@ -1093,10 +1070,7 @@ fn kademlia_protocol_name>( #[cfg(test)] mod tests { - use super::{ - kademlia_protocol_name, legacy_kademlia_protocol_name, DiscoveryConfig, DiscoveryOut, - }; - use crate::config::ProtocolId; + use super::{kademlia_protocol_name, DiscoveryConfig, DiscoveryOut}; use futures::prelude::*; use libp2p::{ core::{ @@ -1124,7 +1098,6 @@ mod tests { let genesis_hash = H256::from_low_u64_be(1); let fork_id = Some("test-fork-id"); - let protocol_id = ProtocolId::from("dot"); // Build swarms whose behaviour is `DiscoveryBehaviour`, each aware of // the first swarm via `with_permanent_addresses`. @@ -1145,7 +1118,7 @@ mod tests { .allow_private_ip(true) .allow_non_globals_in_dht(true) .discovery_limit(50) - .with_kademlia(genesis_hash, fork_id, &protocol_id); + .with_kademlia(genesis_hash, fork_id); config.finish() }; @@ -1207,16 +1180,10 @@ mod tests { } }) .unwrap(); - // Test both genesis hash-based and legacy - // protocol names. - let protocol_names = if swarm_n % 2 == 0 { - vec![kademlia_protocol_name(genesis_hash, fork_id)] - } else { - vec![ - legacy_kademlia_protocol_name(&protocol_id), - kademlia_protocol_name(genesis_hash, fork_id), - ] - }; + + let protocol_names = + vec![kademlia_protocol_name(genesis_hash, fork_id)]; + swarms[swarm_n] .0 .behaviour_mut() @@ -1259,8 +1226,6 @@ mod tests { fn discovery_ignores_peers_with_unknown_protocols() { let supported_genesis_hash = H256::from_low_u64_be(1); let unsupported_genesis_hash = H256::from_low_u64_be(2); - let supported_protocol_id = ProtocolId::from("a"); - let unsupported_protocol_id = ProtocolId::from("b"); let mut discovery = { let keypair = Keypair::generate_ed25519(); @@ -1269,7 +1234,7 @@ mod tests { .allow_private_ip(true) .allow_non_globals_in_dht(true) .discovery_limit(50) - .with_kademlia(supported_genesis_hash, None, &supported_protocol_id); + .with_kademlia(supported_genesis_hash, None); config.finish() }; @@ -1280,7 +1245,6 @@ mod tests { let remote_peer_id = predictable_peer_id(b"00000000000000000000000000000001"); let remote_addr: Multiaddr = "/memory/1".parse().unwrap(); let another_peer_id = predictable_peer_id(b"00000000000000000000000000000002"); - let another_addr: Multiaddr = "/memory/2".parse().unwrap(); // Try adding remote peers with unsupported protocols. discovery.add_self_reported_address( @@ -1288,11 +1252,6 @@ mod tests { &[kademlia_protocol_name(unsupported_genesis_hash, None)], remote_addr.clone(), ); - discovery.add_self_reported_address( - &another_peer_id, - &[legacy_kademlia_protocol_name(&unsupported_protocol_id)], - another_addr.clone(), - ); { let kademlia = discovery.kademlia.as_mut().unwrap(); @@ -1328,63 +1287,5 @@ mod tests { "Expect peer with supported protocol to be added." ); } - - let unsupported_peer_id = predictable_peer_id(b"00000000000000000000000000000002"); - let unsupported_peer_addr: Multiaddr = "/memory/2".parse().unwrap(); - - // Check the unsupported peer is not present before and after the call. - { - let kademlia = discovery.kademlia.as_mut().unwrap(); - assert!( - kademlia - .kbucket(unsupported_peer_id) - .expect("Remote peer id not to be equal to local peer id.") - .is_empty(), - "Expect unsupported peer not to be added." - ); - } - // Note: legacy protocol is not supported without genesis hash and fork ID, - // if the legacy is the only protocol supported, then the peer will not be added. - discovery.add_self_reported_address( - &unsupported_peer_id, - &[legacy_kademlia_protocol_name(&supported_protocol_id)], - unsupported_peer_addr.clone(), - ); - { - let kademlia = discovery.kademlia.as_mut().unwrap(); - assert!( - kademlia - .kbucket(unsupported_peer_id) - .expect("Remote peer id not to be equal to local peer id.") - .is_empty(), - "Expect unsupported peer not to be added." - ); - } - - // Supported legacy and genesis based protocols are allowed to be added. - discovery.add_self_reported_address( - &another_peer_id, - &[ - legacy_kademlia_protocol_name(&supported_protocol_id), - kademlia_protocol_name(supported_genesis_hash, None), - ], - another_addr.clone(), - ); - - { - let kademlia = discovery.kademlia.as_mut().unwrap(); - assert_eq!( - 2, - kademlia.kbuckets().fold(0, |acc, bucket| acc + bucket.num_entries()), - "Expect peers with supported protocol to be added." - ); - assert!( - !kademlia - .kbucket(another_peer_id) - .expect("Remote peer id not to be equal to local peer id.") - .is_empty(), - "Expect peer with supported protocol to be added." - ); - } } } diff --git a/substrate/client/network/src/litep2p/discovery.rs b/substrate/client/network/src/litep2p/discovery.rs index 13cf8a4c6ee0c..414bda6af824b 100644 --- a/substrate/client/network/src/litep2p/discovery.rs +++ b/substrate/client/network/src/litep2p/discovery.rs @@ -19,7 +19,7 @@ //! libp2p-related discovery code for litep2p backend. use crate::{ - config::{NetworkConfiguration, ProtocolId}, + config::NetworkConfiguration, peer_store::PeerStoreProvider, }; @@ -207,11 +207,6 @@ pub struct Discovery { duration_to_next_find_query: Duration, } -/// Legacy (fallback) Kademlia protocol name based on `protocol_id`. -fn legacy_kademlia_protocol_name(id: &ProtocolId) -> ProtocolName { - ProtocolName::from(format!("/{}/kad", id.as_ref())) -} - /// Kademlia protocol name based on `genesis_hash` and `fork_id`. fn kademlia_protocol_name>( genesis_hash: Hash, @@ -236,7 +231,6 @@ impl Discovery { config: &NetworkConfiguration, genesis_hash: Hash, fork_id: Option<&str>, - protocol_id: &ProtocolId, known_peers: HashMap>, listen_addresses: Arc>>, _peerstore_handle: Arc, @@ -261,7 +255,6 @@ impl Discovery { let (kademlia_config, kademlia_handle) = { let protocol_names = vec![ kademlia_protocol_name(genesis_hash.clone(), fork_id), - legacy_kademlia_protocol_name(protocol_id), ]; KademliaConfigBuilder::new() diff --git a/substrate/client/network/src/litep2p/mod.rs b/substrate/client/network/src/litep2p/mod.rs index df4244890f967..e45c7a3b4d64d 100644 --- a/substrate/client/network/src/litep2p/mod.rs +++ b/substrate/client/network/src/litep2p/mod.rs @@ -543,7 +543,6 @@ impl NetworkBackend for Litep2pNetworkBac &network_config, params.genesis_hash, params.fork_id.as_deref(), - ¶ms.protocol_id, known_addresses.clone(), Arc::clone(&listen_addresses), Arc::clone(&peer_store_handle), diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs index 71d0b45aa06da..59d37d1886f10 100644 --- a/substrate/client/network/src/service.rs +++ b/substrate/client/network/src/service.rs @@ -519,11 +519,7 @@ where .collect::>(), ); config.discovery_limit(u64::from(network_config.default_peers_set.out_peers) + 15); - config.with_kademlia( - params.genesis_hash, - params.fork_id.as_deref(), - ¶ms.protocol_id, - ); + config.with_kademlia(params.genesis_hash, params.fork_id.as_deref()); config.with_dht_random_walk(network_config.enable_dht_random_walk); config.allow_non_globals_in_dht(network_config.allow_non_globals_in_dht); config.use_kademlia_disjoint_query_paths(