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

BACKPORT NOTE:
the field `banscore` is used by functional test p2p_quorum_data.py and can't be removed now

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
This commit is contained in:
MarcoFalke 2020-12-28 22:40:33 +01:00 committed by Konstantin Akimov
parent cba01aa8f9
commit 42d4f9a9b9
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
8 changed files with 19 additions and 71 deletions

View File

@ -0,0 +1,7 @@
Updated RPCs
------------
- `getpeerinfo` no longer returns the following fields: `addnode`,
and `whitelisted`, which were previously deprecated in v21. Instead of
`addnode`, the `connection_type` field returns manual. Instead of
`whitelisted`, the `permissions` field indicates if the peer has special
privileges. (#20755)

View File

@ -736,7 +736,6 @@ void CNode::CopyStats(CNodeStats& stats)
X(cleanSubVer); X(cleanSubVer);
} }
stats.fInbound = IsInboundConn(); stats.fInbound = IsInboundConn();
stats.m_manual_connection = IsManualConn();
X(m_bip152_highbandwidth_to); X(m_bip152_highbandwidth_to);
X(m_bip152_highbandwidth_from); X(m_bip152_highbandwidth_from);
{ {
@ -752,7 +751,6 @@ void CNode::CopyStats(CNodeStats& stats)
stats.m_transport_type = info.transport_type; stats.m_transport_type = info.transport_type;
if (info.session_id) stats.m_session_id = HexStr(*info.session_id); if (info.session_id) stats.m_session_id = HexStr(*info.session_id);
} }
X(m_legacyWhitelisted);
X(m_permission_flags); X(m_permission_flags);
X(m_last_ping_time); X(m_last_ping_time);
@ -2010,8 +2008,6 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
.use_v2transport = use_v2transport, .use_v2transport = use_v2transport,
}); });
pnode->AddRef(); pnode->AddRef();
// If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
pnode->m_legacyWhitelisted = legacyWhitelisted;
m_msgproc->InitializeNode(*pnode, nodeServices); m_msgproc->InitializeNode(*pnode, nodeServices);
{ {

View File

@ -241,7 +241,6 @@ public:
int nVersion; int nVersion;
std::string cleanSubVer; std::string cleanSubVer;
bool fInbound; bool fInbound;
bool m_manual_connection;
bool m_bip152_highbandwidth_to; bool m_bip152_highbandwidth_to;
bool m_bip152_highbandwidth_from; bool m_bip152_highbandwidth_from;
int m_starting_height; int m_starting_height;
@ -250,7 +249,6 @@ public:
uint64_t nRecvBytes; uint64_t nRecvBytes;
mapMsgTypeSize mapRecvBytesPerMsgType; mapMsgTypeSize mapRecvBytesPerMsgType;
NetPermissionFlags m_permission_flags; NetPermissionFlags m_permission_flags;
bool m_legacyWhitelisted;
std::chrono::microseconds m_last_ping_time; std::chrono::microseconds m_last_ping_time;
std::chrono::microseconds m_min_ping_time; std::chrono::microseconds m_min_ping_time;
// Our address, as reported by the peer // Our address, as reported by the peer
@ -789,8 +787,6 @@ public:
bool HasPermission(NetPermissionFlags permission) const { bool HasPermission(NetPermissionFlags permission) const {
return NetPermissions::HasFlag(m_permission_flags, permission); return NetPermissions::HasFlag(m_permission_flags, permission);
} }
// This boolean is unusued in actual processing, only present for backward compatibility at RPC/QT level
bool m_legacyWhitelisted{false};
/** fSuccessfullyConnected is set to true on receiving VERACK from the peer. */ /** fSuccessfullyConnected is set to true on receiving VERACK from the peer. */
std::atomic_bool fSuccessfullyConnected{false}; std::atomic_bool fSuccessfullyConnected{false};
// Setting fDisconnect to true will cause the node to be disconnected the // Setting fDisconnect to true will cause the node to be disconnected the

View File

@ -137,8 +137,6 @@ static RPCHelpMan getpeerinfo()
{RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, {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_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, "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::BOOL, "masternode", "Whether connection was due to masternode connection attempt"}, {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, "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, "startingheight", "The starting height (block) of the peer"},
@ -151,8 +149,6 @@ static RPCHelpMan getpeerinfo()
{RPCResult::Type::BOOL, "addr_relay_enabled", "Whether we participate in address relay with this peer"}, {RPCResult::Type::BOOL, "addr_relay_enabled", "Whether we participate in address relay with this peer"},
{RPCResult::Type::NUM, "addr_processed", "The total number of addresses processed, excluding those dropped due to rate limiting"}, {RPCResult::Type::NUM, "addr_processed", "The total number of addresses processed, excluding those dropped due to rate limiting"},
{RPCResult::Type::NUM, "addr_rate_limited", "The total number of addresses dropped due to rate limiting"}, {RPCResult::Type::NUM, "addr_rate_limited", "The total number of addresses dropped due to rate limiting"},
{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::ARR, "permissions", "Any special permissions that have been granted to this peer",
{ {
{RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"}, {RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"},
@ -241,10 +237,6 @@ static RPCHelpMan getpeerinfo()
obj.pushKV("inbound", stats.fInbound); obj.pushKV("inbound", stats.fInbound);
obj.pushKV("bip152_hb_to", stats.m_bip152_highbandwidth_to); obj.pushKV("bip152_hb_to", stats.m_bip152_highbandwidth_to);
obj.pushKV("bip152_hb_from", stats.m_bip152_highbandwidth_from); obj.pushKV("bip152_hb_from", stats.m_bip152_highbandwidth_from);
if (IsDeprecatedRPCEnabled("getpeerinfo_addnode")) {
// addnode is deprecated in v21 for removal in v22
obj.pushKV("addnode", stats.m_manual_connection);
}
obj.pushKV("masternode", stats.m_masternode_connection); obj.pushKV("masternode", stats.m_masternode_connection);
if (fStateStats) { if (fStateStats) {
if (IsDeprecatedRPCEnabled("banscore")) { if (IsDeprecatedRPCEnabled("banscore")) {
@ -264,10 +256,6 @@ static RPCHelpMan getpeerinfo()
obj.pushKV("addr_processed", statestats.m_addr_processed); obj.pushKV("addr_processed", statestats.m_addr_processed);
obj.pushKV("addr_rate_limited", statestats.m_addr_rate_limited); obj.pushKV("addr_rate_limited", statestats.m_addr_rate_limited);
} }
if (IsDeprecatedRPCEnabled("whitelisted")) {
// whitelisted is deprecated in v0.21 for removal in v0.22
obj.pushKV("whitelisted", stats.m_legacyWhitelisted);
}
UniValue permissions(UniValue::VARR); UniValue permissions(UniValue::VARR);
for (const auto& permission : NetPermissions::ToStrings(stats.m_permission_flags)) { for (const auto& permission : NetPermissions::ToStrings(stats.m_permission_flags)) {
permissions.push_back(permission); permissions.push_back(permission);

View File

@ -51,7 +51,7 @@ class P2PBlocksOnly(BitcoinTestFramework):
assert_equal(self.nodes[0].getmempoolinfo()['size'], 1) assert_equal(self.nodes[0].getmempoolinfo()['size'], 1)
self.log.info("Restarting node 0 with relay permission and blocksonly") 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(), []) assert_equal(self.nodes[0].getrawmempool(), [])
first_peer = self.nodes[0].add_p2p_connection(P2PInterface()) first_peer = self.nodes[0].add_p2p_connection(P2PInterface())
second_peer = self.nodes[0].add_p2p_connection(P2PInterface()) second_peer = self.nodes[0].add_p2p_connection(P2PInterface())

View File

@ -36,35 +36,24 @@ class P2PPermissionsTests(BitcoinTestFramework):
# default permissions (no specific permissions) # default permissions (no specific permissions)
["-whitelist=127.0.0.1"], ["-whitelist=127.0.0.1"],
# Make sure the default values in the command line documentation match the ones here # Make sure the default values in the command line documentation match the ones here
["relay", "noban", "mempool", "download"], ["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)
self.checkpermission( self.checkpermission(
# no permission (even with forcerelay) # no permission (even with forcerelay)
["-whitelist=@127.0.0.1", "-whitelistforcerelay=1"], ["-whitelist=@127.0.0.1", "-whitelistforcerelay=1"],
[], [])
False)
self.checkpermission( self.checkpermission(
# relay permission removed (no specific permissions) # relay permission removed (no specific permissions)
["-whitelist=127.0.0.1", "-whitelistrelay=0"], ["-whitelist=127.0.0.1", "-whitelistrelay=0"],
["noban", "mempool", "download"], ["noban", "mempool", "download"])
True)
self.checkpermission( self.checkpermission(
# forcerelay and relay permission added # forcerelay and relay permission added
# Legacy parameter interaction which set whitelistrelay to true # Legacy parameter interaction which set whitelistrelay to true
# if whitelistforcerelay is true # if whitelistforcerelay is true
["-whitelist=127.0.0.1", "-whitelistforcerelay"], ["-whitelist=127.0.0.1", "-whitelistforcerelay"],
["forcerelay", "relay", "noban", "mempool", "download"], ["forcerelay", "relay", "noban", "mempool", "download"])
True)
# Let's make sure permissions are merged correctly # Let's make sure permissions are merged correctly
# For this, we need to use whitebind instead of bind # For this, we need to use whitebind instead of bind
@ -74,39 +63,28 @@ class P2PPermissionsTests(BitcoinTestFramework):
self.checkpermission( self.checkpermission(
["-whitelist=noban@127.0.0.1"], ["-whitelist=noban@127.0.0.1"],
# Check parameter interaction forcerelay should activate relay # Check parameter interaction forcerelay should activate relay
["noban", "bloomfilter", "forcerelay", "relay", "download"], ["noban", "bloomfilter", "forcerelay", "relay", "download"])
False)
self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1") self.replaceinconfig(1, "whitebind=bloomfilter,forcerelay@" + ip_port, "bind=127.0.0.1")
self.checkpermission( self.checkpermission(
# legacy whitelistrelay should be ignored # legacy whitelistrelay should be ignored
["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"], ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"],
["noban", "mempool", "download"], ["noban", "mempool", "download"])
False)
self.checkpermission(
# check without deprecatedrpc=whitelisted
["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"],
["noban", "mempool", "download"],
None)
self.checkpermission( self.checkpermission(
# legacy whitelistforcerelay should be ignored # legacy whitelistforcerelay should be ignored
["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"],
["noban", "mempool", "download"], ["noban", "mempool", "download"])
False)
self.checkpermission( self.checkpermission(
# missing mempool permission to be considered legacy whitelisted # missing mempool permission to be considered legacy whitelisted
["-whitelist=noban@127.0.0.1"], ["-whitelist=noban@127.0.0.1"],
["noban", "download"], ["noban", "download"])
False)
self.checkpermission( self.checkpermission(
# all permission added # all permission added
["-whitelist=all@127.0.0.1"], ["-whitelist=all@127.0.0.1"],
["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"], ["forcerelay", "noban", "mempool", "bloomfilter", "relay", "download", "addr"])
False)
self.stop_node(1) 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) self.nodes[1].assert_start_raises_init_error(["-whitelist=oopsie@127.0.0.1"], "Invalid P2P permission", match=ErrorMatch.PARTIAL_REGEX)
@ -170,19 +148,13 @@ class P2PPermissionsTests(BitcoinTestFramework):
reject_reason='Not relaying non-mempool transaction {} from forcerelay peer=0'.format(txid) reject_reason='Not relaying non-mempool transaction {} from forcerelay peer=0'.format(txid)
) )
def checkpermission(self, args, expectedPermissions, whitelisted): def checkpermission(self, args, expectedPermissions):
if whitelisted is not None:
args = [*args, '-deprecatedrpc=whitelisted']
self.restart_node(1, args) self.restart_node(1, args)
self.connect_nodes(0, 1) self.connect_nodes(0, 1)
peerinfo = self.nodes[1].getpeerinfo()[0] 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'])) assert_equal(len(expectedPermissions), len(peerinfo['permissions']))
for p in expectedPermissions: 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) raise AssertionError("Expected permissions %r is not granted." % p)
def replaceinconfig(self, nodeid, old, new): def replaceinconfig(self, nodeid, old, new):

0
test/functional/rpc_external_queue.py Normal file → Executable file
View File

View File

@ -14,7 +14,6 @@ class GetpeerinfoDeprecationTest(BitcoinTestFramework):
def run_test(self): def run_test(self):
self.test_banscore_deprecation() self.test_banscore_deprecation()
self.test_addnode_deprecation()
def test_banscore_deprecation(self): def test_banscore_deprecation(self):
self.log.info("Test getpeerinfo by default no longer returns a banscore field") self.log.info("Test getpeerinfo by default no longer returns a banscore field")
@ -23,16 +22,6 @@ class GetpeerinfoDeprecationTest(BitcoinTestFramework):
self.log.info("Test getpeerinfo returns banscore with -deprecatedrpc=banscore") self.log.info("Test getpeerinfo returns banscore with -deprecatedrpc=banscore")
assert "banscore" in self.nodes[1].getpeerinfo()[0].keys() 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__": if __name__ == "__main__":
GetpeerinfoDeprecationTest().main() GetpeerinfoDeprecationTest().main()