-
Notifications
You must be signed in to change notification settings - Fork 446
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
fix: return closest peers from FIND_NODE #2499
Conversation
The `FIND_NODE` DHT operation should return the closest peers the node knows to the value. It does not need to return itself in the list because the calling peer already knows about it. Fixes #2450
Looks good, thanks @achingbrain ! |
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.
Looks good, left some comments in line
@@ -799,6 +799,37 @@ describe('KadDHT', () => { | |||
|
|||
expect(res).to.not.be.empty() | |||
}) | |||
|
|||
it('should not include itself in getClosestPeers PEER_RESPONSE', async function () { | |||
this.timeout(240 * 1000) |
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.
Just curious, why are we using a 240 second timeout?
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.
No specific reason, it was copied from the preceding test.
Co-authored-by: Chad Nehemiah <[email protected]>
Partial revert of #2499 If a node is queried for it's own peer id, return it's own peer info. This is necessary because since libp2p/go-libp2p-kad-dht#820 go-libp2p-kad-dht won't add a peer to it's routing tables that doesn't have any DHT peers that are KAD-futher from it's own ID already.
Partial revert of #2499 If a node is queried for it's own peer id, return it's own peer info. This is necessary because since libp2p/go-libp2p-kad-dht#820 go-libp2p-kad-dht won't add a peer to it's routing tables that doesn't have any DHT peers that are KAD-futher from it's own ID already.
The
FIND_NODE
DHT operation should return the closest peers the node knows to the value.It does not need to return itself in the list because the calling peer already knows about it.
Fixes #2450
Change checklist