diff --git a/Cargo.lock b/Cargo.lock index e10988489e201..c558d5af7ee6c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3726,6 +3726,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6245d59a3e82a7fc217c5828a6692dbc6dfb63a0c8c90495621f7b9d79704a0e" +[[package]] +name = "convert_case" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec182b0ca2f35d8fc196cf3404988fd8b8c739a4d270ff118a398feb0cbec1ca" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "convert_case" version = "0.7.1" @@ -4346,7 +4355,7 @@ dependencies = [ "num-traits", "parity-scale-codec", "prost 0.12.6", - "prost-build", + "prost-build 0.13.2", "sc-network", "sc-service", "sp-consensus-babe", @@ -6096,6 +6105,26 @@ dependencies = [ "syn 2.0.98", ] +[[package]] +name = "enum-display" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "02058bb25d8d0605829af88230427dd5cd50661590bd2b09d1baf7c64c417f24" +dependencies = [ + "enum-display-macro", +] + +[[package]] +name = "enum-display-macro" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4be2cf2fe7b971b1865febbacd4d8df544aa6bd377cca011a6d69dcf4c60d94" +dependencies = [ + "convert_case 0.6.0", + "quote 1.0.40", + "syn 1.0.109", +] + [[package]] name = "enum-ordinalize" version = "4.3.0" @@ -8760,15 +8789,6 @@ dependencies = [ "either", ] -[[package]] -name = "itertools" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba291022dbbd398a455acf126c1e341954079855bc60dfdda641363bd6922569" -dependencies = [ - "either", -] - [[package]] name = "itertools" version = "0.13.0" @@ -9849,7 +9869,7 @@ dependencies = [ "thiserror 1.0.65", "tracing", "yamux 0.12.1", - "yamux 0.13.5", + "yamux 0.13.8", ] [[package]] @@ -10026,15 +10046,16 @@ checksum = "4ee93343901ab17bd981295f2cf0026d4ad018c7c31ba84549a4ddbb47a45104" [[package]] name = "litep2p" -version = "0.11.0" +version = "0.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "766f82624163f63b3df107fcdd014acb28fc9e4990d2a70e56dc454287fd4565" +checksum = "1da54ffe750994080fe48ccf5dd298a528406b26b3b544032ae9505ff4d7cbea" dependencies = [ "async-trait", "bs58", "bytes", "cid 0.11.1", "ed25519-dalek", + "enum-display", "futures", "futures-timer", "hickory-resolver 0.25.2", @@ -10047,7 +10068,7 @@ dependencies = [ "parking_lot 0.12.3", "pin-project", "prost 0.13.5", - "prost-build", + "prost-build 0.14.1", "rand 0.8.5", "ring 0.17.14", "serde", @@ -10067,7 +10088,7 @@ dependencies = [ "url", "x25519-dalek", "x509-parser 0.17.0", - "yamux 0.13.5", + "yamux 0.13.8", "yasna", "zeroize", ] @@ -18046,6 +18067,16 @@ dependencies = [ "prost-derive 0.13.5", ] +[[package]] +name = "prost" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7231bd9b3d3d33c86b58adbac74b5ec0ad9f496b19d22801d773636feaa95f3d" +dependencies = [ + "bytes", + "prost-derive 0.14.1", +] + [[package]] name = "prost-build" version = "0.13.2" @@ -18061,7 +18092,27 @@ dependencies = [ "petgraph", "prettyplease", "prost 0.13.5", - "prost-types", + "prost-types 0.13.2", + "regex", + "syn 2.0.98", + "tempfile", +] + +[[package]] +name = "prost-build" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac6c3320f9abac597dcbc668774ef006702672474aad53c6d596b62e487b40b1" +dependencies = [ + "heck 0.5.0", + "itertools 0.14.0", + "log", + "multimap", + "once_cell", + "petgraph", + "prettyplease", + "prost 0.14.1", + "prost-types 0.14.1", "regex", "syn 2.0.98", "tempfile", @@ -18087,7 +18138,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81bddcdb20abf9501610992b6759a4c888aef7d1a7247ef75e2404275ac24af1" dependencies = [ "anyhow", - "itertools 0.12.1", + "itertools 0.11.0", "proc-macro2 1.0.95", "quote 1.0.40", "syn 2.0.98", @@ -18106,6 +18157,19 @@ dependencies = [ "syn 2.0.98", ] +[[package]] +name = "prost-derive" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9120690fafc389a67ba3803df527d0ec9cbbc9cc45e4cc20b332996dfb672425" +dependencies = [ + "anyhow", + "itertools 0.14.0", + "proc-macro2 1.0.95", + "quote 1.0.40", + "syn 2.0.98", +] + [[package]] name = "prost-types" version = "0.13.2" @@ -18115,6 +18179,15 @@ dependencies = [ "prost 0.13.5", ] +[[package]] +name = "prost-types" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9b4db3d6da204ed77bb26ba83b6122a73aeb2e87e25fbf7ad2e84c4ccbf8f72" +dependencies = [ + "prost 0.14.1", +] + [[package]] name = "pulley-interpreter" version = "35.0.0" @@ -19740,7 +19813,7 @@ dependencies = [ "log", "parity-scale-codec", "prost 0.12.6", - "prost-build", + "prost-build 0.13.2", "quickcheck", "rand 0.8.5", "sc-client-api", @@ -20476,7 +20549,7 @@ dependencies = [ "partial_sort", "pin-project", "prost 0.12.6", - "prost-build", + "prost-build 0.13.2", "rand 0.8.5", "sc-block-builder", "sc-client-api", @@ -20550,7 +20623,7 @@ dependencies = [ "log", "parity-scale-codec", "prost 0.12.6", - "prost-build", + "prost-build 0.13.2", "sc-client-api", "sc-network", "sc-network-types", @@ -20594,7 +20667,7 @@ dependencies = [ "mockall", "parity-scale-codec", "prost 0.12.6", - "prost-build", + "prost-build 0.13.2", "quickcheck", "sc-block-builder", "sc-client-api", @@ -28448,9 +28521,9 @@ dependencies = [ [[package]] name = "yamux" -version = "0.13.5" +version = "0.13.8" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3da1acad1c2dc53f0dde419115a38bd8221d8c3e47ae9aeceaf453266d29307e" +checksum = "deab71f2e20691b4728b349c6cee8fc7223880fa67b6b4f92225ec32225447e5" dependencies = [ "futures", "log", diff --git a/Cargo.toml b/Cargo.toml index 0869d70e7f602..7d7a57cece4c9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -908,7 +908,7 @@ linked-hash-map = { version = "0.5.4" } linked_hash_set = { version = "0.1.4" } linregress = { version = "0.5.1" } lite-json = { version = "0.2.0", default-features = false } -litep2p = { version = "0.11.0", features = ["rsa", "websocket"] } +litep2p = { version = "0.12.0", features = ["rsa", "websocket"] } log = { version = "0.4.22", default-features = false } macro_magic = { version = "0.5.1" } maplit = { version = "1.0.2" } diff --git a/prdoc/pr_9685.prdoc b/prdoc/pr_9685.prdoc new file mode 100644 index 0000000000000..e3b416c3ae20d --- /dev/null +++ b/prdoc/pr_9685.prdoc @@ -0,0 +1,12 @@ +title: Upgrade litep2p to v0.12.0 +doc: +- audience: Node Dev + description: litep2p v0.12.0 adds ability to track whether publishing a DHT record + or provider was successful. This PR brings this functionality to substrate. Particularly, + this fixes authority-discovery unnecessarily republishing DHT records due to litep2p + not emitting `KademliaEvent::PutRecordSuccess` before v0.12.0. +crates: +- name: sc-network + bump: major +- name: sc-network-types + bump: major diff --git a/substrate/client/network/src/event.rs b/substrate/client/network/src/event.rs index f5a4b92aef5e3..bb4fa921c9493 100644 --- a/substrate/client/network/src/event.rs +++ b/substrate/client/network/src/event.rs @@ -48,14 +48,12 @@ pub enum DhtEvent { ValueNotFound(Key), /// The record has been successfully inserted into the DHT. - // TODO: this is not implemented with litep2p network backend. ValuePut(Key), /// An error has occurred while putting a record into the DHT. ValuePutFailed(Key), /// Successfully started providing the given key. - // TODO: this is not implemented with litep2p network backend. StartedProviding(Key), /// An error occured while registering as a content provider on the DHT. diff --git a/substrate/client/network/src/litep2p/discovery.rs b/substrate/client/network/src/litep2p/discovery.rs index 4a9b84b3a215a..b727688f5b514 100644 --- a/substrate/client/network/src/litep2p/discovery.rs +++ b/substrate/client/network/src/litep2p/discovery.rs @@ -80,6 +80,16 @@ const MAX_EXTERNAL_ADDRESSES: u32 = 32; /// external. const MIN_ADDRESS_CONFIRMATIONS: usize = 3; +/// Quorum threshold to interpret `PUT_VALUE` & `ADD_PROVIDER` as successful. +/// +/// As opposed to libp2p, litep2p does not finish the query as soon as the required number of +/// peers have reached. Instead, it tries to put the record to all target peers (typically 20) and +/// uses the quorum setting only to determine the success of the query. +/// +/// We set the threshold to 50% of the target peers to account for unreachable peers. The actual +/// number of stored records may be higher. +const QUORUM_THRESHOLD: NonZeroUsize = NonZeroUsize::new(10).expect("10 > 0; qed"); + /// Discovery events. #[derive(Debug)] pub enum DiscoveryEvent { @@ -174,6 +184,14 @@ pub enum DiscoveryEvent { providers: Vec, }, + /// Provider was successfully published. + AddProviderSuccess { + /// Query ID. + query_id: QueryId, + /// Provided key. + provided_key: RecordKey, + }, + /// Query failed. QueryFailed { /// Query ID. @@ -401,7 +419,10 @@ impl Discovery { /// Publish value on the DHT using Kademlia `PUT_VALUE`. pub async fn put_value(&mut self, key: KademliaKey, value: Vec) -> QueryId { self.kademlia_handle - .put_record(Record::new(RecordKey::new(&key.to_vec()), value)) + .put_record( + Record::new(RecordKey::new(&key.to_vec()), value), + Quorum::N(QUORUM_THRESHOLD), + ) .await } @@ -417,6 +438,9 @@ impl Discovery { record, peers.into_iter().map(|peer| peer.into()).collect(), update_local_storage, + // These are the peers that just returned the record to us in authority-discovery, + // so we assume they are all reachable. + Quorum::All, ) .await } @@ -446,8 +470,10 @@ impl Discovery { } /// Start providing `key`. - pub async fn start_providing(&mut self, key: KademliaKey) { - self.kademlia_handle.start_providing(key.into()).await; + pub async fn start_providing(&mut self, key: KademliaKey) -> QueryId { + self.kademlia_handle + .start_providing(key.into(), Quorum::N(QUORUM_THRESHOLD)) + .await } /// Stop providing `key`. @@ -680,6 +706,17 @@ impl Stream for Discovery { providers, })) }, + Poll::Ready(Some(KademliaEvent::AddProviderSuccess { query_id, provided_key })) => { + log::trace!( + target: LOG_TARGET, + "`ADD_PROVIDER` for {query_id:?} with {provided_key:?} succeeded", + ); + + return Poll::Ready(Some(DiscoveryEvent::AddProviderSuccess { + query_id, + provided_key, + })) + }, // We do not validate incoming providers. Poll::Ready(Some(KademliaEvent::IncomingProvider { .. })) => {}, } diff --git a/substrate/client/network/src/litep2p/mod.rs b/substrate/client/network/src/litep2p/mod.rs index 95229ad5ce126..2c8a988294afb 100644 --- a/substrate/client/network/src/litep2p/mod.rs +++ b/substrate/client/network/src/litep2p/mod.rs @@ -153,6 +153,8 @@ enum KadQuery { PutValue(RecordKey, Instant), /// `GET_PROVIDERS` query for key and when it was initiated. GetProviders(RecordKey, Instant), + /// `ADD_PROVIDER` query for key and when it was initiated. + AddProvider(RecordKey, Instant), } /// Networking backend for `litep2p`. @@ -684,7 +686,8 @@ impl NetworkBackend for Litep2pNetworkBac self.discovery.store_record(key, value, publisher.map(Into::into), expires).await; } NetworkServiceCommand::StartProviding { key } => { - self.discovery.start_providing(key).await; + let query_id = self.discovery.start_providing(key.clone()).await; + self.pending_queries.insert(query_id, KadQuery::AddProvider(key, Instant::now())); } NetworkServiceCommand::StopProviding { key } => { self.discovery.stop_providing(key).await; @@ -967,6 +970,42 @@ impl NetworkBackend for Litep2pNetworkBac } } } + Some(DiscoveryEvent::AddProviderSuccess { query_id, provided_key }) => { + match self.pending_queries.remove(&query_id) { + Some(KadQuery::AddProvider(key, started)) => { + debug_assert_eq!(key, provided_key.into()); + + log::trace!( + target: LOG_TARGET, + "`ADD_PROVIDER` for {key:?} ({query_id:?}) succeeded", + ); + + self.event_streams.send(Event::Dht( + DhtEvent::StartedProviding(key.into()) + )); + + if let Some(ref metrics) = self.metrics { + metrics + .kademlia_query_duration + .with_label_values(&["provider-add"]) + .observe(started.elapsed().as_secs_f64()); + } + } + Some(_) => { + log::error!( + target: LOG_TARGET, + "Invalid pending query for `ADD_PROVIDER`: {query_id:?}" + ); + debug_assert!(false); + } + None => { + log::trace!( + target: LOG_TARGET, + "`ADD_PROVIDER` for key {provided_key:?} ({query_id:?}) succeeded (republishing)", + ); + } + } + } Some(DiscoveryEvent::QueryFailed { query_id }) => { match self.pending_queries.remove(&query_id) { Some(KadQuery::FindNode(peer_id, started)) => { @@ -1037,10 +1076,27 @@ impl NetworkBackend for Litep2pNetworkBac .observe(started.elapsed().as_secs_f64()); } }, + Some(KadQuery::AddProvider(key, started)) => { + log::debug!( + target: LOG_TARGET, + "`ADD_PROVIDER` ({query_id:?}) failed with key {key:?}", + ); + + self.event_streams.send(Event::Dht( + DhtEvent::StartProvidingFailed(key) + )); + + if let Some(ref metrics) = self.metrics { + metrics + .kademlia_query_duration + .with_label_values(&["provider-add-failed"]) + .observe(started.elapsed().as_secs_f64()); + } + }, None => { - log::warn!( + log::debug!( target: LOG_TARGET, - "non-existent query failed ({query_id:?})", + "non-existent query (likely republishing a provider) failed ({query_id:?})", ); } }