foster/bitcoin-bitcoin-core
0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-08 10:31:50 -05:00
Code Issues Activity

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

Browse source 
454a4088a8 [doc] Add release notes for removed getpeerinfo fields. (Amiti Uttarwar)
b1a936d4ae [rpc] Remove deprecated "whitelisted" field from getpeerinfo (Amiti Uttarwar)
094c3beaa4 [rpc] Remove deprecated "banscore" field from getpeerinfo (Amiti Uttarwar)
537053336f [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 454a4088a8
  jnewbery:
    ACK 454a4088a8.

Tree-SHA512: ccc0e90c0763eeb8529cf0c46162dbaca3f7773981b3b52d9925166ea7421aed086795d56b320e16c9340f68862388785f52a9b78314865070917b33180d7cd6
This commit is contained in:
MarcoFalke 2020-12-28 22:40:33 +01:00
commit e3dd0a56cf
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
10 changed files with 17 additions and 106 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
doc/release-notes.md
View file

@ -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.

4
src/net.cpp
View file

@ -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);

4
src/net.h
View file

@ -702,7 +702,6 @@ public:
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 @@ public:
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 @@ public:
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
/**

1
src/net_processing.cpp
View file

@ -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;

1
src/net_processing.h
View file

@ -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;

17
src/rpc/net.cpp
View file

@ -128,12 +128,9 @@ 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"},
@ -141,8 +138,6 @@ static RPCHelpMan getpeerinfo()
{
{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);

2
test/functional/p2p_blocksonly.py
View file

@ -59,7 +59,7 @@ class P2PBlocksOnly(BitcoinTestFramework):
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())

50
test/functional/p2p_permissions.py
View file

@ -38,35 +38,24 @@ class P2PPermissionsTests(BitcoinTestFramework):
# 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 @@ class P2PPermissionsTests(BitcoinTestFramework):
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 @@ class P2PPermissionsTests(BitcoinTestFramework):
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):

38
test/functional/rpc_getpeerinfo_deprecation.py
View file

@ -1,38 +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
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__":
GetpeerinfoDeprecationTest().main()

1
test/functional/test_runner.py
View file

@ -279,7 +279,6 @@ BASE_SCRIPTS = [
'feature_config_args.py',
'feature_settings.py',
'rpc_getdescriptorinfo.py',
'rpc_getpeerinfo_deprecation.py',
'rpc_help.py',
'feature_help.py',
'feature_shutdown.py',