Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[RFC] peer discovery with mDNS #80
[RFC] peer discovery with mDNS #80
Changes from 3 commits
a4626ef
9e0e1f5
154424f
b2c450e
105fd2b
a9c587c
2e0219a
8cc1297
53d9faf
409c019
59e0793
2f3e55d
479f07c
4c5a459
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
So, we'll need it for multiaddrs. We're able to get away with this for now but not once we enable QUIC by default. Ideally, we'd only use TXT records. However, we have to supply an A or AAAA record and also have to supply a port. Personally, I'm all for using our TCP port and real IP addresses when announcing but mostly ignoring them when accepting (unless we get no TXT records in which case we can try the IP/PORT from the other records).
Note: We'd probably want to use
dnsaddr=/ip4/.../tcp/...
as the format for TXT records.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.
TXT record
dnsaddr=/full/address
is a problem. Strings must be less than 128 bytes.dnsaddr=/ip6/2406:e001:1467:1:ecec:9ca6:XXXX:XXXX/tcp/4009/ipfs/QmTp1fwYnJxc5rUEVM2xowcDiyR61J7yvmBRUQzT4g8MHs
almost reaches this limit. If the peer-id hashing algorithm changes to sha2-512 then the byte limit is exceeded.I strongly suggest that TXT dnsaddr is not used.
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.
Also, we are just trying to discover peers on the local link. So to my thinking is that an IP address, port and peer ID is all that is needed.
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.
Can we send multiple records?
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.
Well, we also have protocols like QUIC and may add other random protocols in the future. We also do need to put the peer ID somewhere.
It looks like the standard way to do this is to break it into multiple records. You can see how DKIM does it here: https://serverfault.com/questions/255580/how-do-i-enter-a-strong-long-dkim-key-into-dns
We could also use a base64 encoded binary multiaddr but I'd rather not.