#33954 test: add functional test for outbound connection management

ReviewabilityReviewsAgreementSize
#33954 mzumsande test: add functional test for outbound connection managementWaiting on author0 (+2)Strong0 + 398 tests
  • Add functional test coverage for autonomous outbound connection decisions made by bitcoind
  • Enable end-to-end testing of stale tip peers, block-relay rotations, and network diversification
  • Ready to review now because rebase conflicts with master are mechanical
  • Substantive support after running functional tests locally without failures (vasild, Bortlesboat)
  • Concept approval without stated reasons (0xB10C)
  • Unresolved question on whether network connection timing could cause test timeouts (vasild)
  • Open suggestions to pass address data to the factory and clarify a magic number (vasild, Bortlesboat)
  • 0 lines added or modified outside tests
  • 398 lines added or modified in tests
  • 102 lines removed in total
  • 4 files, 3 commits
P2P#58 of 71
P3 · test coverage
  • P3 because it adds functional test coverage for outbound connection heuristics without node changes
  • Helps developers verify autonomous peer selection rules that previously lacked test coverage
  • Ranking pass: Valuable new coverage but test-only; ranked at the head of the test cluster rather than among bug fixes.
Ranking notes
Test infrastructure#13 of 54
P3 · test coverage
  • P3 because it generalizes proxy redirection in the test framework for autonomous outbound connections
  • Deduplicates custom proxy logic across tests and provides a reusable harness for P2P testing
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds an `auto_outbound_mode` setting to the functional test framework and introduces a new test script `p2p_outbound_management.py`. In this mode, the node under test is allowed to autonomously initiate outbound connections to addresses in addrman via a SOCKS5 proxy that redirects them to Python P2P peers or other test nodes. The PR also refactors `p2p_private_broadcast.py` to use the shared test framework implementation instead of its own custom proxy redirect logic.

Problem: P2P logic where bitcoind autonomously chooses when and where to initiate outbound connections has historically lacked functional test coverage because standard tests establish connections inward or use manual `add_outbound_p2p_connection` calls. Important outbound connection management behaviors—such as opening an extra connection on a stale tip, periodic block-relay-only peer rotation, and network diversification—could not be easily tested end-to-end.

Evidence of importance:

  • PR description notes that auto_outbound_mode 'allows to test the code in which the node decides to make connections - in contrast to the existing add_outbound_p2p_connection where the functional test initiates the connections.'
  • Adds functional test coverage for previously uncovered or under-tested outbound P2P mechanisms merged across multiple past PRs: stale tip extra connection (#11560), periodic block-relay-only connections (#19858), and network diversification (#27213).

Scope: The PR extracts and generalizes SOCKS5 redirection machinery introduced in #29415 into `BitcoinTestFramework`, converts `p2p_private_broadcast.py` to use it, and adds the new test `p2p_outbound_management.py`.

Risk of a mistake: low (tests). Only touches Python functional tests and test framework utilities; any bug would result in flaky CI runs or failed tests without affecting production node behavior.

Changed since the previous assessment: Diff provided matches the scope of the previous description; no structural changes to the commits or implementation approach.

Reviewability

Paused · Waiting on author

Open objections from Bortlesboat, vasild with no author reply for 9 days; review would repeat what has been asked.

Waiting on (from the thread read):

  • author: author addressing feedback and finishing non-trivial rebase before undrafting 2026-08-28
  • author: address review feedback and complete rebase before marking ready for review 2026-08-28

Agreement

Strong

substantive support, no open objection (vasild, Bortlesboat)

Thread notes: Converted vasild's earlier interface comments from objections to suggestions per current schema guidelines. Added 0xB10C to participants with Concept ACK, and recorded open suggestions and question follow-ups from vasild and Bortlesboat. / Converted vasild's interface comments from objections to suggestions per current rules. Added missing questions and suggestions from vasild and Bortlesboat, as well as 0xB10C's Concept ACK. The PR remains in draft awaiting author updates after rebasing.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
vasild (member)suggestion, interfaceintroduce proxy_port(n) rather than misusing p2p_port(n) for the SOCKS5 proxyfixed · fix pushed · author set port to 0 as suggested by vasild once port 0 support was addedansweredyes2025-12-08: 'This is somewhat of a misuse of `p2p_port()`. It works, but looking at it I wonder if it can be done better. Maybe introduce `proxy_port(n)`?'
Status from: 2026-02-24: mzumsande: 'done'
vasild (member)suggestion, interfaceallow tests using auto_outbound_mode to override redirection decisions and listener creationfixed · fix pushed · author implemented auto_outbound_factoryansweredyes2025-12-08: 'then it is possible to override from the test: * the decision where to redirect and * the listener creation'
Status from: 2025-12-11: mzumsande: 'done, used the name `auto_outbound_factory`'
maflcko (member)objection, maintenancemanually performing arithmetic on ports risks port collisions across parallel tests or invalid port rangesfixed · fix pushed · author cherry-picked commit enabling port=0 for OS port allocationansweredyes2025-12-11: 'my preference would be to not call `+` or `+=` on an integral port returned by a helper function. This will lead to issues down the line'
Status from: 2026-01-02: mzumsande: 'I cherry-picked the commit from #34186, making it possible to use `port=0` and then let the OS deal with it - much better than assigning the port by the test itself.'
vasild (member)suggestion, interfaceunify auto_outbound_factory and _create_auto_outbound_listener interfaces to return the listener and remove conditional logicfixed · fix pushed · author unified the listener factory interfaceansweredyes2026-01-09: 'Maybe get `auto_outbound_factory()` and `_create_auto_outbound_listener()` to have the same interface?'
Status from: 2026-02-24: mzumsande: 'I like that suggestion - took it!'
vasild (member)suggestion, interfaceuse format_addr_port() for the proxy address in case an IPv6 address is configuredaccepted · author noted proxy address is hardcoded to 127.0.0.1; vasild conceded it only matters if changed in the futureansweredyes2026-01-09: 'Maybe this needs `format_addr_port()` as well, in case the address happens to be IPv6 and needs to be enclosed in `[]`?'
Status from: 2026-02-25: vasild: 'Right, only if changed in the future.'
vasild (member)suggestion, interfacemake node argument consistent across helper functions or drop it from all since all operate on self.nodes[0]fixed · fix pushed · author dropped node argument across helper functionsansweredyes2026-01-09: 'Maybe add such argument also to `disconnect_last_block_relay_peer()` for consistency?'
Status from: 2026-02-24: mzumsande: 'I decided to drop the node arg everywhere.'
vasild (member)question, correctnesswhether a stale-tip extra-outbound connection could be initiated between advancing mocktime and calling generateanswered · author explained CheckForStaleTipAndEvictPeers fires on steady clock independent of mocktimeansweredyes2026-01-09: 'Is there an unlikely scenario that a stale-tip extra-outbound connection is initiated after advancing the mocktime and before `self.generate(node, 1)`?'
Status from: 2026-02-24: mzumsande: 'I think probably not in practice - `CheckForStaleTipAndEvictPeers` fires every 45 seconds (on a steady clock, not influenced by the mocktime bump).'
vasild (member)question, correctnesswhether the node could already have connections to all reachable networks when the function starts, causing has_extra_full_outbound_peer to time outcontested · vasild followed up asking whether Tor and CJDNS connections could be made before wait_for_network_specific_peer is calledansweredyes2026-01-09: 'Is it possible that the node already has at least one connection to all reachable networks when this function starts? Would that result in timeouts due to `has_extra_full_outbound_peer(node)` never returning `True`?'
Status from: 2026-02-25: vasild: 'What about the tor and cjdns connections being established after adding to addrman and before `wait_for_network_specific_peer()` is called?'
vasild (member)suggestion, interfacepass requested destination address and port to auto_outbound_factory in addition to connection indexopenansweredno2026-02-25: 'auto_outbound_factory() might benefit from getting more information about the connection, in addition to its index, like the requested address and port'
Bortlesboat (contributor)suggestion, maintenanceadd a comment explaining the origin and logic behind the magic number 46openansweredno2026-03-15: 'Could you add a brief comment explaining where 46 comes from?'

Support:

  • vasild (member) (Concept ACK, 2025-11-27): Concept ACK, thanks! [not substantive]
  • vasild (member) (ACK, 2026-01-09) [tested: p2p_outbound_management.py, test_runner.py]: I ran `test_runner.py` and `p2p_outbound_management.py` a bunch of times locally and did not observe timeouts.
  • vasild (member) (ACK, 2026-02-25): ACK c733d112309321e3671d31cdca92e7aaa9274975 [not substantive]
  • Bortlesboat (contributor) (ACK, 2026-03-15) [tested: p2p_outbound_management.py, p2p_private_broadcast.py]: ACK c733d112309321e3671d31cdca92e7aaa9274975. Ran p2p_outbound_management.py and p2p_private_broadcast.py locally on Ubuntu 24.04, both pass.
  • 0xB10C (member) (Concept ACK, 2026-10-23): Concept ACK [not substantive]

Participants:

  • vasild (member, support, 17 statements 2025-11-27 to 2026-02-25): Concept ACKed and twice ACKed after local testing; offered suggestions and questions on test design
  • maflcko (member, objection, 1 statements 2025-12-11 to 2025-12-11): advised against manual port arithmetic to prevent port collisions and range issues
  • Bortlesboat (contributor, support, 2 statements 2026-03-15 to 2026-03-15): ACKed after testing both test scripts locally on Ubuntu 24.04; requested a clarifying comment for scheduler constant 46
  • 0xB10C (member, support, 1 statements 2026-10-23 to 2026-10-23): Concept ACK

Files

500 lines under test/bench/ci.

  • test/functional/p2p_outbound_management.py +250/-0
  • test/functional/p2p_private_broadcast.py +30/-101
  • test/functional/test_framework/test_framework.py +117/-1
  • test/functional/test_runner.py +1/-0

Card

The short text the ranking pass compares across PRs.

Adds an auto_outbound_mode helper to the functional test framework and introduces functional test coverage for automatic outbound connection management. Functional tests typically rely on the test runner manually initiating connections, leaving the node's autonomous outbound connection decisions untested in an end-to-end environment. By routing autonomous addrman connections through a SOCKS5 proxy back to controllable Python P2P test peers, this PR exercises stale-tip extra connection logic, scheduled block-relay-only peer probing, and network diversification across IPv4, Tor, and CJDNS. It also dedupes test proxy logic previously isolated inside p2p_private_broadcast.py.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-24T01:18, confidence high, input hash 34162b8654d5bb2b
thread read through 2026-10-23 (33 statements), head abf8fc4f7a
labels: Tests, Needs rebase, CI failed · draft

Categories

P2P

P3 · test coverage (ranking pass: Valuable new coverage but test-only; ranked at the head of the test cluster rather than among bug fixes.)

Adds end-to-end test coverage for core P2P outbound connection management heuristics that were previously difficult to test. As noted in the description, it tests autonomous connection decisions merged across past PRs (#11560, #19858, #27213) without altering node logic.

Membership: Adds functional test coverage for outbound connection management, including stale-tip extra connections, block-relay-only peer rotation, and network diversification.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1

Test infrastructure

P3 · test coverage

Introduces a generalized framework capability for nodes to autonomously initiate outbound connections to simulated peers via a proxy. This deduplicates proxy redirect machinery previously isolated in `p2p_private_broadcast.py` and enables end-to-end outbound P2P tests.

Membership: Label 'Tests' and changes to `test/functional/test_framework/test_framework.py` adding `auto_outbound_mode` and SOCKS5 proxy redirection machinery.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 1