Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31646: test: avoid internet traffic
Browse files Browse the repository at this point in the history
2ed161c test: avoid generating non-loopback traffic from p2p_dns_seeds.py (Vasil Dimov)
a5746dc test: avoid generating non-loopback traffic from feature_config_args.py (Vasil Dimov)
6b3f6ea test: avoid generating non-loopback traffic from p2p_seednode.py (Vasil Dimov)

Pull request description:

  Avoid generating outbound traffic on a non-loopback interface during tests. Fix all tests, including ones that generate DNS traffic.

  ---

  This is a subset of bitcoin/bitcoin#31349 containing only the changes to the tests, without the CI changes to detect future regressions.

ACKs for top commit:
  kevkevinpal:
    light code review ACK [2ed161c](bitcoin/bitcoin@2ed161c)
  brunoerg:
    code review ACK 2ed161c
  BrandonOdiwuor:
    Code Review ACK 2ed161c
  jonatack:
    ACK 2ed161c

Tree-SHA512: 34dcd4a4d0c4edaa68cc7263540af01afd6ef6e90fd6a43dcd1e989dbd7d32cb2c24ad9e68fde75866a935e6dbfe10d45c10f0bc674f40f9ac72ef964e5a380a
  • Loading branch information
glozow committed Jan 14, 2025
2 parents 35bf426 + 2ed161c commit 7cd862a
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 17 deletions.
16 changes: 9 additions & 7 deletions test/functional/feature_config_args.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import tempfile
import time

from test_framework.netutil import UNREACHABLE_PROXY_ARG
from test_framework.test_framework import BitcoinTestFramework
from test_framework.test_node import ErrorMatch
from test_framework import util
Expand Down Expand Up @@ -227,8 +228,8 @@ def test_invalid_command_line_options(self):
)

def test_log_buffer(self):
with self.nodes[0].assert_debug_log(expected_msgs=['Warning: parsed potentially confusing double-negative -connect=0\n']):
self.start_node(0, extra_args=['-noconnect=0'])
with self.nodes[0].assert_debug_log(expected_msgs=['Warning: parsed potentially confusing double-negative -listen=0\n']):
self.start_node(0, extra_args=['-nolisten=0'])
self.stop_node(0)

def test_args_log(self):
Expand Down Expand Up @@ -259,6 +260,7 @@ def test_args_log(self):
'-rpcpassword=',
'-rpcuser=secret-rpcuser',
'-torpassword=secret-torpassword',
UNREACHABLE_PROXY_ARG,
])
self.stop_node(0)

Expand Down Expand Up @@ -307,7 +309,7 @@ def test_seed_peers(self):
],
timeout=10,
):
self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}'])
self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}', UNREACHABLE_PROXY_ARG])

# Only regtest has no fixed seeds. To avoid connections to random
# nodes, regtest is the only network where it is safe to enable
Expand Down Expand Up @@ -355,7 +357,7 @@ def test_seed_peers(self):
],
timeout=10,
):
self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=1', '-addnode=fakenodeaddr', f'-mocktime={start}'])
self.start_node(0, extra_args=['-dnsseed=0', '-fixedseeds=1', '-addnode=fakenodeaddr', f'-mocktime={start}', UNREACHABLE_PROXY_ARG])
with self.nodes[0].assert_debug_log(expected_msgs=[
"Adding fixed seeds as 60 seconds have passed and addrman is empty",
]):
Expand All @@ -371,18 +373,18 @@ def test_connect_with_seednode(self):
# When -connect is supplied, expanding addrman via getaddr calls to ADDR_FETCH(-seednode)
# nodes is irrelevant and -seednode is ignored.
with self.nodes[0].assert_debug_log(expected_msgs=seednode_ignored):
self.start_node(0, extra_args=['-connect=fakeaddress1', '-seednode=fakeaddress2'])
self.start_node(0, extra_args=['-connect=fakeaddress1', '-seednode=fakeaddress2', UNREACHABLE_PROXY_ARG])

# With -proxy, an ADDR_FETCH connection is made to a peer that the dns seed resolves to.
# ADDR_FETCH connections are not used when -connect is used.
with self.nodes[0].assert_debug_log(expected_msgs=dnsseed_ignored):
self.restart_node(0, extra_args=['-connect=fakeaddress1', '-dnsseed=1', '-proxy=1.2.3.4'])
self.restart_node(0, extra_args=['-connect=fakeaddress1', '-dnsseed=1', UNREACHABLE_PROXY_ARG])

# If the user did not disable -dnsseed, but it was soft-disabled because they provided -connect,
# they shouldn't see a warning about -dnsseed being ignored.
with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started,
unexpected_msgs=dnsseed_ignored):
self.restart_node(0, extra_args=['-connect=fakeaddress1', '-proxy=1.2.3.4'])
self.restart_node(0, extra_args=['-connect=fakeaddress1', UNREACHABLE_PROXY_ARG])

# We have to supply expected_msgs as it's a required argument
# The expected_msg must be something we are confident will be logged after the unexpected_msg
Expand Down
9 changes: 5 additions & 4 deletions test/functional/p2p_dns_seeds.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import itertools

from test_framework.netutil import UNREACHABLE_PROXY_ARG
from test_framework.p2p import P2PInterface
from test_framework.test_framework import BitcoinTestFramework

Expand All @@ -14,7 +15,7 @@ class P2PDNSSeeds(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
self.num_nodes = 1
self.extra_args = [["-dnsseed=1"]]
self.extra_args = [["-dnsseed=1", UNREACHABLE_PROXY_ARG]]

def run_test(self):
self.init_arg_tests()
Expand All @@ -29,11 +30,11 @@ def init_arg_tests(self):
self.log.info("Check that setting -connect disables -dnsseed by default")
self.nodes[0].stop_node()
with self.nodes[0].assert_debug_log(expected_msgs=["DNS seeding disabled"]):
self.start_node(0, [f"-connect={fakeaddr}"])
self.start_node(0, extra_args=[f"-connect={fakeaddr}", UNREACHABLE_PROXY_ARG])

self.log.info("Check that running -connect and -dnsseed means DNS logic runs.")
with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
self.restart_node(0, [f"-connect={fakeaddr}", "-dnsseed=1"])
self.restart_node(0, extra_args=[f"-connect={fakeaddr}", "-dnsseed=1", UNREACHABLE_PROXY_ARG])

self.log.info("Check that running -forcednsseed and -dnsseed=0 throws an error.")
self.nodes[0].stop_node()
Expand Down Expand Up @@ -88,7 +89,7 @@ def force_dns_test(self):
with self.nodes[0].assert_debug_log(expected_msgs=["Loading addresses from DNS seed"], timeout=12):
# -dnsseed defaults to 1 in bitcoind, but 0 in the test framework,
# so pass it explicitly here
self.restart_node(0, ["-forcednsseed", "-dnsseed=1"])
self.restart_node(0, ["-forcednsseed", "-dnsseed=1", UNREACHABLE_PROXY_ARG])

# Restore default for subsequent tests
self.restart_node(0)
Expand Down
15 changes: 9 additions & 6 deletions test/functional/p2p_seednode.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import random
import time

from test_framework.netutil import UNREACHABLE_PROXY_ARG
from test_framework.test_framework import BitcoinTestFramework

ADD_NEXT_SEEDNODE = 10
Expand All @@ -17,32 +18,34 @@
class P2PSeedNodes(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
# Specify a non-working proxy to make sure no actual connections to random IPs are attempted.
self.extra_args = [[UNREACHABLE_PROXY_ARG]]
self.disable_autoconnect = False

def test_no_seednode(self):
self.log.info("Check that if no seednode is provided, the node proceeds as usual (without waiting)")
with self.nodes[0].assert_debug_log(expected_msgs=[], unexpected_msgs=["Empty addrman, adding seednode", f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode"], timeout=ADD_NEXT_SEEDNODE):
self.restart_node(0)
self.restart_node(0, extra_args=self.nodes[0].extra_args)

def test_seednode_empty_addrman(self):
seed_node = "0.0.0.1"
seed_node = "25.0.0.1"
self.log.info("Check that the seednode is immediately added on bootstrap on an empty addrman")
with self.nodes[0].assert_debug_log(expected_msgs=[f"Empty addrman, adding seednode ({seed_node}) to addrfetch"], timeout=ADD_NEXT_SEEDNODE):
self.restart_node(0, extra_args=[f'-seednode={seed_node}'])
self.restart_node(0, extra_args=self.nodes[0].extra_args + [f'-seednode={seed_node}'])

def test_seednode_non_empty_addrman(self):
self.log.info("Check that if addrman is non-empty, seednodes are queried with a delay")
seed_node = "0.0.0.2"
seed_node = "25.0.0.2"
node = self.nodes[0]
# Fill the addrman with unreachable nodes
for i in range(10):
ip = f"{random.randrange(128,169)}.{random.randrange(1,255)}.{random.randrange(1,255)}.{random.randrange(1,255)}"
port = 8333 + i
node.addpeeraddress(ip, port)

# Restart the node so seednode is processed again. Specify a non-working proxy to make sure no actual connections to random IPs are attempted.
# Restart the node so seednode is processed again.
with node.assert_debug_log(expected_msgs=["trying v1 connection"], timeout=ADD_NEXT_SEEDNODE):
self.restart_node(0, extra_args=[f'-seednode={seed_node}', '-proxy=127.0.0.1:1'])
self.restart_node(0, extra_args=self.nodes[0].extra_args + [f'-seednode={seed_node}'])

with node.assert_debug_log(expected_msgs=[f"Couldn't connect to peers from addrman after {ADD_NEXT_SEEDNODE} seconds. Adding seednode ({seed_node}) to addrfetch"], unexpected_msgs=["Empty addrman, adding seednode"], timeout=ADD_NEXT_SEEDNODE * 1.5):
node.setmocktime(int(time.time()) + ADD_NEXT_SEEDNODE + 1)
Expand Down
4 changes: 4 additions & 0 deletions test/functional/test_framework/netutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
import array
import os

# Easily unreachable address. Attempts to connect to it will stay within the machine.
# Used to avoid non-loopback traffic or DNS queries.
UNREACHABLE_PROXY_ARG = '-proxy=127.0.0.1:1'

# STATE_ESTABLISHED = '01'
# STATE_SYN_SENT = '02'
# STATE_SYN_RECV = '03'
Expand Down

0 comments on commit 7cd862a

Please sign in to comment.