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

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
dashpay · Jul 28, 2023 · d5c017b · d5c017b
1 parent 5fbf522
commit d5c017b
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 107 deletions.
diff --git a/src/net.cpp b/src/net.cpp
@@ -652,7 +652,6 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
         X(cleanSubVer);
     }
     X(fInbound);
-    X(m_manual_connection);
     X(nStartingHeight);
     {
         LOCK(cs_vSend);
@@ -664,7 +663,6 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
         X(mapRecvBytesPerMsgCmd);
         X(nRecvBytes);
     }
-    X(m_legacyWhitelisted);
     X(m_permissionFlags);
 
     // It is common for nodes with good ping times to suddenly become lagged,
@@ -1244,8 +1242,6 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket,
     CNode* pnode = new CNode(id, nodeServices, hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", true, 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
@@ -883,14 +883,12 @@ class CNodeStats
     int nVersion;
     std::string cleanSubVer;
     bool fInbound;
-    bool m_manual_connection;
     int nStartingHeight;
     uint64_t nSendBytes;
     mapMsgCmdSize mapSendBytesPerMsgCmd;
     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;
@@ -1081,8 +1079,7 @@ 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 fFeeler{false}; // If true this node is being used as a short lived feeler.
     bool fOneShot{false};
     bool m_manual_connection{false};

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1271,7 +1271,6 @@ bool PeerManagerImpl::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);
 
     return true;
 }

diff --git a/src/net_processing.h b/src/net_processing.h
@@ -26,7 +26,6 @@ static const bool DEFAULT_PEERBLOOMFILTERS = true;
 static const bool DEFAULT_PEERBLOCKFILTERS = false;
 
 struct CNodeStateStats {
-    int m_misbehavior_score = 0;
     int nSyncHeight = -1;
     int nCommonHeight = -1;
     std::vector<int> vHeightInFlight;

diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
@@ -118,20 +118,15 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
                     {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\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") + "."},
                     {RPCResult::Type::BOOL, "masternode", "Whether connection was due to masternode connection attempt"},
                     {RPCResult::Type::NUM, "startingheight", "The starting height (block) of the peer"},
-                    {RPCResult::Type::NUM, "banscore", "The ban score (DEPRECATED, returned only if config option -deprecatedrpc=banscore is passed)"},
                     {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::OBJ_DYN, "bytessent_per_msg", "",
                     {
                         {RPCResult::Type::NUM, "msg", "The total bytes sent aggregated by message type\n"
@@ -208,17 +203,9 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
         // their ver message.
         obj.pushKV("subver", stats.cleanSubVer);
         obj.pushKV("inbound", stats.fInbound);
-        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);
         obj.pushKV("startingheight", stats.nStartingHeight);
         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("synced_headers", statestats.nSyncHeight);
             obj.pushKV("synced_blocks", statestats.nCommonHeight);
             UniValue heights(UniValue::VARR);
@@ -227,10 +214,6 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
             }
             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
@@ -60,7 +60,7 @@ def run_test(self):
 
         self.log.info('Check that txs from forcerelay peers are not rejected and relayed to others')
         self.log.info("Restarting node 0 with forcerelay permission and blocksonly")
-        self.restart_node(0, ["-persistmempool=0", "-whitelist=127.0.0.1", "-whitelistforcerelay", "-blocksonly", '-deprecatedrpc=whitelisted'])
+        self.restart_node(0, ["-persistmempool=0", "-whitelist=127.0.0.1", "-whitelistforcerelay", "-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)
             ["[email protected]", "-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(
             ["[email protected]"],
             # 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,[email protected]", "-whitelistrelay"],
-            ["noban", "mempool", "download"],
-            False)
-
-        self.checkpermission(
-            # check without deprecatedrpc=whitelisted
-            ["-whitelist=noban,[email protected]", "-whitelistrelay"],
-            ["noban", "mempool", "download"],
-            None)
+            ["noban", "mempool", "download"])
 
         self.checkpermission(
             # legacy whitelistforcerelay should be ignored
             ["-whitelist=noban,[email protected]", "-whitelistforcerelay"],
-            ["noban", "mempool", "download"],
-            False)
+            ["noban", "mempool", "download"])
 
         self.checkpermission(
             # missing mempool permission to be considered legacy whitelisted
             ["[email protected]"],
-            ["noban", "download"],
-            False)
+            ["noban", "download"])
 
         self.checkpermission(
             # all permission added
             ["[email protected]"],
-            ["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(["[email protected]"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX)
@@ -164,19 +142,13 @@ def in_mempool():
             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
@@ -272,7 +272,6 @@
     'rpc_getdescriptorinfo.py',
     'rpc_getaddressinfo_labels_purpose_deprecation.py',
     'rpc_getaddressinfo_label_deprecation.py',
-    'rpc_getpeerinfo_deprecation.py',
     'rpc_help.py',
     'feature_help.py',
     # Don't append tests at the end to avoid merge conflicts