Skip to content

Commit

Permalink
feat(kad): derive Copy for kbucket::key::Key
Browse files Browse the repository at this point in the history
Derive `Copy` for `kbucket::key::Key` and `kbucket::key::KeyBytes` because `Key` is almost always used with `PeerId`, which is `Copy`.
This will make it easier to iterate over kbuckets.

Pull-Request: #5317.
  • Loading branch information
drHuangMHT committed Apr 19, 2024
1 parent 9a6eccd commit 31457f6
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 62 deletions.
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

0 comments on commit 31457f6

Please sign in to comment.