Merge #19464: net: remove -banscore configuration option

06059b0c2a6c2db70c87a7715f8a344a13400fa1 net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack)
1d4024bca8086cceff7539dd8c15e0b7fe1cc5ea net: remove -banscore configuration option (Jon Atack)

Pull request description:

  per https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340, https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592. Edit: now split into 3 straightforward PRs:
  - net: remove -banscore configuration option (this PR)
  - rpc: deprecate banscore field in getpeerinfo (#19469, *merged*)
  - gui: no longer display banscores (TBA in the gui repo)

ACKs for top commit:
  MarcoFalke:
    review ACK 06059b0c2a6c2db70c87a7715f8a344a13400fa1 📙
  vasild:
    ACK 06059b0c

Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
This commit is contained in:
MarcoFalke 2020-07-14 08:13:18 +02:00 committed by pasta
parent c0c5d05419
commit b9799de985
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
8 changed files with 19 additions and 16 deletions

View File

@ -0,0 +1,7 @@
Updated settings
----------------
- The `-banscore` configuration option, which modified the default threshold for
disconnecting and discouraging misbehaving peers, has been removed as part of
changes in this release to the handling of misbehaving peers. Refer to the
section, "Changes regarding misbehaving peers", for details. (dash#5511)

View File

@ -572,7 +572,6 @@ void SetupServerArgs(NodeContext& node)
argsman.AddArg("-asmap=<file>", strprintf("Specify asn mapping used for bucketing of the peers (default: %s). Relative paths will be prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-addnode=<ip>", "Add a node to connect to and attempt to keep the connection open (see the `addnode` RPC command help for more info). This option can be specified multiple times to add multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
argsman.AddArg("-allowprivatenet", strprintf("Allow RFC1918 addresses to be relayed and connected to (default: %u)", DEFAULT_ALLOWPRIVATENET), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting and discouraging misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
argsman.AddArg("-bind=<addr>[:<port>][=onion]", strprintf("Bind to given address and always listen on it (default: 0.0.0.0). Use [host]:port notation for IPv6. Append =onion to tag any incoming connections to that address and port as incoming Tor connections (default: 127.0.0.1:%u=onion, testnet: 127.0.0.1:%u=onion, regtest: 127.0.0.1:%u=onion)", defaultBaseParams->OnionServiceTargetPort(), testnetBaseParams->OnionServiceTargetPort(), regtestBaseParams->OnionServiceTargetPort()), ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
argsman.AddArg("-connect=<ip>", "Connect only to the specified node; -noconnect disables automatic connections (the rules for this peer are the same as for -addnode). This option can be specified multiple times to connect to multiple nodes.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);

View File

@ -1476,9 +1476,8 @@ void PeerManagerImpl::Misbehaving(const NodeId pnode, const int howmuch, const s
LOCK(peer->m_misbehavior_mutex);
peer->m_misbehavior_score += howmuch;
const int banscore = gArgs.GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD);
const std::string message_prefixed = message.empty() ? "" : (": " + message);
if (peer->m_misbehavior_score >= banscore && peer->m_misbehavior_score - howmuch < banscore)
if (peer->m_misbehavior_score >= DISCOURAGEMENT_THRESHOLD && peer->m_misbehavior_score - howmuch < DISCOURAGEMENT_THRESHOLD)
{
LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, peer->m_misbehavior_score - howmuch, peer->m_misbehavior_score, message_prefixed);
peer->m_should_discourage = true;

View File

@ -31,6 +31,8 @@ static const unsigned int DEFAULT_MAX_ORPHAN_TRANSACTIONS_SIZE = 10; // this all
static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100;
static const bool DEFAULT_PEERBLOOMFILTERS = true;
static const bool DEFAULT_PEERBLOCKFILTERS = false;
/** Threshold for marking a node to be discouraged, e.g. disconnected and added to the discouragement filter. */
static const int DISCOURAGEMENT_THRESHOLD{100};
struct CNodeStateStats {
int m_misbehavior_score = 0;
@ -64,7 +66,7 @@ public:
virtual void SetBestHeight(int height) = 0;
/**
* Increment peer's misbehavior score. If the new value surpasses banscore (specified on startup or by default), mark node to be discouraged, meaning the peer might be disconnected & added to the discouragement filter.
* Increment peer's misbehavior score. If the new value surpasses DISCOURAGEMENT_THRESHOLD (specified on startup or by default), mark node to be discouraged, meaning the peer might be disconnected & added to the discouragement filter.
*/
virtual void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message = "") = 0;

View File

@ -229,7 +229,7 @@ static RPCHelpMan getpeerinfo()
obj.pushKV("startingheight", stats.nStartingHeight);
if (fStateStats) {
if (IsDeprecatedRPCEnabled("banscore")) {
// banscore is deprecated in v0.21 for removal in v0.22
// banscore is deprecated in v21 for removal in v22
obj.pushKV("banscore", statestats.m_misbehavior_score);
}
obj.pushKV("synced_headers", statestats.nSyncHeight);

View File

@ -219,13 +219,13 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
dummyNode1.SetCommonVersion(PROTOCOL_VERSION);
peerLogic->InitializeNode(&dummyNode1);
dummyNode1.fSuccessfullyConnected = true;
peerLogic->Misbehaving(dummyNode1.GetId(), 100); // Should get banned
peerLogic->Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged
{
LOCK(dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
}
BOOST_CHECK(banman->IsDiscouraged(addr1));
BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not discouraged
CAddress addr2(ip(0xa0b0c002), NODE_NONE);
CNode dummyNode2(id++, NODE_NETWORK, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", ConnectionType::INBOUND);
@ -237,7 +237,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK(dummyNode2.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
}
BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not banned yet...
BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not discouraged yet...
BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be
peerLogic->Misbehaving(dummyNode2.GetId(), 50);
{
@ -260,14 +260,13 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
::deterministicMNManager, m_node.cj_ctx, m_node.llmq_ctx, false);
banman->ClearBanned();
gArgs.ForceSetArg("-banscore", "111"); // because 11 is my favorite number
CAddress addr1(ip(0xa0b0c001), NODE_NONE);
CNode dummyNode1(id++, NODE_NETWORK, INVALID_SOCKET, addr1, 3, 1, CAddress(), "", ConnectionType::INBOUND);
dummyNode1.SetCommonVersion(PROTOCOL_VERSION);
peerLogic->InitializeNode(&dummyNode1);
dummyNode1.fSuccessfullyConnected = true;
{
peerLogic->Misbehaving(dummyNode1.GetId(), 100);
peerLogic->Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD - 11);
}
{
LOCK(dummyNode1.cs_sendProcessing);
@ -290,7 +289,6 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
}
BOOST_CHECK(banman->IsDiscouraged(addr1));
gArgs.ForceSetArg("-banscore", ToString(DEFAULT_BANSCORE_THRESHOLD));
peerLogic->FinalizeNode(dummyNode1);
}
@ -314,7 +312,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
peerLogic->InitializeNode(&dummyNode);
dummyNode.fSuccessfullyConnected = true;
peerLogic->Misbehaving(dummyNode.GetId(), 100);
peerLogic->Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD);
{
LOCK(dummyNode.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode));

View File

@ -107,7 +107,6 @@ static const bool DEFAULT_ADDRESSINDEX = false;
static const bool DEFAULT_TIMESTAMPINDEX = false;
static const bool DEFAULT_SPENTINDEX = false;
static const char* const DEFAULT_BLOCKFILTERINDEX = "0";
static const unsigned int DEFAULT_BANSCORE_THRESHOLD = 100;
/** Default for -persistmempool */
static const bool DEFAULT_PERSIST_MEMPOOL = true;
/** Default for -syncmempool */

View File

@ -25,7 +25,7 @@ from test_framework.util import (
assert_greater_than_or_equal,
)
banscore = 10
DISCOURAGEMENT_THRESHOLD = 100
class LazyPeer(P2PInterface):
@ -91,7 +91,6 @@ class P2PVersionStore(P2PInterface):
class P2PLeakTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
self.extra_args = [['-banscore=' + str(banscore)]]
def setup_network(self):
self.disable_mocktime()
@ -111,7 +110,7 @@ class P2PLeakTest(BitcoinTestFramework):
# Send enough ping messages (any non-version message will do) prior to sending
# version to reach the peer discouragement threshold. This should get us disconnected.
for _ in range(banscore):
for _ in range(DISCOURAGEMENT_THRESHOLD):
no_version_ban_peer.send_message(msg_ping())
# Wait until we got the verack in response to the version. Though, don't wait for the node to receive the