Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 96 additions & 23 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
12 changes: 12 additions & 0 deletions prdoc/pr_9685.prdoc
Original file line number Diff line number Diff line change
@@ -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
2 changes: 0 additions & 2 deletions substrate/client/network/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
43 changes: 40 additions & 3 deletions substrate/client/network/src/litep2p/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -174,6 +184,14 @@ pub enum DiscoveryEvent {
providers: Vec<ContentProvider>,
},

/// Provider was successfully published.
AddProviderSuccess {
/// Query ID.
query_id: QueryId,
/// Provided key.
provided_key: RecordKey,
},

/// Query failed.
QueryFailed {
/// Query ID.
Expand Down Expand Up @@ -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<u8>) -> 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
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We add here the return value.
Are there other places in the codebase which could be impacted with this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the only place this is called is in litep2p/mod.rs where we store the return value.

@lrubasze Thanks for the review!

self.kademlia_handle
.start_providing(key.into(), Quorum::N(QUORUM_THRESHOLD))
.await
}

/// Stop providing `key`.
Expand Down Expand Up @@ -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 { .. })) => {},
}
Expand Down
Loading
Loading