From 64c997313adf5312f3c4f18a457cc4ed9e5333ed Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 28 Dec 2020 22:40:33 +0100 Subject: [PATCH] Merge #20755: [rpc] Remove deprecated fields from getpeerinfo 454a4088a87eac5878070b26d13d5574da891877 [doc] Add release notes for removed getpeerinfo fields. (Amiti Uttarwar) b1a936d4ae7dd9030b0720ef05579a90ce2894f1 [rpc] Remove deprecated "whitelisted" field from getpeerinfo (Amiti Uttarwar) 094c3beaa47c909070607e94f2544ed1472ddb17 [rpc] Remove deprecated "banscore" field from getpeerinfo (Amiti Uttarwar) 537053336fbc1b633e7c99286c3e3492eaca1e9d [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 454a4088a87eac5878070b26d13d5574da891877 jnewbery: ACK 454a4088a87eac5878070b26d13d5574da891877. Tree-SHA512: ccc0e90c0763eeb8529cf0c46162dbaca3f7773981b3b52d9925166ea7421aed086795d56b320e16c9340f68862388785f52a9b78314865070917b33180d7cd6 --- src/net.cpp | 4 -- src/net.h | 4 -- src/net_processing.cpp | 1 - src/net_processing.h | 1 - src/rpc/net.cpp | 17 ------- test/functional/p2p_blocksonly.py | 2 +- test/functional/p2p_permissions.py | 50 ++++--------------- .../functional/rpc_getpeerinfo_deprecation.py | 39 --------------- test/functional/test_runner.py | 1 - 9 files changed, 12 insertions(+), 107 deletions(-) delete mode 100755 test/functional/rpc_getpeerinfo_deprecation.py diff --git a/src/net.cpp b/src/net.cpp index 865be0ce66a11a..3ff1d854832b85 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -654,7 +654,6 @@ void CNode::copyStats(CNodeStats &stats, const std::vector &m_asmap) X(cleanSubVer); } stats.fInbound = IsInboundConn(); - stats.m_manual_connection = IsManualConn(); X(nStartingHeight); { LOCK(cs_vSend); @@ -666,7 +665,6 @@ void CNode::copyStats(CNodeStats &stats, const std::vector &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, @@ -1246,8 +1244,6 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, CNode* pnode = new CNode(id, nodeServices, 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 index 0ca0f023364309..3a73ff3a79b447 100644 --- a/src/net.h +++ b/src/net.h @@ -894,14 +894,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; @@ -1092,8 +1090,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 index 517697e108b8af..2d262d5c6e9644 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1265,7 +1265,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 index 0f9bde911e3a4f..092bedd7d1bdce 100644 --- 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 vHeightInFlight; diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 98b84040851804..7cf4eb0cd7fdcf 100644 --- 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 index c724bc236aa21b..f0e716e84f3539 100755 --- 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 index 28e59f9d976f6a..d15ccd3ddda680 100755 --- 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) @@ -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 deleted file mode 100755 index 287c40ae3e19fb..00000000000000 --- a/test/functional/rpc_getpeerinfo_deprecation.py +++ /dev/null @@ -1,39 +0,0 @@ -#!/usr/bin/env python3 -# 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 fields.""" - -from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import connect_nodes - - -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"]) - connect_nodes(self.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__": - GetpeerinfoDeprecationTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 34c3d2f8089092..db2a5abc90e6e6 100755 --- 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', 'feature_blockfilterindex_prune.py'