-
Notifications
You must be signed in to change notification settings - Fork 161
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
docs(iroh-net): Improve pkarr discovery docs #2722
Conversation
This tries to improve the pkarr discovery docs by trying to describe enough about how it works that users do not need to go looking elsewhere.
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/2722/docs/iroh/ Last updated: 2024-09-30T13:00:29Z |
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.
Thank you @flub for writing this sorely missing piece of documentation.
A couple of comments inline.
Also: I agree to your suggested renames above. And if we can combine it all into a single PkarrDiscovery
trait with feature flags and a builder that might be even better IMO.
The current situation is very much the result of us adding piece after piece, and not giving things another high-level view I think. So refactoring this into a more coherent picture would be good.
/// The production pkarr relay run by [number 0]. | ||
/// | ||
/// This server is both a pkarr relay server as well as a DNS resolver, see the [module | ||
/// documentation]. However it does not interact with the Mainline DHT, so is a more |
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.
iroh-dns-server
can interact with the mainline DHT for resolving node ids not found in the local database, see here. It does not (yet?) support publishing records to mainline DHT.
The config template for production does not enable the mainline support by defualt. IIRC we wanted to enable it on dns.iroh.link
though, I do not know OTOH if that is the case. I would guess @Arqu can verify.
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.
Im not sure, lets chat about this on the meeting today.
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.
If it is currently not enabled let's leave it documented like this. I'm not trying to change anything here, merely documenting the current state.
/// `iroh-dns-server`, e.g. as running on [`N0_DNS_PKARR_RELAY_PROD`], as the home server | ||
/// keeps the records for the domain. When using the pkarr relay no DNS is involved and the | ||
/// setting is ignored. | ||
// TODO(flub): huh? |
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.
I also cannot say what the last sentence is supposed to mean.
This paragraph should likely instead explain that this is the timeframe in which, after publishing new node addr info, other nodes might still receive the old outdated info when resolving via DNS.
@flub what's missing here to merge this? |
Co-authored-by: Franz Heinzmann <[email protected]>
Description
This tries to improve the pkarr discovery docs by trying to describe
enough about how it works that users do not need to go looking
elsewhere.
Breaking Changes
none
Notes & open questions
There are so many code overlaps and naming inconsistencies here. But this was
part of docs friday so I'm just documenting things for now.
PkarrPublisher
is probably aPkarrRelayPublisher
PkarrResolver
is also aPkarrRelayResolver
DnsDiscovery
only does resolving ->DnsResolver
DhtDiscovery
can also use relay server, but at least it does both publish and resolveDhtDiscovery
is pretty generic, it can probably be made to do both replacePkarrResolver
andPkarrPublisher
, maybe with custom constructors.Change checklist
[ ] Tests if relevant.[ ] All breaking changes documented.