Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(kad): derive Copy for kbucket::key::Key #5317

Merged
merged 6 commits into from
Apr 19, 2024
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
2 changes: 2 additions & 0 deletions protocols/kad/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
See [PR 5122](https://github.com/libp2p/rust-libp2p/pull/5122).
- Compute `jobs_query_capacity` accurately.
See [PR 5148](https://github.com/libp2p/rust-libp2p/pull/5148).
- Derive `Copy` for `kbucket::key::Key<T>`.
See [PR 5317](https://github.com/libp2p/rust-libp2p/pull/5317).

## 0.45.3

Expand Down
10 changes: 5 additions & 5 deletions protocols/kad/src/behaviour.rs
Original file line number Diff line number Diff line change
Expand Up @@ -931,7 +931,7 @@ where
/// This parameter is used to call [`Behaviour::bootstrap`] periodically and automatically
/// to ensure a healthy routing table.
pub fn bootstrap(&mut self) -> Result<QueryId, NoKnownPeers> {
let local_key = self.kbuckets.local_key().clone();
let local_key = *self.kbuckets.local_key();
let info = QueryInfo::Bootstrap {
peer: *local_key.preimage(),
remaining: None,
Expand Down Expand Up @@ -1396,7 +1396,7 @@ where
remaining,
mut step,
} => {
let local_key = self.kbuckets.local_key().clone();
let local_key = *self.kbuckets.local_key();
let mut remaining = remaining.unwrap_or_else(|| {
debug_assert_eq!(&peer, local_key.preimage());
// The lookup for the local key finished. To complete the bootstrap process,
Expand Down Expand Up @@ -1446,7 +1446,7 @@ where
let peers = self.kbuckets.closest_keys(&target);
let inner = QueryInner::new(info);
self.queries
.continue_iter_closest(query_id, target.clone(), peers, inner);
.continue_iter_closest(query_id, target, peers, inner);
} else {
step.last = true;
self.bootstrap_status.on_finish();
Expand Down Expand Up @@ -1642,14 +1642,14 @@ where
remaining.take().and_then(|mut r| Some((r.next()?, r)))
{
let info = QueryInfo::Bootstrap {
peer: target.clone().into_preimage(),
peer: target.into_preimage(),
remaining: Some(remaining),
step: step.next(),
};
let peers = self.kbuckets.closest_keys(&target);
let inner = QueryInner::new(info);
self.queries
.continue_iter_closest(query_id, target.clone(), peers, inner);
.continue_iter_closest(query_id, target, peers, inner);
} else {
step.last = true;
self.bootstrap_status.on_finish();
Expand Down
18 changes: 6 additions & 12 deletions protocols/kad/src/kbucket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ mod tests {
fn arbitrary(g: &mut Gen) -> TestTable {
let local_key = Key::from(PeerId::random());
let timeout = Duration::from_secs(g.gen_range(1..360));
let mut table = TestTable::new(local_key.clone().into(), timeout);
let mut table = TestTable::new(local_key.into(), timeout);
let mut num_total = g.gen_range(0..100);
for (i, b) in &mut table.buckets.iter_mut().enumerate().rev() {
let ix = BucketIndex(i);
Expand All @@ -543,10 +543,7 @@ mod tests {
for _ in 0..num {
let distance = ix.rand_distance(&mut rand::thread_rng());
let key = local_key.for_distance(distance);
let node = Node {
key: key.clone(),
value: (),
};
let node = Node { key, value: () };
let status = NodeStatus::arbitrary(g);
match b.insert(node, status) {
InsertResult::Inserted => {}
Expand Down Expand Up @@ -643,7 +640,7 @@ mod tests {
#[test]
fn entry_self() {
let local_key = Key::from(PeerId::random());
let mut table = KBucketsTable::<_, ()>::new(local_key.clone(), Duration::from_secs(5));
let mut table = KBucketsTable::<_, ()>::new(local_key, Duration::from_secs(5));

assert!(table.entry(&local_key).is_none())
}
Expand Down Expand Up @@ -671,7 +668,7 @@ mod tests {
let mut expected_keys: Vec<_> = table
.buckets
.iter()
.flat_map(|t| t.iter().map(|(n, _)| n.key.clone()))
.flat_map(|t| t.iter().map(|(n, _)| n.key))
.collect();

for _ in 0..10 {
Expand All @@ -686,7 +683,7 @@ mod tests {
#[test]
fn applied_pending() {
let local_key = Key::from(PeerId::random());
let mut table = KBucketsTable::<_, ()>::new(local_key.clone(), Duration::from_millis(1));
let mut table = KBucketsTable::<_, ()>::new(local_key, Duration::from_millis(1));
let expected_applied;
let full_bucket_index;
loop {
Expand All @@ -698,10 +695,7 @@ mod tests {
match e.insert((), NodeStatus::Connected) {
InsertResult::Pending { disconnected } => {
expected_applied = AppliedPending {
inserted: Node {
key: key.clone(),
value: (),
},
inserted: Node { key, value: () },
evicted: Some(Node {
key: disconnected,
value: (),
Expand Down
41 changes: 10 additions & 31 deletions protocols/kad/src/kbucket/bucket.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,10 +439,7 @@ mod tests {
let num_nodes = g.gen_range(1..K_VALUE.get() + 1);
for _ in 0..num_nodes {
let key = Key::from(PeerId::random());
let node = Node {
key: key.clone(),
value: (),
};
let node = Node { key, value: () };
let status = NodeStatus::arbitrary(g);
match bucket.insert(node, status) {
InsertResult::Inserted => {}
Expand Down Expand Up @@ -492,10 +489,7 @@ mod tests {
// Fill the bucket, thereby populating the expected lists in insertion order.
for status in status {
let key = Key::from(PeerId::random());
let node = Node {
key: key.clone(),
value: (),
};
let node = Node { key, value: () };
let full = bucket.num_entries() == K_VALUE.get();
if let InsertResult::Inserted = bucket.insert(node, status) {
let vec = match status {
Expand All @@ -505,15 +499,12 @@ mod tests {
if full {
vec.pop_front();
}
vec.push_back((status, key.clone()));
vec.push_back((status, key));
}
}

// Get all nodes from the bucket, together with their status.
let mut nodes = bucket
.iter()
.map(|(n, s)| (s, n.key.clone()))
.collect::<Vec<_>>();
let mut nodes = bucket.iter().map(|(n, s)| (s, n.key)).collect::<Vec<_>>();

// Split the list of nodes at the first connected node.
let first_connected_pos = nodes.iter().position(|(s, _)| *s == NodeStatus::Connected);
Expand Down Expand Up @@ -553,10 +544,7 @@ mod tests {
// Add a connected node, which is expected to be pending, scheduled to
// replace the first (i.e. least-recently connected) node.
let key = Key::from(PeerId::random());
let node = Node {
key: key.clone(),
value: (),
};
let node = Node { key, value: () };
match bucket.insert(node.clone(), NodeStatus::Connected) {
InsertResult::Pending { disconnected } => {
assert_eq!(disconnected, first_disconnected.key)
Expand Down Expand Up @@ -609,10 +597,7 @@ mod tests {

// Add a connected pending node.
let key = Key::from(PeerId::random());
let node = Node {
key: key.clone(),
value: (),
};
let node = Node { key, value: () };
if let InsertResult::Pending { disconnected } = bucket.insert(node, NodeStatus::Connected) {
assert_eq!(&disconnected, &first_disconnected.key);
} else {
Expand Down Expand Up @@ -647,13 +632,10 @@ mod tests {

// Capture position and key of the random node to update.
let pos = pos.0 % num_nodes;
let key = bucket.nodes[pos].key.clone();
let key = bucket.nodes[pos].key;

// Record the (ordered) list of status of all nodes in the bucket.
let mut expected = bucket
.iter()
.map(|(n, s)| (n.key.clone(), s))
.collect::<Vec<_>>();
let mut expected = bucket.iter().map(|(n, s)| (n.key, s)).collect::<Vec<_>>();

// Update the node in the bucket.
bucket.update(&key, status);
Expand All @@ -665,11 +647,8 @@ mod tests {
NodeStatus::Disconnected => bucket.first_connected_pos.unwrap_or(num_nodes) - 1,
};
expected.remove(pos);
expected.insert(expected_pos, (key.clone(), status));
let actual = bucket
.iter()
.map(|(n, s)| (n.key.clone(), s))
.collect::<Vec<_>>();
expected.insert(expected_pos, (key, status));
let actual = bucket.iter().map(|(n, s)| (n.key, s)).collect::<Vec<_>>();
expected == actual
}

Expand Down
4 changes: 2 additions & 2 deletions protocols/kad/src/kbucket/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ construct_uint! {
///
/// `Key`s have an XOR metric as defined in the Kademlia paper, i.e. the bitwise XOR of
/// the hash digests, interpreted as an integer. See [`Key::distance`].
#[derive(Clone, Debug)]
#[derive(Clone, Copy, Debug)]
pub struct Key<T> {
preimage: T,
bytes: KeyBytes,
Expand Down Expand Up @@ -145,7 +145,7 @@ impl<T> Hash for Key<T> {
}

/// The raw bytes of a key in the DHT keyspace.
#[derive(PartialEq, Eq, Clone, Debug)]
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
pub struct KeyBytes(GenericArray<u8, U32>);

impl KeyBytes {
Expand Down
11 changes: 5 additions & 6 deletions protocols/kad/src/query/peers/closest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,12 +556,12 @@ mod tests {
#[test]
fn new_iter() {
fn prop(iter: ClosestPeersIter) {
let target = iter.target.clone();
let target = iter.target;

let (keys, states): (Vec<_>, Vec<_>) = iter
.closest_peers
.values()
.map(|e| (e.key.clone(), &e.state))
.map(|e| (e.key, &e.state))
.unzip();

let none_contacted = states.iter().all(|s| matches!(s, PeerState::NotContacted));
Expand Down Expand Up @@ -595,12 +595,12 @@ mod tests {
let mut expected = iter
.closest_peers
.values()
.map(|e| e.key.clone())
.map(|e| e.key)
.collect::<Vec<_>>();
let num_known = expected.len();
let max_parallelism = usize::min(iter.config.parallelism.get(), num_known);

let target = iter.target.clone();
let target = iter.target;
let mut remaining;
let mut num_failures = 0;

Expand Down Expand Up @@ -667,7 +667,7 @@ mod tests {
.values()
.all(|e| !matches!(e.state, PeerState::NotContacted | PeerState::Waiting { .. }));

let target = iter.target.clone();
let target = iter.target;
let num_results = iter.config.num_results;
let result = iter.into_result();
let closest = result.map(Key::from).collect::<Vec<_>>();
Expand Down Expand Up @@ -744,7 +744,6 @@ mod tests {
.next()
.unwrap()
.key
.clone()
.into_preimage();

// Poll the iterator for the first peer to be in progress.
Expand Down
10 changes: 5 additions & 5 deletions protocols/kad/src/query/peers/closest/disjoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ mod tests {
peers.into_iter()
});

ResultIter::new(target.clone(), iters)
ResultIter::new(target, iters)
}

fn shrink(&self) -> Box<dyn Iterator<Item = Self>> {
Expand All @@ -481,7 +481,7 @@ mod tests {
.collect();

Box::new(ResultIterShrinker {
target: self.target.clone(),
target: self.target,
peers,
iters,
})
Expand Down Expand Up @@ -511,7 +511,7 @@ mod tests {
Some(iter.into_iter())
});

Some(ResultIter::new(self.target.clone(), iters))
Some(ResultIter::new(self.target, iters))
}
}

Expand Down Expand Up @@ -867,7 +867,7 @@ mod tests {
let closest = drive_to_finish(
PeerIterator::Closest(ClosestPeersIter::with_config(
cfg.clone(),
target.clone(),
target,
known_closest_peers.clone(),
)),
graph.clone(),
Expand All @@ -877,7 +877,7 @@ mod tests {
let disjoint = drive_to_finish(
PeerIterator::Disjoint(ClosestDisjointPeersIter::with_config(
cfg,
target.clone(),
target,
known_closest_peers.clone(),
)),
graph,
Expand Down
2 changes: 1 addition & 1 deletion protocols/kad/src/record/store/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl RecordStore for MemoryStore {
providers.as_mut()[i] = record;
} else {
// It is a new provider record for that key.
let local_key = self.local_key.clone();
let local_key = self.local_key;
let key = kbucket::Key::new(record.key.clone());
let provider = kbucket::Key::from(record.provider);
if let Some(i) = providers.iter().position(|p| {
Expand Down
Loading