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

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

Contributors
naiyoma DrahtBot stratospher Sjors laanwj mzumsande

Labels
P2P