-
Notifications
You must be signed in to change notification settings - Fork 984
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): implement common traits on RoutingUpdate
#4270
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Can you add a changelog entry and bump the version? See https://github.com/libp2p/rust-libp2p/blob/master/CONTRIBUTING.md#write-changelog-entries-for-user-facing-changes.
I am open to that but it will be a breaking change so we can't merge that immediately. Are you willing to submit a PR and explore the idea? |
Yeah, will look into it. p.s. Updated changelog and added more derivable traits |
RoutingUpdate
RoutingUpdate
RoutingUpdate
RoutingUpdate
I can't push to your branch unfortunately but you'll also have to update the lockfile and bump the version of Alternatively, you can apply this patch: Subject: [PATCH] Bump version in root manifest
---
Index: Cargo.lock
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Cargo.lock b/Cargo.lock
--- a/Cargo.lock (revision ae2b23056ea58467f8d322f789a62bdc8e06684f)
+++ b/Cargo.lock (revision 87a70ef5a5db7c4bd1278959eae53cf7fdd9aa71)
@@ -2813,7 +2813,7 @@
[[package]]
name = "libp2p-kad"
-version = "0.44.3"
+version = "0.44.4"
dependencies = [
"arrayvec",
"async-std",
Index: Cargo.toml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Cargo.toml b/Cargo.toml
--- a/Cargo.toml (revision ae2b23056ea58467f8d322f789a62bdc8e06684f)
+++ b/Cargo.toml (revision 87a70ef5a5db7c4bd1278959eae53cf7fdd9aa71)
@@ -73,7 +73,7 @@
libp2p-gossipsub = { version = "0.45.0", path = "protocols/gossipsub" }
libp2p-identify = { version = "0.43.0", path = "protocols/identify" }
libp2p-identity = { version = "0.2.2" }
-libp2p-kad = { version = "0.44.3", path = "protocols/kad" }
+libp2p-kad = { version = "0.44.4", path = "protocols/kad" }
libp2p-mdns = { version = "0.44.0", path = "protocols/mdns" }
libp2p-metrics = { version = "0.13.1", path = "misc/metrics" }
libp2p-mplex = { version = "0.40.0", path = "muxers/mplex" } |
@arsenron let us know in case you need the patch release cut rather sooner than later. |
A few weeks ago when I was debugging my wrong setup with kademlia, I could not conveniently debug `RoutingUpdate` after adding an address to the DHT. It would be nice to have a few derivable traits on a public enum. Pull-Request: #4270.
Description
A few weeks ago when I was debugging my wrong setup with kademlia, I could not conveniently debug
RoutingUpdate
after adding an address to the DHT. It would be nice to have a few derivable traits on a public enum.Notes & open questions
Maybe we could make
Kademlia::add_address
return Result, where could specify the reason why adding address to the DHT failed. Something likeResult<Success/Pending, BucketFull/SelfEntry>
Change checklist