Merge f1b142856a into c5e44a0435

Merge bitcoin/bitcoin#32369 : test: Use the correct node for doubled keypath test
32d55e28af test: Use the correct node for doubled keypath test (Ava Chow) Pull request description: #29124 had a silent merge conflict with #32350 which resulted in it using the wrong node. Fix the test to use the correct v22 node. ACKs for top commit: maflcko: lgtm ACK 32d55e28af rkrux: ACK 32d55e28af BrandonOdiwuor: Code Review ACK 32d55e28af Tree-SHA512: 1e0231985beb382b16e1d608c874750423d0502388db0c8ad450b22d17f9d96f5e16a6b44948ebda5efc750f62b60d0de8dd20131f449427426a36caf374af92
2025-04-29 06:49:38 -04:00 · 2025-04-29 11:50:53 +02:00 · 2025-04-29 09:59:42 +01:00 · 2025-04-28 14:44:17 -07:00 · 2025-04-25 15:20:34 +02:00 · 2025-04-25 15:12:03 +02:00
4 changed files with 14 additions and 3 deletions
--- a/src/net.cpp
+++ b/src/net.cpp
@ -367,7 +367,7 @@ CNode* CConnman::FindNode(const CService& addr)

 bool CConnman::AlreadyConnectedToAddress(const CAddress& addr)
 {
-    return FindNode(static_cast<CNetAddr>(addr)) || FindNode(addr.ToStringAddrPort());
+    return FindNode(static_cast<CNetAddr>(addr));
 }

 bool CConnman::CheckIncomingNonce(uint64_t nonce)
--- a/src/test/fuzz/netaddress.cpp
+++ b/src/test/fuzz/netaddress.cpp
@ -101,10 +101,14 @@ FUZZ_TARGET(netaddress)
    (void)net_addr.GetReachabilityFrom(other_net_addr);
    (void)sub_net.Match(other_net_addr);

-    const CService other_service{net_addr, fuzzed_data_provider.ConsumeIntegral<uint16_t>()};
+    const CService other_service{fuzzed_data_provider.ConsumeBool() ? net_addr : other_net_addr, fuzzed_data_provider.ConsumeIntegral<uint16_t>()};
    assert((service == other_service) != (service != other_service));
    (void)(service < other_service);

+    if (service.ToStringAddrPort() == other_service.ToStringAddrPort()) {
+        assert(static_cast<CNetAddr>(service) == static_cast<CNetAddr>(other_service));
+    }
+
    const CSubNet sub_net_copy_1{net_addr, other_net_addr};
    const CSubNet sub_net_copy_2{net_addr};

--- a/src/test/net_peer_connection_tests.cpp
+++ b/src/test/net_peer_connection_tests.cpp
@ -155,6 +155,13 @@ BOOST_FIXTURE_TEST_CASE(test_addnode_getaddednodeinfo_and_connection_detection,
        BOOST_CHECK(connman->AlreadyConnectedPublic(node->addr));
    }

+    BOOST_TEST_MESSAGE("\nCheck that peers with the same addresses as connected peers but different ports are detected as connected.");
+    for (auto node : connman->TestNodes()) {
+        uint16_t changed_port = node->addr.GetPort() + 1;
+        CService address_with_changed_port{node->addr, changed_port};
+        BOOST_CHECK(connman->AlreadyConnectedPublic(CAddress{address_with_changed_port, NODE_NONE}));
+    }
+
    // Clean up
    for (auto node : connman->TestNodes()) {
        peerman->FinalizeNode(*node);
--- a/test/functional/wallet_backwards_compatibility.py
+++ b/test/functional/wallet_backwards_compatibility.py
@ -87,7 +87,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
        # 0.21.x and 22.x would both produce bad derivation paths when topping up an inactive hd chain
        # Make sure that this is being automatically cleaned up by migration
        node_master = self.nodes[1]
-        node_v22 = self.nodes[self.num_nodes - 5]
+        node_v22 = self.nodes[self.num_nodes - 3]
        wallet_name = "bad_deriv_path"
        node_v22.createwallet(wallet_name=wallet_name, descriptors=False)
        bad_deriv_wallet = node_v22.get_wallet_rpc(wallet_name)
Author	SHA1	Message	Date
Vasil Dimov	810ec2ddd4	Merge `f1b142856a` into `c5e44a0435`	2025-04-29 11:50:53 +02:00
merge-script	c5e44a0435	Merge bitcoin/bitcoin#32369 : test: Use the correct node for doubled keypath test Some checks are pending CI / macOS 14 native, arm64, fuzz (push) Waiting to run Details CI / Windows native, VS 2022 (push) Waiting to run Details CI / Windows native, fuzz, VS 2022 (push) Waiting to run Details CI / Linux->Windows cross, no tests (push) Waiting to run Details CI / Windows, test cross-built (push) Blocked by required conditions Details CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run Details CI / test each commit (push) Waiting to run Details CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run Details `32d55e28af` test: Use the correct node for doubled keypath test (Ava Chow) Pull request description: #29124 had a silent merge conflict with #32350 which resulted in it using the wrong node. Fix the test to use the correct v22 node. ACKs for top commit: maflcko: lgtm ACK `32d55e28af` rkrux: ACK `32d55e28af` BrandonOdiwuor: Code Review ACK `32d55e28af` Tree-SHA512: 1e0231985beb382b16e1d608c874750423d0502388db0c8ad450b22d17f9d96f5e16a6b44948ebda5efc750f62b60d0de8dd20131f449427426a36caf374af92	2025-04-29 09:59:42 +01:00
Ava Chow	32d55e28af	test: Use the correct node for doubled keypath test	2025-04-28 14:44:17 -07:00
David Gumberg	f1b142856a	test: Same addr, diff port is already connected	2025-04-25 15:20:34 +02:00
Vasil Dimov	94e85a82a7	net: remove unnecessary check from AlreadyConnectedToAddress() `CConnman::AlreadyConnectedToAddress()` searches the existent nodes by address or by address-and-port: ```cpp FindNode(static_cast<CNetAddr>(addr)) \|\| FindNode(addr.ToStringAddrPort()) ``` but: * if there is a match by just the address, then the address-and-port search will not be evaluated and the whole condition will be `true` * if the there is no node with the same address, then the second search by address-and-port will not find a match either. The search by address-and-port is comparing against `CNode::m_addr_name` which could be a hostname, e.g. `"node.foobar.com:8333"`, but `addr.ToStringAddrPort()` is always going to be numeric.	2025-04-25 15:12:03 +02:00