Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: multiaddr validation to add peer id for listening #2833

Merged

Conversation

vasco-santos
Copy link
Member

When we add the own peer id to a multiaddr, we should verify if it already has it, or if it is an address of circuit-relay / stardust. Otherwise, we might end up with multiaddrs like: /ip4/127.0.0.1/tcp/5892/ws/p2p/PEER_ID/p2p/PEER_ID

@vasco-santos vasco-santos force-pushed the fix/multiaddr-validation-to-add-peer-id-for-listening branch from e3a9919 to cccb0b2 Compare March 6, 2020 17:56
@achingbrain
Copy link
Member

Do you think this would be better done by only appending the peer id to certain types of multiaddrs, a la #2727 ?

@achingbrain achingbrain linked an issue Mar 9, 2020 that may be closed by this pull request
@vasco-santos
Copy link
Member Author

I haven't considered that since I did not know about the issue, but this is a good question!

This also needs to be considered for stardust, and possibly others in the future. This might make the validation of a growing list of different multiaddrs, which is my biggest concerns.

FYI, we currently consider multiaddr without p2p as valid: https://github.com/multiformats/js-mafmt/blob/master/src/index.js#L77-L96

@achingbrain achingbrain merged commit 78cbec1 into master Mar 27, 2020
@achingbrain achingbrain deleted the fix/multiaddr-validation-to-add-peer-id-for-listening branch March 27, 2020 13:41
mistakia pushed a commit to mistakia/js-ipfs that referenced this pull request Apr 3, 2020
* fix: multiaddr validation to add peer id for listening
* chore: update packages/ipfs/src/core/components/start.js

Co-authored-by: Alex Potsides <[email protected]>
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.

Use mafmt module to only append peer ids to certain multiaddrs
2 participants