libp2p / js-libp2p-mdns

libp2p MulticastDNS Peer Discovery
https://libp2p.io
Other
51 stars 17 forks source link

Service names #68

Closed richardschneider closed 6 years ago

richardschneider commented 6 years ago
richardschneider commented 6 years ago

@diasdavid had to make changes to the tests, please review them.

Also, how to add AppVeyor CI?

richardschneider commented 6 years ago

@diasdavid RTM

lidel commented 6 years ago

@richardschneider Hi! There is ongoing work on mDNS API for browser extensions and ipfs.local came up.

Is there a technical reason why this change was done, or was it just pure aesthetics? :)

Change the service name from _ipfs-discovery._udp to ipfs

Problem I: ipfs.local does not seem to follow DNS-SD spec (https://tools.ietf.org/html/rfc6763#section-7) which expects _ipfs._tcp.local for TCP and _ipfs._udp.local [sic!] for everything else. This breaks things, as we can't reuse Firefox code compatible with mentioned spec.

Problem II: go version seems to be using _ipfs-discovery._udp, which breaks mDNS interop between go and js versions: https://github.com/libp2p/go-libp2p/blob/f5e43430a1d0f4c1af97338537f3c9b0c7442354/p2p/discovery/mdns.go#L24

See https://github.com/mozilla/libdweb/issues/7#issuecomment-404926726 for more details.

cc @jacobheun

richardschneider commented 6 years ago

@lidel MDNS does not require DNS-SD nor does DNS-SD require MDNS. However, they can work togther.

I was wrong to use _ipfs.local it should be ipfs._tcp.local, thus making it comply with DNS-SD.

DNS-SD also requires a SRV and TXT resource record and expects A and/or AAAA records.

lidel commented 6 years ago

@richardschneider If we can comply with DNS-SD then we should, as it makes it more interoperable with how things work in Firefox and other tools.

I was going to PR to fix this, but not sure what value should we switch to. go-ipfs is using _ipfs-discovery._udp.local, shouldn't we use the same?

richardschneider commented 6 years ago

Let's move this discussion over to https://github.com/libp2p/libp2p/issues/28.

The current consensus is _ipfs._upd.local.