#36014 init: ignore repeated `-addnode` startup values
https://github.com/bitcoin/bitcoin/pull/36014 · · +59/-9 in 4 files, 2 commits · labels: none
Goal
- Prevent duplicate manual node entries from repeated startup options
- Stops redundant DNS queries and repeated connection retries when nodes are unreachable
Routes startup `-addnode` options through `CConnman::AddNode` instead of appending them directly to `m_added_node_params`, dropping duplicates and logging a warning. Additionally enhances `AddNode` to recognize host-port equivalence when the explicit port matches the default port.
Problem: Node operators passing repeated `-addnode` arguments or mixing bare hostnames with default-port hostnames end up with duplicate entries stored in the connection manager. While the target is unreachable, each entry triggers independent DNS resolutions and connection retries every loop.
Category: P2P (#33 of 65)
P3 · bug fix
- P3 because redundant DNS queries and retries only occur with misconfigured duplicate inputs
- Modest benefit from eliminating unnecessary network chatter for manual connections
Fixes a minor peer connection management bug where duplicate configuration entries cause unnecessary DNS queries and sequential connection retries. The impact is modest because it requires misconfigured or duplicate `-addnode` parameters, but aligning startup and RPC behavior eliminates redundant network chatter.
Membership: Modifies CConnman startup added-node handling and manual peer connection deduplication in src/net.cpp and src/net.h.
Factors: security/stability 1, bug 1, performance 1, user value 1, leverage 0
Reviewability: Ready
- Ready for review
- Clean code, passing CI, and suggested changes have been addressed
The code is clean, CI passes, and recent reviewer comments were addressed or labeled non-blocking nits.
Author status: active
Resolved concerns:
- pablomartin4btc suggested checking default-port equivalence in `AddNode`, which was implemented in commit beadf6a96d.
- danielabrozzoni suggested routing startup options through `AddNode` directly in `CConnman`, implemented in commit 4ebff1af2a.
Agreement: Strong
- Strong support for deduplicating added node inputs
- Approved after adding host-port equivalence handling (pablomartin4btc)
- Concept approval recommending routing startup options through connection manager (danielabrozzoni)
Strong: substantive ACK after suggested port and Init improvements were implemented.
Both reviewers supported deduplicating `-addnode` inputs and provided design suggestions that the author implemented as co-authored commits, leading to a formal ACK.
- pablomartin4btc suggested host-port equivalence and gave a full ACK on the latest push.
- danielabrozzoni provided a Concept ACK and recommended using `CConnman::AddNode` at startup.
Review verdicts (DrahtBot): 1
- ACK: pablomartin4btc
- Concept ACK: danielabrozzoni
Files
38 lines under test/bench/ci.
- test/functional/feature_config_args.py +23/-0
- src/net.cpp +14/-2
- src/test/net_peer_connection_tests.cpp +15/-0
- src/net.h +7/-7
Card
This PR deduplicates startup `-addnode` entries by processing them through `CConnman::AddNode` and adds host-port equivalence checks so that default-port variants are treated as identical. It solves redundant sequential retries, DNS resolutions, and log spam when equivalent manual peers are configured. Reviewers strongly support the approach and contributed suggestions that were incorporated into the current commits. The PR is fully reviewable and has an explicit ACK from pablomartin4btc.