Skip to content
This repository was archived by the owner on Jul 21, 2023. It is now read-only.

Fix: send correct payload in ADD_PROVIDER RPC #127

Merged
merged 2 commits into from
May 30, 2019

Conversation

dirkmc
Copy link
Contributor

@dirkmc dirkmc commented May 29, 2019

In the payload for ADD_PROVIDER we were sending remote peers their own id instead of the local node's id. The tests still passed because on the receiving end we ignore the payload and just look at the connection to get the provider peer id.

@dirkmc
Copy link
Contributor Author

dirkmc commented May 29, 2019

Note that the go implementation of the DHT does reject ADD_PROVIDER messages where the peer ID in the payload does not match the remote peer id, so this fix should improve provides

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the fix @dirkmc

@vasco-santos vasco-santos merged commit 8d92d5a into master May 30, 2019
@vasco-santos vasco-santos deleted the fix/add-provider-payload branch May 30, 2019 09:06
@daviddias
Copy link
Member

daviddias commented May 30, 2019

Excellent catch, @dirkmc !!

Are you adding a tiny interop test to ipfs/interop that checks js-ipfs providing to a go-ipfs network?

@dirkmc
Copy link
Contributor Author

dirkmc commented May 30, 2019

That's a good idea, I will add a test once this is merged: ipfs/interop#36

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants