Merge #20755: [rpc] Remove deprecated fields from getpeerinfo

MarcoFalke · MarcoFalke · commit e3dd0a56cfa7 · 2020-12-28T22:40:39.000+01:00
454a408 [doc] Add release notes for removed getpeerinfo fields. (Amiti Uttarwar) b1a936d [rpc] Remove deprecated "whitelisted" field from getpeerinfo (Amiti Uttarwar) 094c3be [rpc] Remove deprecated "banscore" field from getpeerinfo (Amiti Uttarwar) 5370533 [rpc] Remove deprecated "addnode" field from getpeerinfo (Amiti Uttarwar) Pull request description: This PR removes support for 3 fields on the `getpeerinfo` RPC that were deprecated in v0.21- `addnode`, `banscore` & `whitelisted`. ACKs for top commit: sipa: utACK 454a408 jnewbery: ACK 454a408. Tree-SHA512: ccc0e90c0763eeb8529cf0c46162dbaca3f7773981b3b52d9925166ea7421aed086795d56b320e16c9340f68862388785f52a9b78314865070917b33180d7cd6
diff --git a/doc/release-notes.md b/doc/release-notes.md
@@ -63,6 +63,11 @@ P2P and network changes
 
 Updated RPCs
 ------------
+- `getpeerinfo` no longer returns the following fields: `addnode`, `banscore`,
+  and `whitelisted`, which were previously deprecated in 0.21. Instead of
+  `addnode`, the `connection_type` field returns manual. Instead of
+  `whitelisted`, the `permissions` field indicates if the peer has special
+  privileges. The `banscore` field has simply been removed. (#20755)
 
 Changes to Wallet or GUI related RPCs can be found in the GUI or Wallet section below.
 
diff --git a/src/net.cpp b/src/net.cpp
@@ -587,7 +587,6 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
         X(cleanSubVer);
     }
     stats.fInbound = IsInboundConn();
-    stats.m_manual_connection = IsManualConn();
     X(m_bip152_highbandwidth_to);
     X(m_bip152_highbandwidth_from);
     {
@@ -600,7 +599,6 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
         X(mapRecvBytesPerMsgCmd);
         X(nRecvBytes);
     }
-    X(m_legacyWhitelisted);
     X(m_permissionFlags);
     if (m_tx_relay != nullptr) {
         LOCK(m_tx_relay->cs_feeFilter);
@@ -1123,8 +1121,6 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     CNode* pnode = new CNode(id, nodeServices, GetBestHeight(), hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND, inbound_onion);
     pnode->AddRef();
     pnode->m_permissionFlags = permissionFlags;
-    // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
-    pnode->m_legacyWhitelisted = legacyWhitelisted;
     pnode->m_prefer_evict = discouraged;
     m_msgproc->InitializeNode(pnode);
 
diff --git a/src/net.h b/src/net.h
@@ -702,7 +702,6 @@ class CNodeStats
     int nVersion;
     std::string cleanSubVer;
     bool fInbound;
-    bool m_manual_connection;
     bool m_bip152_highbandwidth_to;
     bool m_bip152_highbandwidth_from;
     int m_starting_height;
@@ -711,7 +710,6 @@ class CNodeStats
     uint64_t nRecvBytes;
     mapMsgCmdSize mapRecvBytesPerMsgCmd;
     NetPermissionFlags m_permissionFlags;
-    bool m_legacyWhitelisted;
     int64_t m_ping_usec;
     int64_t m_ping_wait_usec;
     int64_t m_min_ping_usec;
@@ -892,8 +890,6 @@ class CNode
     bool HasPermission(NetPermissionFlags permission) const {
         return NetPermissions::HasFlag(m_permissionFlags, permission);
     }
-    // This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level
-    bool m_legacyWhitelisted{false};
     bool fClient{false}; // set by version message
     bool m_limited_node{false}; //after BIP159, set by version message
     /**
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -874,7 +874,6 @@ bool PeerManager::GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats) {
 
     PeerRef peer = GetPeerRef(nodeid);
     if (peer == nullptr) return false;
-    stats.m_misbehavior_score = WITH_LOCK(peer->m_misbehavior_mutex, return peer->m_misbehavior_score);
     stats.m_starting_height = peer->m_starting_height;
 
     return true;
diff --git a/src/net_processing.h b/src/net_processing.h
@@ -33,7 +33,6 @@ static const bool DEFAULT_PEERBLOCKFILTERS = false;
 static const int DISCOURAGEMENT_THRESHOLD{100};
 
 struct CNodeStateStats {
-    int m_misbehavior_score = 0;
     int nSyncHeight = -1;
     int nCommonHeight = -1;
     int m_starting_height = -1;
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
@@ -128,21 +128,16 @@ static RPCHelpMan getpeerinfo()
                             {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"},
                             {RPCResult::Type::BOOL, "bip152_hb_to", "Whether we selected peer as (compact blocks) high-bandwidth peer"},
                             {RPCResult::Type::BOOL, "bip152_hb_from", "Whether peer selected us as (compact blocks) high-bandwidth peer"},
-                            {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::STR, "connection_type", "Type of connection: \n" + Join(CONNECTION_TYPE_DOC, ",\n") + ".\n"
                                                                       "Please note this output is unlikely to be stable in upcoming releases as we iterate to\n"
                                                                       "best capture connection behaviors."},
-                            {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"},
                             {RPCResult::Type::NUM, "synced_headers", "The last header we have in common with this peer"},
                             {RPCResult::Type::NUM, "synced_blocks", "The last block we have in common with this peer"},
                             {RPCResult::Type::ARR, "inflight", "",
                             {
                                 {RPCResult::Type::NUM, "n", "The heights of blocks we're currently asking from this peer"},
                             }},
-                            {RPCResult::Type::BOOL, "whitelisted", /* optional */ true, "Whether the peer is whitelisted with default permissions\n"
-                                                                                        "(DEPRECATED, returned only if config option -deprecatedrpc=whitelisted is passed)"},
                             {RPCResult::Type::ARR, "permissions", "Any special permissions that have been granted to this peer",
                             {
                                 {RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"},
@@ -224,15 +219,7 @@ static RPCHelpMan getpeerinfo()
         obj.pushKV("inbound", stats.fInbound);
         obj.pushKV("bip152_hb_to", stats.m_bip152_highbandwidth_to);
         obj.pushKV("bip152_hb_from", stats.m_bip152_highbandwidth_from);
-        if (IsDeprecatedRPCEnabled("getpeerinfo_addnode")) {
-            // addnode is deprecated in v0.21 for removal in v0.22
-            obj.pushKV("addnode", stats.m_manual_connection);
-        }
         if (fStateStats) {
-            if (IsDeprecatedRPCEnabled("banscore")) {
-                // banscore is deprecated in v0.21 for removal in v0.22
-                obj.pushKV("banscore", statestats.m_misbehavior_score);
-            }
             obj.pushKV("startingheight", statestats.m_starting_height);
             obj.pushKV("synced_headers", statestats.nSyncHeight);
             obj.pushKV("synced_blocks", statestats.nCommonHeight);
@@ -242,10 +229,6 @@ static RPCHelpMan getpeerinfo()
             }
             obj.pushKV("inflight", heights);
         }
-        if (IsDeprecatedRPCEnabled("whitelisted")) {
-            // whitelisted is deprecated in v0.21 for removal in v0.22
-            obj.pushKV("whitelisted", stats.m_legacyWhitelisted);
-        }
         UniValue permissions(UniValue::VARR);
         for (const auto& permission : NetPermissions::ToStrings(stats.m_permissionFlags)) {
             permissions.push_back(permission);
diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py
@@ -59,7 +59,7 @@ def run_test(self):
 
         self.log.info('Check that txs from peers with relay-permission are not rejected and relayed to others')
         self.log.info("Restarting node 0 with relay permission and blocksonly")
-        self.restart_node(0, ["-persistmempool=0", "-whitelist=relay@127.0.0.1", "-blocksonly", '-deprecatedrpc=whitelisted'])
+        self.restart_node(0, ["-persistmempool=0", "-whitelist=relay@127.0.0.1", "-blocksonly"])
         assert_equal(self.nodes[0].getrawmempool(), [])
         first_peer = self.nodes[0].add_p2p_connection(P2PInterface())
         second_peer = self.nodes[0].add_p2p_connection(P2PInterface())
diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py
@@ -38,35 +38,24 @@ def run_test(self):
             # default permissions (no specific permissions)
             ["-whitelist=127.0.0.1"],
             # Make sure the default values in the command line documentation match the ones here
-            ["relay", "noban", "mempool", "download"],
-            True)
-
-        self.checkpermission(
-            # check without deprecatedrpc=whitelisted
-            ["-whitelist=127.0.0.1"],
-            # Make sure the default values in the command line documentation match the ones here
-            ["relay", "noban", "mempool", "download"],
-            None)
+            ["relay", "noban", "mempool", "download"])
 
         self.checkpermission(
             # no permission (even with forcerelay)
             ["-whitelist=@127.0.0.1", "-whitelistforcerelay=1"],
-            [],
-            False)
+            [])
 
         self.checkpermission(
             # relay permission removed (no specific permissions)
             ["-whitelist=127.0.0.1", "-whitelistrelay=0"],
-            ["noban", "mempool", "download"],
-            True)
+            ["noban", "mempool", "download"])
 
         self.checkpermission(
             # forcerelay and relay permission added
             # Legacy parameter interaction which set whitelistrelay to true
             # if whitelistforcerelay is true
             ["-whitelist=127.0.0.1", "-whitelistforcerelay"],
-            ["forcerelay", "relay", "noban", "mempool", "download"],
-            True)
+            ["forcerelay", "relay", "noban", "mempool", "download"])
 
         # Let's make sure permissions are merged correctly
         # For this, we need to use whitebind instead of bind
@@ -76,39 +65,28 @@ def run_test(self):
         self.checkpermission(
             ["-whitelist=noban@127.0.0.1"],
             # Check parameter interaction forcerelay should activate relay
-            ["noban", "bloomfilter", "forcerelay", "relay", "download"],
-            False)
+            ["noban", "bloomfilter", "forcerelay", "relay", "download"])
         self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1")
 
         self.checkpermission(
             # legacy whitelistrelay should be ignored
             ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"],
-            ["noban", "mempool", "download"],
-            False)
-
-        self.checkpermission(
-            # check without deprecatedrpc=whitelisted
-            ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"],
-            ["noban", "mempool", "download"],
-            None)
+            ["noban", "mempool", "download"])
 
         self.checkpermission(
             # legacy whitelistforcerelay should be ignored
             ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"],
-            ["noban", "mempool", "download"],
-            False)
+            ["noban", "mempool", "download"])
 
         self.checkpermission(
             # missing mempool permission to be considered legacy whitelisted
             ["-whitelist=noban@127.0.0.1"],
-            ["noban", "download"],
-            False)
+            ["noban", "download"])
 
         self.checkpermission(
             # all permission added
             ["-whitelist=all@127.0.0.1"],
-            ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"],
-            False)
+            ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"])
 
         self.stop_node(1)
         self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX)
@@ -169,19 +147,13 @@ def check_tx_relay(self):
             reject_reason='Not relaying non-mempool transaction {} from forcerelay peer=0'.format(txid)
         )
 
-    def checkpermission(self, args, expectedPermissions, whitelisted):
-        if whitelisted is not None:
-            args = [*args, '-deprecatedrpc=whitelisted']
+    def checkpermission(self, args, expectedPermissions):
         self.restart_node(1, args)
         self.connect_nodes(0, 1)
         peerinfo = self.nodes[1].getpeerinfo()[0]
-        if whitelisted is None:
-            assert 'whitelisted' not in peerinfo
-        else:
-            assert_equal(peerinfo['whitelisted'], whitelisted)
         assert_equal(len(expectedPermissions), len(peerinfo['permissions']))
         for p in expectedPermissions:
-            if not p in peerinfo['permissions']:
+            if p not in peerinfo['permissions']:
                 raise AssertionError("Expected permissions %r is not granted." % p)
 
     def replaceinconfig(self, nodeid, old, new):
diff --git a/test/functional/rpc_getpeerinfo_deprecation.py b/test/functional/rpc_getpeerinfo_deprecation.py
diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py
@@ -279,7 +279,6 @@
     'feature_config_args.py',
     'feature_settings.py',
     'rpc_getdescriptorinfo.py',
-    'rpc_getpeerinfo_deprecation.py',
     'rpc_help.py',
     'feature_help.py',
     'feature_shutdown.py',
