diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -9,3 +9,7 @@ - `getpeerinfo` no longer returns the `banscore` field unless the configuration option `-deprecatedrpc=banscore` is used. The `banscore` field will be fully removed in the next major release. + +- 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. diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -617,11 +617,6 @@ "prefixed by the net-specific datadir location.", DEFAULT_ASMAP_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-banscore=", - strprintf("Threshold for disconnecting and discouraging " - "misbehaving peers (default: %u)", - DEFAULT_BANSCORE_THRESHOLD), - ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-bantime=", strprintf("Default duration (in seconds) of manually " "configured bans (default: %u)", diff --git a/src/net_processing.h b/src/net_processing.h --- a/src/net_processing.h +++ b/src/net_processing.h @@ -29,6 +29,9 @@ */ static const unsigned int DEFAULT_BLOCK_RECONSTRUCTION_EXTRA_TXN = 100; 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}; class PeerLogicValidation final : public CValidationInterface, public NetEventsInterface { @@ -131,7 +134,11 @@ /** Get statistics from node state */ bool GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats); -/** Increase a node's misbehavior score. */ +/** + * Increment peer's misbehavior score. If the new value >= + * DISCOURAGEMENT_THRESHOLD, mark the node to be discouraged, meaning the peer + * might be disconnected and added to the discouragement filter. + */ void Misbehaving(NodeId nodeid, int howmuch, const std::string &message = "") EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/net_processing.cpp b/src/net_processing.cpp --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1167,7 +1167,9 @@ } /** - * Mark a misbehaving peer to be banned depending upon the value of `-banscore`. + * Increment peer's misbehavior score. If the new value >= + * DISCOURAGEMENT_THRESHOLD, mark the node to be discouraged, meaning the peer + * might be disconnected and added to the discouragement filter. */ void Misbehaving(NodeId pnode, int howmuch, const std::string &message) { AssertLockHeld(cs_main); @@ -1181,10 +1183,9 @@ } state->nMisbehavior += howmuch; - int banscore = gArgs.GetArg("-banscore", DEFAULT_BANSCORE_THRESHOLD); std::string message_prefixed = message.empty() ? "" : (": " + message); - if (state->nMisbehavior >= banscore && - state->nMisbehavior - howmuch < banscore) { + if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && + state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) { LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior - howmuch, diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -256,8 +256,8 @@ dummyNode1.fSuccessfullyConnected = true; { LOCK(cs_main); - // Should get banned. - Misbehaving(dummyNode1.GetId(), 100, ""); + // Should be discouraged + Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD); } { LOCK2(cs_main, dummyNode1.cs_sendProcessing); @@ -265,7 +265,7 @@ peerLogic->SendMessages(config, &dummyNode1, interruptDummy)); } BOOST_CHECK(banman->IsDiscouraged(addr1)); - // Different IP, not banned + // Different IP, not discouraged BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001 | 0x0000ff00))); CAddress addr2(ip(0xa0b0c002), NODE_NONE); @@ -284,7 +284,7 @@ BOOST_CHECK( peerLogic->SendMessages(config, &dummyNode2, interruptDummy)); } - // 2 not banned yet... + // 2 not discouraged yet... BOOST_CHECK(!banman->IsDiscouraged(addr2)); // ... but 1 still should be BOOST_CHECK(banman->IsDiscouraged(addr1)); @@ -317,8 +317,6 @@ *m_node.mempool); banman->ClearBanned(); - // because 11 is my favorite number. - gArgs.ForceSetArg("-banscore", "111"); CAddress addr1(ip(0xa0b0c001), NODE_NONE); CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, CAddress(), "", ConnectionType::INBOUND); @@ -328,7 +326,7 @@ dummyNode1.fSuccessfullyConnected = true; { LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 100, ""); + Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD - 11); } { LOCK2(cs_main, dummyNode1.cs_sendProcessing); @@ -356,7 +354,6 @@ peerLogic->SendMessages(config, &dummyNode1, interruptDummy)); } BOOST_CHECK(banman->IsDiscouraged(addr1)); - gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); bool dummy; peerLogic->FinalizeNode(config, dummyNode1.GetId(), dummy); @@ -389,7 +386,7 @@ { LOCK(cs_main); - Misbehaving(dummyNode.GetId(), 100, ""); + Misbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); } { LOCK2(cs_main, dummyNode.cs_sendProcessing); diff --git a/src/validation.h b/src/validation.h --- a/src/validation.h +++ b/src/validation.h @@ -134,7 +134,6 @@ static const bool DEFAULT_CHECKPOINTS_ENABLED = true; static const bool DEFAULT_TXINDEX = 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; diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -25,7 +25,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import wait_until -banscore = 10 +DISCOURAGEMENT_THRESHOLD = 100 class CLazyNode(P2PInterface): @@ -94,7 +94,7 @@ # behavior changes def on_open(self): super().on_open() - for i in range(banscore): + for _ in range(DISCOURAGEMENT_THRESHOLD): self.send_message(msg_verack()) def on_reject(self, message): pass @@ -131,7 +131,6 @@ class P2PLeakTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [['-banscore=' + str(banscore)]] def run_test(self): no_version_bannode = self.nodes[0].add_p2p_connection(