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.

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

chainparams: Add achow101 DNS seeder #30007

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

achow101
Copy link
Member

I wrote a DNS seeder and have been running it for the past 2 months now. I believe it is ready/good enough to be used as an additional DNS seeder for all of our supported public networks.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 30, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK laanwj, 1440000bytes
Concept ACK willcl-ark, ariard, Sjors, virtu, kristapsk

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

src/kernel/chainparams.cpp Outdated Show resolved Hide resolved
@laanwj
Copy link
Member

laanwj commented May 1, 2024

Concept ACK

Flags: x9
Status DNS name                                 Totals     IPv4                  IPv6
                                                nconn/n    nconn/n           TTL nconn/n           TTL
* mainnet
OK     seed.bitcoin.sipa.be.                    25/25      25/25            2640 0/0
OK     dnsseed.bluematt.me.                     29/31      20/21              60 9/10               60
OK     dnsseed.bitcoin.dashjr-list-of-p2p-nodes 22/22      22/22            2644 0/0
ERR    seed.bitcoinstats.com.                   0/0        SERVFAIL              SERVFAIL
ERR    seed.bitcoin.jonasschnelli.ch.           0/0        SERVFAIL              SERVFAIL
OK     seed.btc.petertodd.net.                  11/36      7/23             2705 4/13             3600
OK     seed.bitcoin.sprovoost.nl.               22/36      15/23            2705 7/13             3600
OK     dnsseed.emzy.de.                         35/39      21/25            2764 14/14            3600
OK     seed.bitcoin.wiz.biz.                    23/31      16/21              60 7/10               60
OK     dnsseed.mainnet.bitcoin.achow101.com.    38/40      20/20              30 18/20              30

* testnet
NONE   testnet-seed.bitcoin.jonasschnelli.ch.   0/0        0/0                   0/0
OK     seed.tbtc.petertodd.net.                 15/36      9/23             2841 6/13             3600
NONE   testnet-seed.bluematt.me.                0/0        0/0                   0/0
NONE   seed.testnet.bitcoin.sprovoost.nl.       0/0        0/0                   0/0
OK     dnsseed.testnet.bitcoin.achow101.com.    31/40      15/20              30 16/20              29

* signet
NONE   seed.signet.bitcoin.sprovoost.nl.        0/0        0/0                   0/0
OK     dnsseed.signet.bitcoin.achow101.com.     30/40      15/20              30 15/20              30
  • Returns a good number of results (20 per query), of which a large fraction tends to be connectable within one second.
  • A TTL of 30 seconds is low compared to the others. FWIW, in dnsseed-policy.md a minimum of 60 seconds is mentioned. (this was fixed and changed to 60)

@willcl-ark
Copy link
Member

Concept ACK

Some of our DNS seeds are currently not performing well, so adding a new/more reliable one seems logical to me.

I have also been running this seeder myself for some time (at seed.bitcoin.fish.foo) and the program seems to work well from the operator side too, not requiring any intervention in the few weeks i've been running it.

I ran a different test to @laanwj on mainnet IPV4 only, and did find seeds generally returning results:

Details
Results:

seed.bitcoin.sipa.be.
  x1         responses: 25
  x5         responses: 25
  x9         responses: 25
  xd         responses: 25

dnsseed.bluematt.me.
  x9         responses: 21

dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us.
  (no flag)  responses: 22
  x9         responses: 22

seed.bitcoinstats.com.
  x1         responses: 17
  x2         responses: 17
  x3         responses: 17
  x4         responses: 17
  x5         responses: 17
  x6         responses: 17
  x7         responses: 17
  x8         responses: 17
  x9         responses: 17
  xa         responses: 17
  xb         responses: 17
  xc         responses: 17
  xd         responses: 17
  xe         responses: 17
  xf         responses: 17

seed.bitcoin.jonasschnelli.ch.
  x1         responses: 23
  x5         responses: 23
  x9         responses: 23
  xd         responses: 23

seed.btc.petertodd.net.
  x1         responses: 23
  x5         responses: 23
  x9         responses: 23
  xd         responses: 23

seed.bitcoin.sprovoost.nl.
  (no flag)  responses: 23
  x9         responses: 23

dnsseed.emzy.de.
  (no flag)  responses: 26
  x9         responses: 25

seed.bitcoin.wiz.biz.
  (no flag)  responses: 21
  x9         responses: 21

dnsseed.mainnet.bitcoin.achow101.com.
  x1         responses: 20
  x5         responses: 20
  x9         responses: 20
  x49        responses: 20
  x809       responses: 20
  x849       responses: 20
  xd         responses: 20
  x400       responses: 20
  x404       responses: 20
  x408       responses: 20
  x448       responses: 20
  xc08       responses: 20
  xc48       responses: 20
  x40c       responses: 20

I will try to expand my test script soon to include testnet and also to query whether the flags are accurate for returned results so I can verify both this new seed, and existing seeds.

@achow101
Copy link
Member Author

achow101 commented May 1, 2024

A TTL of 30 seconds is low compared to the others. FWIW, in dnsseed-policy.md a minimum of 60 seconds is mentioned.

I've changed it to 60 seconds.

@mzumsande
Copy link
Contributor

@virtu FYI, would it be easily possible to run some of the metrics of https://21.ninja/dns-seeds/ for this new seeder?

@ariard
Copy link
Member

ariard commented May 2, 2024

Concept ACK.

By the way, it would be great if mainnet DNS seeders are considering to sign by default the peers records. This could be amply checked in ThreadDNSAddressSeed() or in semi-automatic fashion in the logs. ECDSA secp256k1 isn’t supported by default in DNS, though you have ed25519 or ECDSA P256 which are widely supported by key management softwares.

@laanwj
Copy link
Member

laanwj commented May 2, 2024

By the way, it would be great if mainnet DNS seeders are considering to sign by default the peers records. This could be amply checked in ThreadDNSAddressSeed()

There are some that do (#19714), but as far as i know, there is no cross-platform API for checking DNSSEC status from user code. i've unlocked that issue for discussion.

@virtu
Copy link
Contributor

virtu commented May 2, 2024

@virtu FYI, would it be easily possible to run some of the metrics of https://21.ninja/dns-seeds/ for this new seeder?

Concept ACK

@mzumsande, the seed is now being monitored on dev.21.ninja.

There may be some graph artifacts until a second data point becomes available. But so far data looks good: 40 advertised addresses (half of them ipv4, the other ipv6), and 35 of them reachable.

@1440000bytes
Copy link

Why does the seeder consider 'default port' for good nodes?

@achow101
Copy link
Member Author

achow101 commented May 3, 2024

Why does the seeder consider 'default port' for good nodes?

DNS cannot provide port numbers, but a port must be known when connecting to a node. So we assume the default port, and because of that assumption, DNS seeders need to return nodes that are listening on the default port.

@Sjors
Copy link
Member

Sjors commented May 3, 2024

Concept ACK

There's discussion in #29911 about whether we should mention the specific feature bits here.

I tested that the mainnet seed result returns both IPv4 and IPv6 records and tried to connect to a random result. I didn't do any fancier analysis.

@1440000bytes
Copy link

Concept ACK on adding another DNS seeder

Why does the seeder consider 'default port' for good nodes?

DNS cannot provide port numbers, but a port must be known when connecting to a node. So we assume the default port, and because of that assumption, DNS seeders need to return nodes that are listening on the default port.

TXT records could work but that will require lot of other changes (out of scope)

@laanwj
Copy link
Member

laanwj commented May 4, 2024

TXT records could work but that will require lot of other changes (out of scope)

Right-DNS can serve arbitrary information, but it would complicate things in the client: the cross-platform libc resolver, getaddrinfo, can only resolve addresses. So for that'd we'd need to add some dependency library for DNS resolving. The question is whether it's worth it.

Also, IIRC caching DNS servers don't always cache TXT records (because they can store arbitrary data); the caching and the privacy that comes with it, is the advantage of using DNS in the first place, over simply using bitcoin protocol for seeding.

Mind that the DNS seeds are only an entry point to the network. The gossip network itself can handle alternative ports fine, so from that point on, nodes with other ports can be discovered by a node. In the main threat scenario that would make this entry point useless, a hypothetical future where port 8333 would be blocked by ISPs, it's extremely likely that the DNS seeds would also be blocked entirely as they're easy to enumerate.

@achow101
Copy link
Member Author

achow101 commented May 5, 2024

I've implemented DNSSEC

@laanwj
Copy link
Member

laanwj commented May 5, 2024

I've implemented DNSSEC

That's neat!

i think it's still missing some part, resolving through Google's DNS (which has more verbose error messages than my ISP) gives:

$ dig x9.dnsseed.signet.bitcoin.achow101.com. @1.1.1.1
⋮
;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 1232
; EDE: 10 (RRSIGs Missing): (failed to verify signatures for x9.dnsseed.signet.bitcoin.achow101.com. opt-out proof)
⋮

@achow101
Copy link
Member Author

achow101 commented May 5, 2024

i think it's still missing some part, resolving through Google's DNS (which has more verbose error messages than my ISP) gives:

Should be fixed now

Copy link
Member

@laanwj laanwj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good now!

ACK ee218aa

Copy link

@1440000bytes 1440000bytes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ee218aa

@luke-jr
Copy link
Member

luke-jr commented May 7, 2024

@achow101 Are you sure you want to put this on a common domain with other things?

@kristapsk
Copy link
Contributor

Concept ACK on using different software for various DNS seeders. Need to do more review / testing on this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet