Merge bitcoin/bitcoin#22043: rpc, test: addpeeraddress test coverage, code simplify/constness

b36e0cd1b9d361ac6f9777c09328a13e9ee923be rpc: simplify addpeeraddress and improve code constness (Jon Atack)
6b1926cf1eac1ad1850599d2753dd22bc21fd327 test: addpeeraddress functional test coverage (Jon Atack)

Pull request description:

  - Add functional test coverage for rpc addpeeraddress
  - Simplify addpeeraddress and improve code constness

ACKs for top commit:
  klementtan:
    ACK [`b36e0cd`](https://github.com/bitcoin/bitcoin/pull/22043/commits/b36e0cd1b9d361ac6f9777c09328a13e9ee923be)
  MarcoFalke:
    review ACK b36e0cd1b9d361ac6f9777c09328a13e9ee923be 💭

Tree-SHA512: 01773fb70f23db5abf46806bb27804e48feff27272b2e6582bd5b886e9715088eb2d84755106bce2ad6f88e21582f7f071a30a89d5b17286d899c3dd8553b4fc

2 files changed, 37 insertions, 16 deletions

diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 4999eefc24..c4d6e3d546 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -931,26 +931,22 @@ static RPCHelpMan addpeeraddress()
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Address manager functionality missing or disabled");
     }
 
-    UniValue obj(UniValue::VOBJ);
-
-    std::string addr_string = request.params[0].get_str();
-    uint16_t port{static_cast<uint16_t>(request.params[1].get_int())};
+    const std::string& addr_string{request.params[0].get_str()};
+    const uint16_t port{static_cast<uint16_t>(request.params[1].get_int())};
 
+    UniValue obj(UniValue::VOBJ);
     CNetAddr net_addr;
-    if (!LookupHost(addr_string, net_addr, false)) {
-        obj.pushKV("success", false);
-        return obj;
-    }
-    CAddress address = CAddress({net_addr, port}, ServiceFlags(NODE_NETWORK|NODE_WITNESS));
-    address.nTime = GetAdjustedTime();
-    // The source address is set equal to the address. This is equivalent to the peer
-    // announcing itself.
-    if (!node.addrman->Add(address, address)) {
-        obj.pushKV("success", false);
-        return obj;
+    bool success{false};
+
+    if (LookupHost(addr_string, net_addr, false)) {
+        CAddress address{CAddress({net_addr, port}, ServiceFlags(NODE_NETWORK | NODE_WITNESS))};
+        address.nTime = GetAdjustedTime();
+        // The source address is set equal to the address. This is equivalent to the peer
+        // announcing itself.
+        if (node.addrman->Add(address, address)) success = true;
     }
 
-    obj.pushKV("success", true);
+    obj.pushKV("success", success);
     return obj;
 },
     };
diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index 2a58f8b3f7..9a2817a6ee 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -69,6 +69,7 @@ class NetTest(BitcoinTestFramework):
         self.test_getaddednodeinfo()
         self.test_service_flags()
         self.test_getnodeaddresses()
+        self.test_addpeeraddress()
 
     def test_connection_count(self):
         self.log.info("Test getconnectioncount")
@@ -236,6 +237,30 @@ class NetTest(BitcoinTestFramework):
         assert_raises_rpc_error(-8, "Address count out of range", self.nodes[0].getnodeaddresses, -1)
         assert_raises_rpc_error(-8, "Network not recognized: Foo", self.nodes[0].getnodeaddresses, 1, "Foo")
 
+    def test_addpeeraddress(self):
+        self.log.info("Test addpeeraddress")
+        node = self.nodes[1]
+
+        self.log.debug("Test that addpeerinfo is a hidden RPC")
+        # It is hidden from general help, but its detailed help may be called directly.
+        assert "addpeerinfo" not in node.help()
+        assert "addpeerinfo" in node.help("addpeerinfo")
+
+        self.log.debug("Test that adding an empty address fails")
+        assert_equal(node.addpeeraddress(address="", port=8333), {"success": False})
+        assert_equal(node.getnodeaddresses(count=0), [])
+
+        self.log.debug("Test that adding a valid address succeeds")
+        assert_equal(node.addpeeraddress(address="1.2.3.4", port=8333), {"success": True})
+        addrs = node.getnodeaddresses(count=0)
+        assert_equal(len(addrs), 1)
+        assert_equal(addrs[0]["address"], "1.2.3.4")
+        assert_equal(addrs[0]["port"], 8333)
+
+        self.log.debug("Test that adding the same address again when already present fails")
+        assert_equal(node.addpeeraddress(address="1.2.3.4", port=8333), {"success": False})
+        assert_equal(len(node.getnodeaddresses(count=0)), 1)
+
 
 if __name__ == '__main__':
     NetTest().main()