Skip to content

Conversation

RF5
Copy link
Contributor

@RF5 RF5 commented Apr 10, 2022

This PR attempts to address some of the areas of improvement raised in #17020 . Concretely, my proposed change is fairly minor but addresses the following changes to makeseeds.py:

  • Increase max seeds per ASN for IPv6 to 10 as recommended here, while keeping max seeds per ASN for IPv4 at 2.
  • Bump MIN_BLOCKS to 730000.
  • Improved script clarity: added function types and more docs to functions, added progress indicator when performing ASN lookup, and change string formatting to better align with bitcoin python style guidelines

With the different ASN limits for IPv4 and IPv6, and the new minimum block requirement, the current stats look look like:

  IPv4   IPv6  Onion Pass
470689  73238      0 Initial
470689  73238      0 Skip entries with invalid address
470689  73238      0 After removing duplicates
470688  73238      0 Skip entries from suspicious hosts
  6098   1676      0 Enforce minimal number of blocks
  5252   1443      0 Require service bit 1
  3812    898      0 Require minimum uptime
  3738    877      0 Require a known and recent user agent
  3715    869      0 Filter out hosts with multiple bitcoin ports
   512    512      0 Look up ASNs and limit results per ASN and per net

The new ASN max seeds of 10 allows for 512 IPv6 addresses to be included, up from the ~150 that was filtered by the previous version.

While there is more to do for #17020 , these changes I think are fairly isolated from the rest and should make it a bit easier for others to get up to speed with what the functions in the script do.

@DrahtBot
Copy link
Contributor

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24824 (net: create IP to ASN database from file - makeseeds.py by russeree)

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.

@RF5 RF5 mentioned this pull request Apr 12, 2022
11 tasks
@laanwj
Copy link
Member

laanwj commented Apr 13, 2022

Thanks for working on this.
Concept ACK
Code review ACK 30aa28e (I'm not sure about some of the mypy annotations see below)
The progress indicator is neat!

@laanwj
Copy link
Member

laanwj commented Apr 14, 2022

ACK after squashing commits.

@RF5
Copy link
Contributor Author

RF5 commented Apr 14, 2022

Squashed, thanks again for the help reviewing.

@laanwj
Copy link
Member

laanwj commented Apr 15, 2022

Concept and code review ACK c457fb1

@laanwj laanwj merged commit 7da4f65 into bitcoin:master Apr 15, 2022
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Post-merge ACK, modulo suggestions in #24863.

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 18, 2022
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 5, 2022
Summary:
Backport of [[ bitcoin/bitcoin#24818 | core#24818 ]]

This patch increases the number of ipv6 seeds from 2 to 10 per ASN and
increases MIN_BLOCKS to 730000.

Most of the refactors are not big improvements in clarity but it does bring
 our code more inline with Bitcoin Core.

Test Plan:
  python3 makeseeds.py < seeds_main.txt > nodes_main.txt
  python3 makeseeds.py < seeds_test.txt > nodes_test.txt
  git diff

Diff should be more additions than removals and retain most of the old entries.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12141
@bitcoin bitcoin locked and limited conversation to collaborators Apr 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants