implement the new mDNS spec, run both implementations in parallel #1125
Conversation
marten-seemann
force-pushed
the
mdns
branch
3 times, most recently
from
623bfd9 to
c84d427
Compare
marten-seemann
force-pushed
the
mdns
branch
8 times, most recently
from
426446b to
53aaaec
Compare
| return out, nil | ||
| } | ||
|
|
||
| func NewMdnsServiceLegacy(ctx context.Context, peerhost host.Host, interval time.Duration) (Service, error) { |
There was a problem hiding this comment.
Should we make this private?
| lk sync.Mutex | ||
| notifees []Notifee | ||
| interval time.Duration | ||
| func NewMdnsServiceNew(host host.Host) *mdnsService { |
There was a problem hiding this comment.
... and this one as well?
|
So, I guess the question is, do we even need the "muxer" type? I'd consider:
|
|
@marten-seemann what is the easiest way for me to build a Golang binary that is running these changes to test interoperability with the Rust implementation? |
It's definitely cleaner code-wise, but it's also more disruptive. libp2p users will have to actively change their code if they want to maintain compatibility with libp2p instances that haven't updated yet. Assuming that people will have similar rollout and adoption rates as we're seeing in the IPFS network, they won't have to worry at all about this change. |
We're already breaking the constructor, right? This is one of those cases where I'd rather have users make an explicit/informed choice. That way, users can choose when they want to deprecate the old method. Also with respect to breaking changes, I didn't look too closely before but it looks like we're removing the "service" string. That could be an issue for some upstream users. |
Yes, that is intentional. We're now implementing the spec, and the spec defines what the service name is: https://github.com/libp2p/specs/blob/master/discovery/mdns.md#definitions. If there are use cases where users might want to set the service name to something different, we might want to update the spec. |
I think we might. Take a look at https://grep.app/search?q=github.com/libp2p/go-libp2p/p2p/discovery. It looks like most systems set a custom tag. This is actually pretty reasonable because one usually wants to find like-services, not random libp2p nodes. If we release as-is, all of these services are going to be unhappy. |
|
Good point. We'll update the spec. #1161 implements the solution suggested by @Stebalien. |
|
@marten-seemann does the spec still need an update in regards to the service name? |
Friendly ping @marten-seemann. |
No description provided.