diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -6,10 +6,15 @@ This release includes the following features and fixes: -- `getpeerinfo` no longer returns the `banscore` field unless the configuration +- The `getpeerinfo` RPC 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. + +GUI changes +----------- + +- The GUI Peers window no longer displays a "Ban Score" field. diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -1270,36 +1270,13 @@ - - - Ban Score - - - - - - - IBeamCursor - - - N/A - - - Qt::PlainText - - - Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse - - - - Connection Time - + IBeamCursor @@ -1315,14 +1292,14 @@ - + Last Send - + IBeamCursor @@ -1338,14 +1315,14 @@ - + Last Receive - + IBeamCursor @@ -1361,14 +1338,14 @@ - + Sent - + IBeamCursor @@ -1384,14 +1361,14 @@ - + Received - + IBeamCursor @@ -1407,14 +1384,14 @@ - + Ping Time - + IBeamCursor @@ -1430,7 +1407,7 @@ - + The duration of a currently outstanding ping. @@ -1440,7 +1417,7 @@ - + IBeamCursor @@ -1456,14 +1433,14 @@ - + Min Ping - + IBeamCursor @@ -1479,14 +1456,14 @@ - + Time Offset - + IBeamCursor diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1307,10 +1307,6 @@ // This check fails for example if the lock was busy and // nodeStateStats couldn't be fetched. if (stats->fNodeStateStatsAvailable) { - // Ban score is init to 0 - ui->peerBanScore->setText( - QString("%1").arg(stats->nodeStateStats.nMisbehavior)); - // Sync height is init to -1 if (stats->nodeStateStats.nSyncHeight > -1) { ui->peerSyncHeight->setText( 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 @@ -234,7 +234,7 @@ connman->ClearNodes(); } -BOOST_AUTO_TEST_CASE(DoS_banning) { +BOOST_AUTO_TEST_CASE(peer_discouragement) { const Config &config = GetConfig(); std::atomic interruptDummy(false); @@ -277,7 +277,7 @@ dummyNode2.fSuccessfullyConnected = true; { LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), 50, ""); + Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1); } { LOCK2(cs_main, dummyNode2.cs_sendProcessing); @@ -290,75 +290,22 @@ BOOST_CHECK(banman->IsDiscouraged(addr1)); { LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), 50, ""); + // 2 reaches discouragement threshold + Misbehaving(dummyNode2.GetId(), 1); } { LOCK2(cs_main, dummyNode2.cs_sendProcessing); BOOST_CHECK( peerLogic->SendMessages(config, &dummyNode2, interruptDummy)); } - BOOST_CHECK(banman->IsDiscouraged(addr2)); + BOOST_CHECK(banman->IsDiscouraged(addr1)); // Expect both 1 and 2 + BOOST_CHECK(banman->IsDiscouraged(addr2)); // to be discouraged now bool dummy; peerLogic->FinalizeNode(config, dummyNode1.GetId(), dummy); peerLogic->FinalizeNode(config, dummyNode2.GetId(), dummy); } -BOOST_AUTO_TEST_CASE(DoS_banscore) { - const Config &config = GetConfig(); - std::atomic interruptDummy(false); - - auto banman = std::make_unique(GetDataDir() / "banlist.dat", - config.GetChainParams(), nullptr, - DEFAULT_MISBEHAVING_BANTIME); - auto connman = std::make_unique(config, 0x1337, 0x1337); - auto peerLogic = std::make_unique( - connman.get(), banman.get(), *m_node.scheduler, *m_node.chainman, - *m_node.mempool); - - banman->ClearBanned(); - CAddress addr1(ip(0xa0b0c001), NODE_NONE); - CNode dummyNode1(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr1, 3, 1, - CAddress(), "", ConnectionType::INBOUND); - dummyNode1.SetSendVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(config, &dummyNode1); - dummyNode1.nVersion = 1; - dummyNode1.fSuccessfullyConnected = true; - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), DISCOURAGEMENT_THRESHOLD - 11); - } - { - LOCK2(cs_main, dummyNode1.cs_sendProcessing); - BOOST_CHECK( - peerLogic->SendMessages(config, &dummyNode1, interruptDummy)); - } - BOOST_CHECK(!banman->IsDiscouraged(addr1)); - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 10, ""); - } - { - LOCK2(cs_main, dummyNode1.cs_sendProcessing); - BOOST_CHECK( - peerLogic->SendMessages(config, &dummyNode1, interruptDummy)); - } - BOOST_CHECK(!banman->IsDiscouraged(addr1)); - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 1, ""); - } - { - LOCK2(cs_main, dummyNode1.cs_sendProcessing); - BOOST_CHECK( - peerLogic->SendMessages(config, &dummyNode1, interruptDummy)); - } - BOOST_CHECK(banman->IsDiscouraged(addr1)); - - bool dummy; - peerLogic->FinalizeNode(config, dummyNode1.GetId(), dummy); -} - BOOST_AUTO_TEST_CASE(DoS_bantime) { const Config &config = GetConfig(); std::atomic interruptDummy(false); 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 @@ -88,10 +88,10 @@ # anyway, and eventually get disconnected. -class CNodeNoVersionBan(CLazyNode): - # send a bunch of veracks without sending a message. This should get us disconnected. - # NOTE: implementation-specific check here. Remove if bitcoind ban - # behavior changes +class CNodeNoVersionMisbehavior(CLazyNode): + # Send enough veracks without a message to reach the peer discouragement + # threshold. This should get us disconnected. NOTE: implementation-specific + # test; update if our discouragement policy for peer misbehavior changes. def on_open(self): super().on_open() for _ in range(DISCOURAGEMENT_THRESHOLD): @@ -133,8 +133,8 @@ self.num_nodes = 1 def run_test(self): - no_version_bannode = self.nodes[0].add_p2p_connection( - CNodeNoVersionBan(), send_version=False, wait_for_verack=False) + no_version_disconnect_node = self.nodes[0].add_p2p_connection( + CNodeNoVersionMisbehavior(), send_version=False, wait_for_verack=False) no_version_idlenode = self.nodes[0].add_p2p_connection( CNodeNoVersionIdle(), send_version=False, wait_for_verack=False) no_verack_idlenode = self.nodes[0].add_p2p_connection( @@ -144,8 +144,10 @@ # verack, since we never sent one no_verack_idlenode.wait_for_verack() - wait_until(lambda: no_version_bannode.ever_connected, - timeout=10, lock=mininode_lock) + wait_until( + lambda: no_version_disconnect_node.ever_connected, + timeout=10, + lock=mininode_lock) wait_until(lambda: no_version_idlenode.ever_connected, timeout=10, lock=mininode_lock) wait_until(lambda: no_verack_idlenode.version_received, @@ -158,8 +160,8 @@ # Give the node enough time to possibly leak out a message time.sleep(5) - # This node should have been banned - assert not no_version_bannode.is_connected + # Expect this node to be disconnected for misbehavior + assert not no_version_disconnect_node.is_connected self.nodes[0].disconnect_p2ps() @@ -167,7 +169,7 @@ wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 0) # Make sure no unexpected messages came in - assert not no_version_bannode.unexpected_msg + assert not no_version_disconnect_node.unexpected_msg assert not no_version_idlenode.unexpected_msg assert not no_verack_idlenode.unexpected_msg