test: avoid non-loopback network traffic from node_init_tests/init_test #35193

pull vasild wants to merge 1 commits into bitcoin:master from vasild:avoid_internet_traffic_from_init_test changing 2 files +5 −1
  1. vasild commented at 11:05 AM on May 1, 2026: contributor

    The test node_init_tests/init_test calls: AppInitMain() -> StartMapPort() -> StartThreadMapPort() -> ThreadMapPort() -> ProcessPCP() -> PCPRequestPortMap() -> CreateSock() and on the returned value from CreateSock() it calls the Connect() method.

    Thus, change BasicTestingSetup::BasicTestingSetup() to set -natpmp to 0. This way node_init_tests/init_test or other tests will not do network activity due to ThreadMapPort().

    Also add a comment about natpmp=0 in test/functional/test_framework/util.py.

    Also set -dnsseed=0 in BasicTestingSetup::BasicTestingSetup() to avoid DNS queries.

  2. DrahtBot added the label Tests on May 1, 2026
  3. DrahtBot commented at 11:05 AM on May 1, 2026: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

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

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK ryanofsky, fjahr

    If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  4. vasild commented at 11:18 AM on May 1, 2026: contributor

    @fjahr, @ryanofsky, maybe you would be interested in reviewing this PR because it is part of #31349 which you ACKed before in #31349 (comment) and #31349#pullrequestreview-3589303104

  5. fjahr commented at 10:58 AM on May 2, 2026: contributor

    ACK 7545f8dfd3012eff018b31110daee227cb87fe85

    Verified this is just the same commit pulled out of https://github.com/bitcoin/bitcoin/pull/31349

  6. ferminquant commented at 4:28 PM on May 8, 2026: none

    I tested this with strace -f -e trace=network and confirmed that the patch does stop the NAT-PMP/PCP ThreadMapPort() traffic described in the PR body.

    However, given the PR title and the link to #31349 / #31339, I expected node_init_tests/init_test to avoid non-loopback traffic more generally. The test still performs DNS lookups for x9.dummySeed.invalid through the system resolver. In my environment that resolver is local-routed, but #31349 discusses cases where this same DNS lookup goes to non-loopback resolvers such as 8.8.8.8 or 1111:1111::1.

    As a control, rerunning the test with -dnsseed=0 removed those DNS queries, leaving only the expected 127.0.0.1 Tor-control attempt.

    So I think the NAT-PMP part works, but either BasicTestingSetup should also force -dnsseed=0, or the PR title/description should make clear that this only addresses the ThreadMapPort() source of traffic rather than all non-loopback traffic from node_init_tests/init_test.

  7. ryanofsky approved
  8. ryanofsky commented at 10:50 PM on May 13, 2026: contributor

    Code review ACK 7545f8dfd3012eff018b31110daee227cb87fe85

  9. vasild commented at 9:20 AM on May 15, 2026: contributor

    The test still performs DNS lookups ... -dnsseed=0 removed those DNS queries

    Oh, looks like you figured out the unexplained local failures reported on #31349 plus proposed a fix!

  10. test: avoid non-loopback network traffic from node_init_tests/init_test
    The test calls:
    `AppInitMain()` -> `StartMapPort()` -> `StartThreadMapPort()` ->
    `ThreadMapPort()` -> `ProcessPCP()` -> `PCPRequestPortMap()` ->
    `CreateSock()` and on the returned value from `CreateSock()` it calls
    the `Connect()` method.
    
    Thus, change `BasicTestingSetup::BasicTestingSetup()` to set `-natpmp`
    to 0. This way `node_init_tests/init_test` or other tests will not do
    network activity due to `ThreadMapPort()`.
    
    Also add a comment about `natpmp=0` in
    `test/functional/test_framework/util.py`.
    
    Also set `-dnsseed=0` in `BasicTestingSetup::BasicTestingSetup()` to
    avoid DNS queries.
    1c500b1709
  11. vasild force-pushed on May 15, 2026
  12. vasild commented at 10:17 AM on May 15, 2026: contributor

    7545f8dfd3012eff018b31110daee227cb87fe85...1c500b17098e2c65129a1dda7345b9961f4a951f: added -dnsseed=0 as per #35193 (comment)

  13. ryanofsky approved
  14. ryanofsky commented at 5:39 PM on May 18, 2026: contributor

    Code review ACK 1c500b17098e2c65129a1dda7345b9961f4a951f, just disabling -dnsseed since previous review, which makes sense.

    Curious if it also makes sense to disable dnsseed in test/functional/test_framework/util.py?

  15. DrahtBot requested review from fjahr on May 18, 2026
  16. fjahr commented at 5:52 PM on May 18, 2026: contributor

    re-ACK 1c500b17098e2c65129a1dda7345b9961f4a951f

  17. fanquake merged this on May 19, 2026
  18. fanquake closed this on May 19, 2026

  19. yuvicc referenced this in commit 9df13f29ba on May 26, 2026
  20. vasild deleted the branch on Jun 18, 2026


fjahr

Labels

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: 2026-07-02 03:50 UTC

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