init: ignore repeated `-addnode` startup values #36014

Startup -addnode values are copied directly into the connection manager's added-node list. Unlike runtime addnode add calls, this path does not reject repeated values.

For example:

-addnode=example.com
-addnode=example.com

Both entries are currently stored. While the destination is disconnected, ThreadOpenAddedConnections() processes each entry during every retry cycle.

This can cause redundant DNS lookups, connection attempts, and log messages. getaddednodeinfo also reports the repeated entry.

This change is complementary to, and independent of, #35600.

#35600 prevents overlapping manual connection attempts by tracking destinations while a connection attempt is in progress. This PR instead removes repeated -addnode values before the connection threads start, preventing duplicate stored entries and sequential redundant retries.

It does not replace or broaden #35600's in-flight connection handling.

<!--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/36014.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process.

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.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #35940 (net: allow selecting BIP152 high-bandwidth peers with -addnode by w0xlt)
• #34844 (util: Add util::NotNull<SmartPtrType> by maflcko)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Concept ACK

Comparing this patch to the validation performed in CConnman::AddNode() (the RPC path), which resolves the target — full resolution is out of scope as clearly explained in the description and commit body. What about also checking the port (perhaps using SplitHostPort), so e.g. node.example and node.example:8333 (mainnet default port) would be treated as the same target, and the second one excluded as a duplicate?

Concept ACK on ignoring repeated values.

When looking at how addnode handles repeated values, I noticed that it uses CConnMan::AddNode, which invokes LookupNumeric to parse IP addresses and remove equivalent representations (it doesn't do any DNS resolution). For consistency, it may make sense for -addnode values to use the same check.

One way would be to call AddNode inside of CConnman::Init, but I'm not sure if it's okay to call LogWarning from there:

diff --git a/src/net.h b/src/net.h
index ea0c651d11..d69b9a4df3 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1140,12 +1140,13 @@ public:
         vWhitelistedRangeIncoming = connOptions.vWhitelistedRangeIncoming;
         vWhitelistedRangeOutgoing = connOptions.vWhitelistedRangeOutgoing;
         {
-            LOCK(m_added_nodes_mutex);
             // Attempt v2 connection if we support v2 - we'll reconnect with v1 if our
             // peer doesn't support it or immediately disconnects us for another reason.
             const bool use_v2transport(GetLocalServices() & NODE_P2P_V2);
             for (const std::string& added_node : connOptions.m_added_nodes) {
-                m_added_node_params.push_back({added_node, use_v2transport});
+                if (!AddNode({added_node, use_v2transport})) {
+                    LogWarning("Ignoring duplicate -addnode value: %s", added_node);
+                }
             }
         }

Otherwise, you can duplicate the logic in init.cpp.

To be clear, I'm okay even with the code as-is, if it gets too complicated to add the LookupNumeric checks.

@pablomartin4btc @danielabrozzoni Thanks for the suggestions. Great catches. I split the change into two commits and credited each of you as a co-author on the respective commit.