seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds #31960

jonatack commented at 6:26 PM on February 27, 2025: member

In makeseeds.py:

fix the user agent regex (by laanwj)
fix the I2P column header spacing
update the regex (it was also not updated for the previous release)
update MIN_BLOCKS (4320 blocks/month * ~6.5 months) and add README documentation to remember to update it
further robustness/standardness/consistency improvements to the regexes (by l0rinc)

Add signet and testnet4 seeds to the README and to generate-seeds.py

Update the fixed seeds in src/chainparamsseeds.h

In kernel/chainparams.cpp:

add signet fixed seeds if default network
remove hardcoded signet seeds

Update contrib/seeds/.gitignore with signet and testnet4

The previous 2 seeds updates were #30008 and #30695.

mainnet:

$ contrib/seeds$ python3 makeseeds.py -a asmap-filled.dat -s seeds_main.txt > nodes_main.txt

Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
  IPv4   IPv6  Onion    I2P  CJDNS Pass                                               
 17252   3630  21079   3095     12 Initial
 17252   3630  21079   3095     12 Skip entries with invalid address
  8444   1742  14607   2330     10 After removing duplicates
  8194   1691  14321   2102     10 Enforce minimal number of blocks
  7838   1578  14321   2102     10 Require service bit 1
  6802   1326  14321   2102     10 Require minimum uptime
  6762   1321  14320   2102     10 Require a known and recent user agent
  6762   1321  14320   2102     10 Filter out hosts with multiple bitcoin ports
   512    485    512    512     10 Look up ASNs and limit results per ASN and per net

signet:

$ contrib/seeds$ python3 makeseeds.py -a asmap-filled.dat -s seeds_signet.txt -m 237800 > nodes_signet.txt


Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
  IPv4   IPv6  Onion    I2P  CJDNS Pass                                               
   110     47     63      9      4 Initial
   110     47     63      9      4 Skip entries with invalid address
   110     47     63      9      4 After removing duplicates
    83     31     58      9      4 Enforce minimal number of blocks
    83     31     58      9      4 Require service bit 1
    83     31     57      9      4 Require minimum uptime
    83     31     57      9      4 Require a known and recent user agent
    83     31     57      7      4 Filter out hosts with multiple bitcoin ports
    42     30     57      7      4 Look up ASNs and limit results per ASN and per net

testnet:

$ contrib/seeds$ python3 makeseeds.py -a asmap-filled.dat -s seeds_test.txt > nodes_test.txt

Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
  IPv4   IPv6  Onion    I2P  CJDNS Pass                                               
   204     73     96     11      5 Initial
   204     73     96     11      5 Skip entries with invalid address
   204     73     96     11      5 After removing duplicates
   204     73     96     11      5 Enforce minimal number of blocks
   204     73     96     11      5 Require service bit 1
   195     69     87      9      5 Require minimum uptime
   193     69     87      9      5 Require a known and recent user agent
   193     69     87      9      5 Filter out hosts with multiple bitcoin ports
    79     39     87      9      5 Look up ASNs and limit results per ASN and per net

testnet4

$ contrib/seeds$ python3 makeseeds.py -a asmap-filled.dat -s seeds_testnet4.txt -m 72600 > nodes_testnet4.txt

Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
  IPv4   IPv6  Onion    I2P  CJDNS Pass                                               
   149    115     69     11      4 Initial
   149    115     69     11      4 Skip entries with invalid address
   149    115     69     11      4 After removing duplicates
   104     75     52      7      4 Enforce minimal number of blocks
   104     75     52      7      4 Require service bit 1
   100     73     51      7      4 Require minimum uptime
   100     73     51      7      4 Require a known and recent user agent
   100     73     51      7      4 Filter out hosts with multiple bitcoin ports
    43     46     51      7      4 Look up ASNs and limit results per ASN and per net

DrahtBot commented at 6:26 PM on February 27, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31960.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	l0rinc, achow101, laanwj
Concept ACK	1440000bytes

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#31991 (RFC: Add operator""_uint256 compile-time user-defined literal by l0rinc)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

laanwj added the label Scripts and tools on Mar 3, 2025

in contrib/seeds/makeseeds.py:42 in 2fb1485c1c outdated

  38 | @@ -39,16 +39,16 @@
  39 |      r"0.16.(0|1|2|3|99)|"
  40 |      r"0.17.(0|0.1|1|2|99)|"
  41 |      r"0.18.(0|1|99)|"
  42 | -    r"0.19.(0|1|2|99)|"
  43 | +    r"0.19.(0|1|2|99).(0|1)|"

laanwj commented at 12:41 PM on March 3, 2025:

The regexp doesn't match the entire string until the end, so "0.19.0.1" and such already match. This change will only make it that "0.19.1" no longer matches.

jonatack commented at 5:22 PM on March 3, 2025:

Oops -- done, thank you!

in contrib/seeds/makeseeds.py:51 in 2fb1485c1c outdated

  51 | -    r"26.(0|1|99).0|"
  52 | -    r"27.(0|1|99).0|"
  53 | -    r"28.(0|99).0|"
  54 | +    r"26.(0|1|2|99).0|"
  55 | +    r"27.(0|1|2|99).0|"
  56 | +    r"28.(0|1|99).0|"

laanwj commented at 12:42 PM on March 3, 2025:

We shouldn't have | at the end of the last clause, as this will make it match the empty string too (so effectively everything starting with Satoshi: matches).

jonatack commented at 5:31 PM on March 3, 2025:

We shouldn't have | at the end of the last clause, as this will make it match the empty string too (so effectively everything starting with Satoshi: matches).

Good catch. It looks like this would mean that the regex has been doing this since a8ec9eede4c745c6b6fd76974816ffad8034129a one year ago?

laanwj commented at 6:11 PM on March 3, 2025:

Yes, i didn't notice before ! It's too easy to make this mistake.

It also makes me wonder whether this is a useful filter at all, or whether to replace it with a simpler one that extracts the version number, followed by some minimum/maximum bounds check. But that's not something that needs to be addressed in this PR.

jonatack commented at 6:12 PM on March 3, 2025:

Not sure I see a difference in the makeseeds output. Done in the first commit attributed to you (thanks!)

laanwj commented at 6:18 PM on March 3, 2025:

Thanks!

glozow added this to the milestone 29.0 on Mar 3, 2025

1440000bytes commented at 4:11 PM on March 3, 2025: none

Concept ACK

jonatack force-pushed on Mar 3, 2025

jonatack commented at 6:20 PM on March 3, 2025: member

Updated to take @laanwj's review feedback and re-ran the seeds scripts.

laanwj commented at 6:56 PM on March 3, 2025: member

Code review ACK b2dc4458ac86c0a633ca3eb87827ac2731993f97 on the script changes

mainnet

Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
  IPv4   IPv6  Onion    I2P  CJDNS Pass
 17173   3638  21303   3116     12 Initial
 17173   3638  21303   3116     12 Skip entries with invalid address
  8378   1755  14661   2343     10 After removing duplicates
  8337   1736  14656   2343     10 Enforce minimal number of blocks
  7986   1612  14654   2343     10 Require service bit 1
  6963   1365  14644   2343     10 Require minimum uptime
  6917   1359  14643   2343     10 Require a known and recent user agent
  6917   1359  14643   2343     10 Filter out hosts with multiple bitcoin ports
   512    502    512    512     10 Look up ASNs and limit results per ASN and per net

testnet

Loading asmap database "asmap-filled.dat"…Done.
Loading and parsing DNS seeds…Done.
  IPv4   IPv6  Onion    I2P  CJDNS Pass
   204     74     95     11      5 Initial
   204     74     95     11      5 Skip entries with invalid address
   204     74     95     11      5 After removing duplicates
   204     74     95     11      5 Enforce minimal number of blocks
   204     74     95     11      5 Require service bit 1
   197     68     87      9      5 Require minimum uptime
   195     68     87      9      5 Require a known and recent user agent
   195     68     87      9      5 Filter out hosts with multiple bitcoin ports
    78     38     87      9      5 Look up ASNs and limit results per ASN and per net

i have re-run the steps in an attempt to check the output, but the lack of determinism makes it hard to say anything. All that could be concluded is that the diff is smaller from the previous state to the current one, to the one between the current one and my re-run:

$ git show HEAD --shortstat nodes_main.txt
commit b2dc4458ac86c0a633ca3eb87827ac2731993f97 (HEAD)
 1 file changed, 1618 insertions(+), 1627 deletions(-)
$ git diff --shortstat nodes_main.txt
 1 file changed, 419 insertions(+), 418 deletions(-)

$ git show HEAD --shortstat nodes_test.txt
commit b2dc4458ac86c0a633ca3eb87827ac2731993f97 (HEAD)
 1 file changed, 128 insertions(+), 222 deletions(-)
$ git diff --shortstat nodes_test.txt
(no output)

The testnet output does match 100%.

jonatack commented at 7:31 PM on March 3, 2025: member

Yes, for the avoidance of doubt, I ran the script with no manual curation (as done previously from v21 to v27).

achow101 commented at 10:45 PM on March 3, 2025: member

Could also do testnet4, my seeder publishes a seeds.txt.gz at https://testnet4.achownodes.xyz/seeds.txt.gz. And also signet: https://signet.achownodes.xyz/seeds.txt.gz

in contrib/seeds/makeseeds.py:51 in 6597e87666 outdated

  47 | @@ -48,7 +48,7 @@
  48 |      r"25.(0|1|2|99).0|"
  49 |      r"26.(0|1|99).0|"
  50 |      r"27.(0|1|99).0|"
  51 | -    r"28.(0|99).0|"
  52 | +    r"28.(0|99).0"

achow101 commented at 11:03 PM on March 3, 2025:

In 6597e87666daa7fabdf310e9ad39fc8a7c81ceea "makeseeds: fix incorrect regex"

Perhaps put the | in the front of every regex rather than the end, to make it easier to update in the future and not accidentally run into this problem again.

jonatack commented at 9:50 PM on March 4, 2025:

Done

jonatack force-pushed on Mar 4, 2025

jonatack commented at 9:52 PM on March 4, 2025: member

Could also do testnet4, my seeder publishes a seeds.txt.gz at testnet4.achownodes.xyz/seeds.txt.gz. And also signet: signet.achownodes.xyz/seeds.txt.gz

Done. Also updated MIN_BLOCKS and added a reminder to the README.

jonatack renamed this:
~~seeds: update makeseeds regex and DNS fixed seeds~~
seeds: add signet/testnet4, update makeseeds regex, minblocks, fixed seeds
on Mar 4, 2025

in contrib/seeds/makeseeds.py:32 in 959d6621b2 outdated

  25 | @@ -26,29 +26,29 @@
  26 |      'ipv6': 10,
  27 |  }
  28 |  
  29 | -MIN_BLOCKS = 840000
  30 | +MIN_BLOCKS = 868000
  31 |  
  32 |  PATTERN_IPV4 = re.compile(r"^((\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})):(\d+)$")
  33 |  PATTERN_IPV6 = re.compile(r"^\[([0-9a-z:]+)\]:(\d+)$")

l0rinc commented at 10:04 PM on March 4, 2025:

These regexes are all over the place, when are we using [0-9] at the beginning and \d at the end? And there's no need to escape \]. And why are we matching g..z, isn't this a hexadecimal number? Why aren't port capped at max uint16_t?

PATTERN_IPV6 = re.compile(r"^\[([\da-f:]+)]:(\d{1,5})$")

And since https://datatracker.ietf.org/doc/html/rfc5952#section-2.3 indicates that we should accept uppercase as well

PATTERN_IPV6 = re.compile(r"^\[([\da-f:]+)]:(\d{1,5})$", re.IGNORECASE)

jonatack commented at 7:34 PM on March 6, 2025:

done

in contrib/seeds/makeseeds.py:37 in 959d6621b2 outdated

  48 | -    r"24.(0|1|2|99).(0|1)|"
  49 | -    r"25.(0|1|2|99).0|"
  50 | -    r"26.(0|1|99).0|"
  51 | -    r"27.(0|1|99).0|"
  52 | -    r"28.(0|99).0|"
  53 | +    r"0.14.(0|1|2|3|99)"

l0rinc commented at 10:05 PM on March 4, 2025:

above we're escaping the ., here we're not.

    r"0\.14\.([0-3]|99)"

Otherwise we'd match: /Satoshi:0₿21₿0

jonatack commented at 7:07 PM on March 6, 2025:

Hi @l0rinc, this diff, if I understand you correctly?

 PATTERN_AGENT = re.compile(
     r"^/Satoshi:("
-    r"0.14.(0|1|2|3|99)"
-    r"|0.15.(0|1|2|99)"
-    r"|0.16.(0|1|2|3|99)"
-    r"|0.17.(0|0.1|1|2|99)"
-    r"|0.18.(0|1|99)"
-    r"|0.19.(0|1|2|99)"
-    r"|0.20.(0|1|2|99)"
-    r"|0.21.(0|1|2|99)"
-    r"|22.(0|1|99).0"
-    r"|23.(0|1|2|99).0"
-    r"|24.(0|1|2|99).(0|1)"
-    r"|25.(0|1|2|99).0"
-    r"|26.(0|1|2|99).0"
-    r"|27.(0|1|2|99).0"
-    r"|28.(0|1|99).0"
+    r"0\.14\.([0-3]|99)"
+    r"|0\.15\.(0|1|2|99)"
+    r"|0\.16\.(0|1|2|3|99)"
+    r"|0\.17\.(0|0.1|1|2|99)"
+    r"|0\.18\.(0|1|99)"
+    r"|0\.19\.(0|1|2|99)"
+    r"|0\.20\.(0|1|2|99)"
+    r"|0\.21\.(0|1|2|99)"
+    r"|22\.(0|1|99)\.0"
+    r"|23\.(0|1|2|99)\.0"
+    r"|24\.(0|1|2|99)\.(0|1)"
+    r"|25\.(0|1|2|99)\.0"
+    r"|26\.(0|1|2|99)\.0"
+    r"|27\.(0|1|2|99)\.0"
+    r"|28\.(0|1|99)\.0"
     r")")

Edit: done per the above diff.

in contrib/seeds/makeseeds.py:31 in 959d6621b2 outdated

  25 | @@ -26,29 +26,29 @@
  26 |      'ipv6': 10,
  27 |  }
  28 |  
  29 | -MIN_BLOCKS = 840000
  30 | +MIN_BLOCKS = 868000
  31 |  
  32 |  PATTERN_IPV4 = re.compile(r"^((\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})):(\d+)$")

l0rinc commented at 10:22 PM on March 4, 2025:

ipv4 fragments cannot start with any \d (e.g. 345 isn't valid), it's should start with a [0-2]?\d{1,2} and ports should also be bounded. But in other places I found this regex used to match the IPv4 range properly:

PATTERN_IPV4 = re.compile(r"^((25[0-5]|2[0-4]\d|[01]?\d{1,2})\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})\.(25[0-5]|2[0-4]\d|[01]?\d{1,2})):(\d{1,5})$")

but I think this should be good enough for us:

PATTERN_IPV4 = re.compile(r"^(([0-2]?\d{1,2})\.([0-2]?\d{1,2})\.([0-2]?\d{1,2})\.([0-2]?\d{1,2})):(\d{1,5})$")

jonatack commented at 7:35 PM on March 6, 2025:

done

in contrib/seeds/makeseeds.py:52 in 959d6621b2 outdated

  63 | +    r"|24.(0|1|2|99).(0|1)"
  64 | +    r"|25.(0|1|2|99).0"
  65 | +    r"|26.(0|1|2|99).0"
  66 | +    r"|27.(0|1|2|99).0"
  67 | +    r"|28.(0|1|99).0"
  68 |      r")")

l0rinc commented at 10:36 PM on March 4, 2025:

is this meant to look for prefixes or full matches?

    r")$")

achow101 commented at 1:26 AM on March 5, 2025:

Prefixes, the user agent may contain a comment at the end which we can ignore.

in contrib/seeds/makeseeds.py:40 in 959d6621b2 outdated

  51 | -    r"27.(0|1|99).0|"
  52 | -    r"28.(0|99).0|"
  53 | +    r"0.14.(0|1|2|3|99)"
  54 | +    r"|0.15.(0|1|2|99)"
  55 | +    r"|0.16.(0|1|2|3|99)"
  56 | +    r"|0.17.(0|0.1|1|2|99)"

l0rinc commented at 10:38 PM on March 4, 2025:

this is a bit weird, but if it's correct, we can compress it a bit more:

    r"|0\.17\.(0\.1|[0-2]|99)"

achow101 commented at 1:29 AM on March 5, 2025:

I don't think this needs compression, explicitly listing the minor versions is easier to read IMO.

jonatack commented at 7:08 PM on March 6, 2025:

I don't think this needs compression, explicitly listing the minor versions is easier to read IMO.

Agree

in contrib/seeds/makeseeds.py:34 in 959d6621b2 outdated

  30 | +MIN_BLOCKS = 868000
  31 |  
  32 |  PATTERN_IPV4 = re.compile(r"^((\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})):(\d+)$")
  33 |  PATTERN_IPV6 = re.compile(r"^\[([0-9a-z:]+)\]:(\d+)$")
  34 |  PATTERN_ONION = re.compile(r"^([a-z2-7]{56}\.onion):(\d+)$")
  35 |  PATTERN_I2P = re.compile(r"^([a-z2-7]{52}\.b32.i2p):(\d+)$")

l0rinc commented at 10:39 PM on March 4, 2025:

the first . is escaped, the second isn't - seems fishy:

PATTERN_I2P = re.compile(r"^([a-z2-7]{52}\.b32\.i2p):(\d{1,5})$")

jonatack commented at 7:35 PM on March 6, 2025:

done

l0rinc changes_requested

l0rinc commented at 10:48 PM on March 4, 2025: contributor

I have only reviewed the regexes, please see my suggestions.

<details> <summary>Regexes</summary>

PATTERN_IPV4 = re.compile(r"^(([0-2]?\d{1,2})\.([0-2]?\d{1,2})\.([0-2]?\d{1,2})\.([0-2]?\d{1,2})):(\d{1,5})$")
PATTERN_IPV6 = re.compile(r"^\[([\da-f:]+)]:(\d{1,5})$", re.IGNORECASE)
PATTERN_ONION = re.compile(r"^([a-z2-7]{56}\.onion):(\d{1,5})$")
PATTERN_I2P = re.compile(r"^([a-z2-7]{52}\.b32\.i2p):(\d{1,5})$")
PATTERN_AGENT = re.compile(
    r"^/Satoshi:("
    r"0\.14\.([0-3]|99)"
    r"|0\.15\.([0-2]|99)"
    r"|0\.16\.([0-3]|99)"
    r"|0\.17\.(0\.1|[0-2]|99)"
    r"|0\.18\.([01]|99)"
    r"|0\.19\.([0-2]|99)"
    r"|0\.20\.([0-2]|99)"
    r"|0\.21\.([0-2]|99)"
    r"|22\.([01]|99)\.0"
    r"|23\.([01]|99)\.0"
    r"|24\.([0-2]|99)\.[01]"
    r"|25\.([0-2]|99)\.0"
    r"|26\.([01]|99)\.0"
    r"|27\.([01]|99)\.0"
    r"|28\.(0|99)\.0"
    r")")

</details>

achow101 commented at 1:35 AM on March 5, 2025: member

While src/chainparamsseeds.h now has chainparams_seed_signet, it is not used anywhere. SigNetParams does not set vFixedSeeds as the other ones do. It probably should, but only when using the default testnet, as we do for the DNS seeds.

jonatack force-pushed on Mar 6, 2025

jonatack commented at 8:08 PM on March 6, 2025: member

Thanks for the reviews @l0rinc and @achow101. Updated to take @achow101's suggestion, added a commit by @l0rinc with his review feedback (makeseeds output appears unchanged by the robustness/consistency changes to the regexes), updated .gitignore in /contrib/seeds, re-ran the scripts, repushed and updated the PR description.

jonatack force-pushed on Mar 6, 2025

jonatack commented at 8:15 PM on March 6, 2025: member

Re-pushed to add a missing escape to the regexes commit that I had overlooked.

makeseeds: fix incorrect regex

We shouldn't have | at the end of the last clause, as this will make it match
the empty string too (so effectively everything starting with Satoshi: matches).

While doing this, put the | at the beginning of every line of regex rather than
the end, to make it easier to update in the future without accidentally running
into this problem again.

9b0d2e5094

makeseeds: update user agent regex 6ae7a3bc4e

makeseeds: update MIN_BLOCKS, add reminder to README 94e21aa5fc

makeseeds: align I2P column header 2bcccaa410

seeds: add signet seeds 60f17dd816

seeds: add testnet4 seeds c4ed23e539

generate-seeds: update and add signet 98f84d6c23

makeseeds: regex improvements 236687083f

achow101 commented at 8:51 PM on March 6, 2025: member

Needs rebase

jonatack force-pushed on Mar 6, 2025

jonatack commented at 8:53 PM on March 6, 2025: member

Rebased

achow101 commented at 9:12 PM on March 6, 2025: member

ACK 89fd9a4a87d772798671da18538e45f0272365e4

DrahtBot requested review from laanwj on Mar 6, 2025

seeds: update fixed dns seeds 49f155efbf

chainparams: add signet fixed seeds if default network d4ab1150c4

chainparams: remove hardcoded signet seeds 48f07ac9da

seeds: update .gitignore with signet and testnet4 f0b659716b

in contrib/seeds/makeseeds.py:33 in 89fd9a4a87 outdated

  31 |  
  32 | -PATTERN_IPV4 = re.compile(r"^((\d{1,3})\.(\d{1,3})\.(\d{1,3})\.(\d{1,3})):(\d+)$")
  33 | -PATTERN_IPV6 = re.compile(r"^\[([0-9a-z:]+)\]:(\d+)$")
  34 | +PATTERN_IPV4 = re.compile(r"^(([0-2]?\d{1,2})\.([0-2]?\d{1,2})\.([0-2]?\d{1,2})\.([0-2]?\d{1,2})):(\d{1,5})$")
  35 | +PATTERN_IPV6 = re.compile(r"^\[([\da-f:]+)]:(\d{1,5})$", re.IGNORECASE)
  36 |  PATTERN_ONION = re.compile(r"^([a-z2-7]{56}\.onion):(\d+)$")

l0rinc commented at 9:37 PM on March 6, 2025:

For consistency this can also be:

PATTERN_ONION = re.compile(r"^([a-z2-7]{56}\.onion):(\d{1,5})$")

jonatack commented at 9:46 PM on March 6, 2025:

Will do if I need to re-run the scripts and re-push.

in src/kernel/chainparams.cpp:425 in 89fd9a4a87 outdated

 416 | @@ -417,17 +417,15 @@ class SigNetParams : public CChainParams {
 417 |      explicit SigNetParams(const SigNetOptions& options)
 418 |      {
 419 |          std::vector<uint8_t> bin;
 420 | +        vFixedSeeds.clear();
 421 |          vSeeds.clear();
 422 |  
 423 |          if (!options.challenge) {
 424 |              bin = "512103ad5e0edad18cb1f0fc0d28a3d4f1f3e445640337489abb10404f2d1e086be430210359ef5021964fe22d6f8e05b2463c9540ce96883fe3b278760f048f5189f2e6c452ae"_hex_v_u8;
 425 | +            vFixedSeeds = std::vector<uint8_t>(std::begin(chainparams_seed_signet), std::end(chainparams_seed_signet));

l0rinc commented at 9:39 PM on March 6, 2025:

the chainparams: add signet fixed seeds if default network commit should go after seeds: update fixed dns seeds which introduces chainparams_seed_signet
Now that we're pre-clearing it at the beginning, it may be slightly simpler to do:

            vFixedSeeds.assign(std::begin(chainparams_seed_signet), std::end(chainparams_seed_signet));

jonatack commented at 9:42 PM on March 6, 2025:

Already repushed, with no change apart from commit order for the CI (thanks!)

l0rinc commented at 9:44 PM on March 6, 2025:

I see, what do you think about 2)?

jonatack commented at 9:50 PM on March 6, 2025:

No strong opinion, might leave as-is for consistency with the existing code in that file setting vFixedSeeds for mainnet, test and testnet4.

jonatack force-pushed on Mar 6, 2025

jonatack commented at 9:41 PM on March 6, 2025: member

Moved 49f155efbfb65ab60c7c67597f68489893015c71 before the chainparams updates so the commit-by-commit CI passes, no other change.

l0rinc commented at 9:43 PM on March 6, 2025: contributor

I have mostly reviewed the regexes, for the rest it's only a very lightweight ACK f0b659716bd455dca02053df573d888b5a115aa4

I see you've added new values to 23,26,27,28 as well in makeseeds: update user agent regex - I can't validate those, but I assume you had a reason to extend them.

DrahtBot requested review from achow101 on Mar 6, 2025

jonatack commented at 9:45 PM on March 6, 2025: member

I see you've added new values to 23,26,27,28 as well in makeseeds: update user agent rege - I can't validate those, but I assume you had a reason to extend them.

Those are for additional releases, e.g. seen when running git tag -n | sort -V (thanks for reviewing, LMK please if any desired releases are missing).

in contrib/seeds/makeseeds.py:52 in f0b659716b

  66 | +    r"|24\.(0|1|2|99)\.(0|1)"
  67 | +    r"|25\.(0|1|2|99)\.0"
  68 | +    r"|26\.(0|1|2|99)\.0"
  69 | +    r"|27\.(0|1|2|99)\.0"
  70 | +    r"|28\.(0|1|99)\.0"
  71 |      r")")

l0rinc commented at 12:12 PM on March 7, 2025:

Those are for additional releases

Hmmm, if I get the list of every version via git fetch upstream --prune --prune-tags --force && git tag --sort=creatordate | sed -n 's#^v##p', I'm getting 319 tags, many of which (including the latest ones) aren't currently matched by PATTERN_AGENT, see https://www.online-python.com/3iU2QMKu97

Can we generalize the agent pattern to something like this instead?

PATTERN_AGENT = re.compile(r"^/Satoshi:(\d+(\.\d+){0,3}[-\w]*)$")

achow101 commented at 4:19 PM on March 7, 2025:

This pattern is specifically not generalized because we want to restrict to at least post-segwit versions, hence beginning at 0.14.0. We might even want to go further and restrict to post-taproot versions, although the benefit there is much less significant.

Furthermore, while the tag is similar to the user agent, they do not match exactly. Specifically, since the version style change in 22.0, the tag is typically MAJOR.MINOR, but the user agent is always MAJOR.MINOR.PATCH. So 22.0 is 22.0.0 in the user agent. Release candidates additionally produce user agents without the rc suffix. Hence your test script is not representative of what the makeseeds.py actually tests.

Lastly, we explicitly list out the possible versions in order to be more confident that the included seed nodes are running actual Bitcoin Core releases, and also master branch builds.

l0rinc commented at 4:28 PM on March 7, 2025:

Thank you for the correction! Would it make sense to use the generalized, simple regex for extracting the version only and match that against a pre-specified whitelist of valid versions instead?

achow101 commented at 4:38 PM on March 7, 2025:

Would it make sense to use the generalized, simple regex for extracting the version only and match that against a pre-specified whitelist of valid versions instead?

I don't see why that would be better than this approach.

jonatack commented at 5:00 PM on March 7, 2025:

This pattern is specifically not generalized because we want to restrict to at least post-segwit versions, hence beginning at 0.14.0. We might even want to go further and restrict to post-taproot versions, although the benefit there is much less significant.

Furthermore, while the tag is similar to the user agent, they do not match exactly. Specifically, since the version style change in 22.0, the tag is typically MAJOR.MINOR, but the user agent is always MAJOR.MINOR.PATCH. So 22.0 is 22.0.0 in the user agent. Release candidates additionally produce user agents without the rc suffix. Hence your test script is not representative of what the makeseeds.py actually tests.

Lastly, we explicitly list out the possible versions in order to be more confident that the included seed nodes are running actual Bitcoin Core releases, and also master branch builds.

Good explanation. As a follow-up might be worth documenting in makeseeds.py and/or the readme.

jonatack commented at 8:13 PM on March 7, 2025:

Here is a python script (based on yours, @l0rinc, thanks!) that sanity checks the regex in this PR. Only the last entry "24.1rc1" doesn't match (as expected), and a non-existing version like 24.3.0 also doesn't match. See the output of RPC getpeerinfo's "subver" field for the user agent format, e.g., /Satoshi:28.0.0/

<details><summary>regex-test.py</summary><p>

#!/usr/bin/env python3
import re

TAGS = """
0.14.0
0.14.1
0.14.2
0.14.3
0.15.0
0.15.0.1
0.15.1
0.16.0
0.16.1
0.16.2
0.16.3
0.15.2
0.14.3
0.17.0
0.17.0.1
0.17.1
0.18.0
0.18.1
0.17.2
0.19.0
0.19.0.1
0.19.1
0.20.0
0.20.1
0.19.2
0.21.0
0.21.1
22.0.0
0.21.2
0.20.2
23.0.0
24.0.0
22.1.0
24.0.1
23.1.0
24.1.0
23.2.0
25.0.0-final
25.1.0
24.2.0
26.0.0
26.1.0
25.2.0
25.1.0(FutureBit-Apollo-Node)
27.0.0
27.1.0
26.2.0
28.0.0
27.2.0
28.1.0
28.99.0
0.20.0-final
24.1rc1
""".strip().splitlines()

# final regex in this PR31960
pattern = re.compile(
    r"^/Satoshi:("
    r"0\.14\.(0|1|2|3|99)"
    r"|0\.15\.(0|1|2|99)"
    r"|0\.16\.(0|1|2|3|99)"
    r"|0\.17\.(0|0\.1|1|2|99)"
    r"|0\.18\.(0|1|99)"
    r"|0\.19\.(0|1|2|99)"
    r"|0\.20\.(0|1|2|99)"
    r"|0\.21\.(0|1|2|99)"
    r"|22\.(0|1|99)\.0"
    r"|23\.(0|1|2|99)\.0"
    r"|24\.(0|1|2|99)\.(0|1)"
    r"|25\.(0|1|2|99)\.0"
    r"|26\.(0|1|2|99)\.0"
    r"|27\.(0|1|2|99)\.0"
    r"|28\.(0|1|99)\.0"
    r")")

for tag in TAGS:
    if not pattern.match(f"/Satoshi:{tag}/"):
        print(f"Tag did NOT match: {tag}")
        exit(1)

print("All tags matched successfully.")

</p></details>

laanwj commented at 4:33 AM on March 8, 2025:

I don't see why that would be better than this approach.

The only possible advantage i can think of to use a straight-up list and a fixed regexp, would be that it is less prone to subtle errors in the regexp like #31960#pullrequestreview-2654030620 .

l0rinc approved

achow101 commented at 12:25 AM on March 8, 2025: member

ACK f0b659716bd455dca02053df573d888b5a115aa4

laanwj approved

laanwj commented at 4:27 AM on March 8, 2025: member

re-ACK f0b659716bd455dca02053df573d888b5a115aa4

glozow merged this on Mar 11, 2025

glozow closed this on Mar 11, 2025

jonatack deleted the branch on Mar 11, 2025

TheCharlatan referenced this in commit 2cab7a09ea on Mar 16, 2025

stickies-v referenced this in commit d760fd3dda on Mar 17, 2025

stickies-v referenced this in commit cc83553352 on Mar 17, 2025

stickies-v referenced this in commit 2614933f06 on Mar 17, 2025

stickies-v referenced this in commit b70418c5fc on Mar 17, 2025

stickies-v referenced this in commit 69f8a1fe50 on Mar 17, 2025