net: update comment for service bit support info for seed.bitcoin.sipa.be #29809

pull naiyoma wants to merge 2 commits into bitcoin:master from naiyoma:update-dns-seed-flags changing 1 files +1 −1
  1. naiyoma commented at 8:40 pm on April 4, 2024: contributor
    This PR updates the comment regarding the supported service bits for seed.bitcoin.sipa.be based on this reference: https://github.com/sipa/bitcoin-seeder/blob/ff482e465ff84ea6fa276d858ccb7ef32e3355d3/main.cpp#L208
  2. net: update service bit support info for seed.bitcoin.sipa.be 48afa53282
  3. DrahtBot commented at 8:40 pm on April 4, 2024: contributor

    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
    Concept ACK stratospher

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

  4. DrahtBot added the label P2P on Apr 4, 2024
  5. in src/kernel/chainparams.cpp:134 in 48afa53282 outdated
    130@@ -131,7 +131,7 @@ class CMainParams : public CChainParams {
    131         // This is fine at runtime as we'll fall back to using them as an addrfetch if they don't support the
    132         // service bits we want, but we should get them updated to support all service bits wanted by any
    133         // release ASAP to avoid it where possible.
    134-        vSeeds.emplace_back("seed.bitcoin.sipa.be."); // Pieter Wuille, only supports x1, x5, x9, and xd
    135+        vSeeds.emplace_back("seed.bitcoin.sipa.be."); // Pieter Wuille, only supports x1, x5, x9, xd, x809, x849, x400, x404, x408, x448, xc08, xc48 and x40c
    


    stratospher commented at 3:52 am on April 5, 2024:
    48afa53: there’s also x49.
  6. stratospher commented at 4:40 am on April 5, 2024: contributor

    Concept ACK. thanks for opening this! it’s easy to forget to update support for service bit filtering here and the list just keeps getting longer.

    another possibility could be just mentioning that the seeder supports service bit filtering and linking to the seeder repo to check the exact list of service bits supported. curious to know what other people think.

  7. net: update comment for service bit support info for seed.bitcoin.sipa.be 9de8cfb04e
  8. naiyoma commented at 11:55 am on April 5, 2024: contributor

    @stratospher thanks for the review . I think it would be more ideal to simply reference the seeder repo. However, for the sake of uniformity, I believe it’s better to continue listing them as is currently being done.

    Concept ACK. thanks for opening this! it’s easy to forget to update support for service bit filtering here and the list just keeps getting longer.

    another possibility could be just mentioning that the seeder supports service bit filtering and linking to the seeder repo to check the exact list of service bits supported. curious to know what other people think.

  9. Sjors commented at 2:35 pm on April 5, 2024: member

    You can’t know by looking at the code which configuration @sipa - or anyone else running that code - is using. And keeping track of all permutations gets quite noisy.

    For my own seed I’ve occasionally added more filters for new features that were being developed. I wouldn’t want to keep making pull requests to change the comment (mine doesn’t mention any).

    Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py

  10. laanwj commented at 2:56 pm on April 8, 2024: member

    Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py

    This would be nice, and fairly easy if we can assume disallowed filters return nothing/an error, but seems like there’s a exponential number of potential combinations to test? Or is something smarter than brute force possible?

  11. naiyoma commented at 6:09 am on April 9, 2024: contributor
    @Sjors and @laanwj Thank you for the review. I’ll look into how https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py can be modified . I’m pretty new to this codebase, and I was trying to figure out who supports which service bits.
  12. naiyoma marked this as a draft on Apr 9, 2024
  13. DrahtBot added the label CI failed on Apr 18, 2024
  14. mzumsande commented at 5:36 pm on April 23, 2024: contributor
    I also think it would be good to get this out of chainparams, bitcoind only ever uses “x9” anyway as far as I can see, so I’m not sure why the status of other combinations needs to be tracked here. Maybe adding the info in the first place was meant as a service to other software that would actually use those other combinations?
  15. DrahtBot removed the label CI failed on Apr 24, 2024
  16. naiyoma commented at 10:43 am on April 26, 2024: contributor

    I also think it would be good to get this out of chainparams, bitcoind only ever uses “x9” anyway as far as I can see, so I’m not sure why the status of other combinations needs to be tracked here. Maybe adding the info in the first place was meant as a service to other software that would actually use those other combinations?

    I’m working on this script to filter seeds, I think it’s okay to delete all the combinations comments and just reference the script

  17. DrahtBot added the label CI failed on Oct 19, 2024
  18. DrahtBot removed the label CI failed on Oct 24, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-11 06:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me