Merge bitcoin#19725: [RPC] Add connection type to getpeerinfo, improv…

…e logs

a512925 [doc] Release notes (Amiti Uttarwar)
50f94b3 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar)
df091b9 [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar)
395acfa [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar)
49c10a9 [log] Add connection type to log statement (Amiti Uttarwar)

Pull request description:

  After bitcoin#19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field.

  This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string.

  Suggested by sdaftuar- bitcoin#19316 (comment) & bitcoin#19316 (comment)

ACKs for top commit:
  jnewbery:
    Code review ACK a512925.
  sipa:
    utACK a512925
  guggero:
    Tested and code review ACK a512925.
  MarcoFalke:
    cr ACK a512925 🌇
  promag:
    Code review ACK a512925.

Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e

doc/release-notes-19725.md

@@ -0,0 +1,10 @@
+
+Updated RPCs
+------------
+
+- The `getpeerinfo` RPC no longer returns the `addnode` field by default. This
+  field will be fully removed in the next major release.  It can be accessed
+  with the configuration option `-deprecatedrpc=getpeerinfo_addnode`. However,
+  it is recommended to instead use the `connection_type` field (it will return
+  `manual` when addnode is true). (#6033)
+

src/rpc/net.cpp

@@ -145,7 +145,8 @@ static RPCHelpMan getpeerinfo()
                     {RPCResult::Type::NUM, "version", "The peer version, such as 70001"},
                     {RPCResult::Type::STR, "subver", "The string version"},
                     {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"},
-                    {RPCResult::Type::BOOL, "addnode", "Whether connection was due to addnode/-connect or if it was an automatic/inbound connection"},
+                    {RPCResult::Type::BOOL, "addnode", "Whether connection was due to addnode/-connect or if it was an automatic/inbound connection\n"
+                                                       "(DEPRECATED, returned only if the config option -deprecatedrpc=getpeerinfo_addnode is passed)"},
                     {RPCResult::Type::BOOL, "masternode", "Whether connection was due to masternode connection attempt"},
                     {RPCResult::Type::NUM, "banscore", "The ban score (DEPRECATED, returned only if config option -deprecatedrpc=banscore is passed)"},
                     {RPCResult::Type::NUM, "startingheight", "The starting height (block) of the peer"},
@@ -242,7 +243,10 @@ static RPCHelpMan getpeerinfo()
         // their ver message.
         obj.pushKV("subver", stats.cleanSubVer);
         obj.pushKV("inbound", stats.fInbound);
-        obj.pushKV("addnode", stats.m_manual_connection);
+        if (IsDeprecatedRPCEnabled("getpeerinfo_addnode")) {
+            // addnode is deprecated in v0.21 for removal in v0.22
+            obj.pushKV("addnode", stats.m_manual_connection);
+        }
         obj.pushKV("masternode", stats.m_masternode_connection);
         if (fStateStats) {
             if (IsDeprecatedRPCEnabled("banscore")) {

test/functional/rpc_getpeerinfo_banscore_deprecation.py → test/functional/rpc_getpeerinfo_deprecation.py

@@ -2,23 +2,37 @@
 # Copyright (c) 2020 The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Test deprecation of getpeerinfo RPC banscore field."""
+"""Test deprecation of getpeerinfo RPC fields."""
 
 from test_framework.test_framework import BitcoinTestFramework
 
 
-class GetpeerinfoBanscoreDeprecationTest(BitcoinTestFramework):
+class GetpeerinfoDeprecationTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
         self.extra_args = [[], ["-deprecatedrpc=banscore"]]
 
     def run_test(self):
+        self.test_banscore_deprecation()
+        self.test_addnode_deprecation()
+
+    def test_banscore_deprecation(self):
         self.log.info("Test getpeerinfo by default no longer returns a banscore field")
         assert "banscore" not in self.nodes[0].getpeerinfo()[0].keys()
 
         self.log.info("Test getpeerinfo returns banscore with -deprecatedrpc=banscore")
         assert "banscore" in self.nodes[1].getpeerinfo()[0].keys()
 
+    def test_addnode_deprecation(self):
+        self.restart_node(1, ["-deprecatedrpc=getpeerinfo_addnode"])
+        self.connect_nodes(0, 1)
+
+        self.log.info("Test getpeerinfo by default no longer returns an addnode field")
+        assert "addnode" not in self.nodes[0].getpeerinfo()[0].keys()
+
+        self.log.info("Test getpeerinfo returns addnode with -deprecatedrpc=addnode")
+        assert "addnode" in self.nodes[1].getpeerinfo()[0].keys()
+
 
 if __name__ == "__main__":
-    GetpeerinfoBanscoreDeprecationTest().main()
+    GetpeerinfoDeprecationTest().main()

test/functional/test_runner.py

@@ -332,7 +332,7 @@
     'feature_config_args.py',
     'feature_settings.py',
     'rpc_getdescriptorinfo.py',
-    'rpc_getpeerinfo_banscore_deprecation.py',
+    'rpc_getpeerinfo_deprecation.py',
     'rpc_help.py',
     'feature_help.py',
     'feature_blockfilterindex_prune.py'