Skip to content
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.

Sign up for GitHub

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

Jump to bottom

SRV implementation #5018

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

SRV implementation #5018

wants to merge 16 commits into from
+1,400 −1,009

Conversation

crocodile-dentist
Copy link
Contributor

@crocodile-dentist crocodile-dentist commented Dec 3, 2024 •
edited
Loading

Description

This change introduces support for DNS SRV record lookups. When a relay is associated via SRV record, a two step resolution is attempted to first identify the relevant domains from which to make a secondary lookup to obtain the final address. All lookups are early bound in the sense that only the domains with top priority (ie. with the lowest priority numeric value in the record) are considered, and afterwards a single domain is randomly sampled it is queried for the address. If the lookup fails it is not retried.

The tracing of dns lookups has been reworked. The tracing happens in the DNSActions's dnsLookupWithTTL function, and the corresponding constructors from eg. TraceLocalRootPeers (and for Public and Ledger types) have been removed. This simplifies the logic in localRootsPeersProvider, publicRootPeersProvider and resolveLedgerPeers functions since the lookup doesn't have to return additional information regarding the domain names which have been resolved, which is used in the testing framework. But the main impetus was that this information can be traced separately, and the resolve function can return socket addresses directly.

Checklist

Quality

  • Commit sequence makes sense and have useful messages, see ref.
  • New tests are added and existing tests are updated.
  • Self-reviewed the PR.

Maintenance

  • Linked an issue or added the PR to the current sprint of ouroboros-network project.
  • Added labels.
  • Updated changelog files.
  • The documentation has been properly updated, see ref.

@crocodile-dentist crocodile-dentist self-assigned this Dec 3, 2024
@crocodile-dentist crocodile-dentist requested a review from a team as a code owner December 3, 2024 10:31
@crocodile-dentist crocodile-dentist marked this pull request as draft December 3, 2024 10:32
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/srv branch 13 times, most recently from 34fd024 to 75341dc Compare December 10, 2024 10:33
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/srv branch 3 times, most recently from b3f0498 to 4649b5a Compare December 11, 2024 15:20
@coot coot mentioned this pull request Dec 11, 2024
Open
@coot coot linked an issue Dec 11, 2024 that may be closed by this pull request
Add support for SRV records #2883
Open
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/srv branch 6 times, most recently from 37c578d to 9718a9c Compare December 13, 2024 11:39
coot
coot reviewed Dec 13, 2024
View reviewed changes
ouroboros-network-api/src/Ouroboros/Network/PeerSelection/RelayAccessPoint.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNSActions.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNSActions.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNSActions.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNSActions.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNSActions.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/DNSActions.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/LedgerPeers.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/LedgerPeers.hs Outdated Show resolved Hide resolved
ouroboros-network/src/Ouroboros/Network/PeerSelection/RootPeersDNS/LedgerPeers.hs Outdated Show resolved Hide resolved
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/srv branch 8 times, most recently from 78a59fa to 6f44d0d Compare January 20, 2025 13:20
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/srv branch 3 times, most recently from f2f6462 to e9fe90e Compare January 27, 2025 10:23
@crocodile-dentist
Add RelayAccessSRVDomain constructor to RelayAccessPoint type
02f633c 
Update JSON instances and transplant CBOR instances from
RelayAccessPointCoded type, which will be removed in a
followup commit, to support LedgerPeerSnapshot type for
Genesis big ledger peers snapshot.
@crocodile-dentist
Bump LedgerPeerSnapshot version
08413d3 
Directly leverages JSON and CBOR instances for RelayAccessPoint
instead of RelayAccessPointCoded, which will be removed in a
followup commit. The upshot of this change is that all domain
names in the snapshot file will be keyed by 'address' instead
of 'domain', but other than that there is no change to it. What
we gain is less maintenance burden of those extra instances that
now can be dropped.
@crocodile-dentist
Removes RelayAccessPointCoded and associated instances
175fac2
@crocodile-dentist
Bump LedgerPeerSnapshot tests for V2
597e8ef
@crocodile-dentist
Rework RelayAccessPoint round trip test
879bd90 
Tests round trip encoding, modulo fully qualified domain
names, which previously was the only distinct feature
of the RelayAccessPointCoded type which has been removed.
@crocodile-dentist
PeerSelection test types and instances
fe4c035
@crocodile-dentist
PeerSelection test changes
7de6f5c
@crocodile-dentist crocodile-dentist force-pushed the mwojtowicz/srv branch 2 times, most recently from f428325 to d48fb7a Compare January 27, 2025 15:54
@crocodile-dentist
Testnet diffusion script generation refactor for SRV records
26f3c7f 
Reorganizes generation of diffusion script to accomodate
mock DNS SRV lookups. The generation of DNS lookup map
is postponed to genDomainMapScript function, where it is convenient
to create followup lookup domains for relays tagged as
RelayAccessSRVDomain. So now the strategy is first to generate some
relays, and then adapt the DNS lookup map for a particular set of
choices.
This map was previously generated in the Arbitrary instance for
RelayAccessInfosWithDNS, so simultaneously with the relays,
which was not optimal for the present more sequential nature needed
to generate the testcase. The latter type  was therefore also
removed.
Previous RelayAccessInfos was renamed to TestnetRelayInfos and
augmented with some fields for convenience.
@crocodile-dentist
Testnet tests adaptations
fda5626
@crocodile-dentist
changelogs
114ac28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bolt12 bolt12 bolt12 requested changes

@coot coot coot left review comments

Requested changes must be addressed to merge this pull request.

Assignees

@crocodile-dentist crocodile-dentist

Labels
DNS Issues / PRs related to DNS
Projects
Ouroboros Network
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for SRV records
3 participants
@crocodile-dentist @coot @bolt12