protocols/kad: Expose kbucket distance range

[mxinden]

Instead of only returning non-empty kbuckets via Kademlia::kbuckets,
return empty ones as well.

Doing so enables consumers to (1) reason about each and every bucket and
(2) depend on a stable order of the buckets.

Add KBucketRef::range exposing the minimum inclusive and maximum
inclusive Distance for the bucket

More specifically within substrate I would like to expose the following metric:

HELP Number of nodes per protocol and Kademlia kbucket
substrate_sub_libp2p_kademlia_num_nodes_per_kbucket{protocol="sub", bucket="255"} 20

[romanb]

The only small issue I have with this is that it leaks implementation details. Ideally, the implementation strategy for the buckets should not leak in the public API, but that is what happens when not filtering out empty buckets - empty buckets only exist because of the 256-fixed-array implementation, e.g. I don't think they would be present in an implementation based on a dynamically growing "bucket-tree", what is actually suggested in the paper. Of course, one could artificially "fill up" the gaps in the iterator with empty buckets in that case, but even then there would no longer always be 256 buckets because the ranges covered by the buckets vary in that case, if I remember correctly.

Isn't what you're mainly interested in the distances covered by the buckets? The order should always be stable. Maybe exposing the ranges covered by each bucket would be sufficient? If you know which distance each non-empty bucket covers, you also know the "empty space" in-between.

[mxinden]

I agree that my suggestion leaks implementation details. E.g. making NUM_BUCKETS configurable in the future would be a breaking change.

@romanb are you suggesting a change along the lines of the following?

diff --git a/protocols/kad/src/kbucket.rs b/protocols/kad/src/kbucket.rs
index 2ea7f362..715eedde 100644
--- a/protocols/kad/src/kbucket.rs
+++ b/protocols/kad/src/kbucket.rs
@@ -447,6 +447,14 @@ where
     TKey: Clone + AsRef<KeyBytes>,
     TVal: Clone
 {
+    pub fn min_distance(&self) -> Distance {
+        unimplemented!()
+    }
+
+    pub fn max_distance(&self) -> Distance {
+        unimplemented!()
+    }
+

[romanb]

@mxinden Yes, something like that. Or just a single method along the lines of fn range(&self) -> (Distance, Distance).

[mxinden]

@romanb would you mind taking another look?

[romanb]

Feel free to update the changelog and merge at your convenience.