Skip to content

Commit 12acde3

Browse files
refactor: limit FindNodeReq responses to K_VALUE (#5971)
Change related to discussion: #5969 --------- Co-authored-by: guillaumemichel <[email protected]> Co-authored-by: Guillaume Michel <[email protected]>
1 parent 3875abb commit 12acde3

File tree

3 files changed

+65
-8
lines changed

3 files changed

+65
-8
lines changed

protocols/kad/src/behaviour.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ where
720720
where
721721
K: Into<kbucket::Key<K>> + Into<Vec<u8>> + Clone,
722722
{
723-
self.get_closest_peers_inner(key, None)
723+
self.get_closest_peers_inner(key, K_VALUE)
724724
}
725725

726726
/// Initiates an iterative query for the closest peers to the given key.
@@ -738,10 +738,10 @@ where
738738
// since it would involve forging a new key and additional requests.
739739
// Hence bound to K_VALUE here to set clear expectation and prevent unexpected behaviour.
740740
let capped_num_results = std::cmp::min(num_results, K_VALUE);
741-
self.get_closest_peers_inner(key, Some(capped_num_results))
741+
self.get_closest_peers_inner(key, capped_num_results)
742742
}
743743

744-
fn get_closest_peers_inner<K>(&mut self, key: K, num_results: Option<NonZeroUsize>) -> QueryId
744+
fn get_closest_peers_inner<K>(&mut self, key: K, num_results: NonZeroUsize) -> QueryId
745745
where
746746
K: Into<kbucket::Key<K>> + Into<Vec<u8>> + Clone,
747747
{
@@ -778,7 +778,7 @@ where
778778
self.kbuckets
779779
.closest(key)
780780
.filter(move |e| e.node.key.preimage() != source)
781-
.take(self.queries.config().replication_factor.get())
781+
.take(K_VALUE.get())
782782
.map(KadPeer::from)
783783
}
784784

@@ -3205,8 +3205,8 @@ pub enum QueryInfo {
32053205
key: Vec<u8>,
32063206
/// Current index of events.
32073207
step: ProgressStep,
3208-
/// If required, `num_results` specifies expected responding peers
3209-
num_results: Option<NonZeroUsize>,
3208+
/// Specifies expected number of responding peers
3209+
num_results: NonZeroUsize,
32103210
},
32113211

32123212
/// A (repeated) query initiated by [`Behaviour::get_providers`].

protocols/kad/src/behaviour/test.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,3 +1606,60 @@ fn get_providers_limit_n_2() {
16061606
fn get_providers_limit_n_5() {
16071607
get_providers_limit::<5>();
16081608
}
1609+
1610+
// Test that nodes respond with K amount of peers even when replication factor is set lower than K.
1611+
#[test]
1612+
fn get_closest_peers_should_return_up_to_k_peers() {
1613+
let k_value = K_VALUE.get();
1614+
1615+
// Rplication factor should not influence the amount of peers returned in `GetClosestPeers`.
1616+
for replication_factor in 5..k_value + 1 {
1617+
// Should be enough nodes for every node to have >= K nodes in their RT.
1618+
let num_of_nodes = 3 * k_value;
1619+
1620+
let mut cfg = Config::new(PROTOCOL_NAME);
1621+
cfg.set_replication_factor(NonZeroUsize::new(replication_factor).unwrap());
1622+
1623+
let swarms = build_connected_nodes_with_config(num_of_nodes, replication_factor - 1, cfg);
1624+
let mut swarms = swarms
1625+
.into_iter()
1626+
.map(|(_addr, swarm)| swarm)
1627+
.collect::<Vec<_>>();
1628+
1629+
// Ask first node to search for a random peer.
1630+
let search_target = PeerId::random();
1631+
swarms[0].behaviour_mut().get_closest_peers(search_target);
1632+
1633+
block_on(poll_fn(move |ctx| {
1634+
for swarm in &mut swarms {
1635+
loop {
1636+
match swarm.poll_next_unpin(ctx) {
1637+
Poll::Ready(Some(SwarmEvent::Behaviour(
1638+
Event::OutboundQueryProgressed {
1639+
result: QueryResult::GetClosestPeers(Ok(ok)),
1640+
..
1641+
},
1642+
))) => {
1643+
assert_eq!(&ok.key[..], search_target.to_bytes().as_slice());
1644+
// Verify that we get K_VALUE amount of peers even with lower
1645+
// replication factor.
1646+
assert_eq!(
1647+
ok.peers.len(),
1648+
k_value,
1649+
"Expected K_VALUE ({}) peers but got {}",
1650+
k_value,
1651+
ok.peers.len()
1652+
);
1653+
return Poll::Ready(());
1654+
}
1655+
// Ignore any other event.
1656+
Poll::Ready(Some(_)) => (),
1657+
e @ Poll::Ready(_) => panic!("Unexpected return value: {e:?}"),
1658+
Poll::Pending => break,
1659+
}
1660+
}
1661+
}
1662+
Poll::Pending
1663+
}))
1664+
}
1665+
}

protocols/kad/src/query.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,9 @@ impl QueryPool {
143143
{
144144
let num_results = match info {
145145
QueryInfo::GetClosestPeers {
146-
num_results: Some(val),
147-
..
146+
num_results: val, ..
148147
} => val,
148+
QueryInfo::Bootstrap { .. } => K_VALUE,
149149
_ => self.config.replication_factor,
150150
};
151151

0 commit comments

Comments
 (0)