diff options
| -rw-r--r-- | doc/release-notes.md | 12 | ||||
| -rw-r--r-- | src/qt/forms/debugwindow.ui | 63 | ||||
| -rw-r--r-- | src/qt/rpcconsole.cpp | 3 | ||||
| -rw-r--r-- | src/test/denialofservice_tests.cpp | 54 | ||||
| -rwxr-xr-x | test/functional/p2p_leak.py | 18 |
5 files changed, 44 insertions, 106 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index 2a2e0f9472..23983dcd7b 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -98,7 +98,7 @@ will trigger BIP 125 (replace-by-fee) opt-in. (#11413) - The `bumpfee` command now uses `conf_target` rather than `confTarget` in the options. (#11413) -- `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. (#19469) @@ -123,8 +123,9 @@ 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. (#19464) + changes in 0.20.1 and in this release to the handling of misbehaving peers. + Refer to "Changes regarding misbehaving peers" in the 0.20.1 release notes for + details. (#19464) - The `-debug=db` logging category, which was deprecated in 0.20 and replaced by `-debug=walletdb` to distinguish it from `coindb`, has been removed. (#19202) @@ -303,6 +304,11 @@ issue. GUI changes ----------- +- The GUI Peers window no longer displays a "Ban Score" field. This is part of + changes in 0.20.1 and in this release to the handling of misbehaving + peers. Refer to "Changes regarding misbehaving peers" in the 0.20.1 release + notes for details. (#19512) + Low-level changes ================= diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 1217ca3e2e..4747dd6d26 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -1264,36 +1264,13 @@ </widget> </item> <item row="8" column="0"> - <widget class="QLabel" name="label_24"> - <property name="text"> - <string>Ban Score</string> - </property> - </widget> - </item> - <item row="8" column="1"> - <widget class="QLabel" name="peerBanScore"> - <property name="cursor"> - <cursorShape>IBeamCursor</cursorShape> - </property> - <property name="text"> - <string>N/A</string> - </property> - <property name="textFormat"> - <enum>Qt::PlainText</enum> - </property> - <property name="textInteractionFlags"> - <set>Qt::LinksAccessibleByMouse|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set> - </property> - </widget> - </item> - <item row="9" column="0"> <widget class="QLabel" name="label_22"> <property name="text"> <string>Connection Time</string> </property> </widget> </item> - <item row="9" column="1"> + <item row="8" column="1"> <widget class="QLabel" name="peerConnTime"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1309,14 +1286,14 @@ </property> </widget> </item> - <item row="10" column="0"> + <item row="9" column="0"> <widget class="QLabel" name="label_15"> <property name="text"> <string>Last Send</string> </property> </widget> </item> - <item row="10" column="1"> + <item row="9" column="1"> <widget class="QLabel" name="peerLastSend"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1332,14 +1309,14 @@ </property> </widget> </item> - <item row="11" column="0"> + <item row="10" column="0"> <widget class="QLabel" name="label_19"> <property name="text"> <string>Last Receive</string> </property> </widget> </item> - <item row="11" column="1"> + <item row="10" column="1"> <widget class="QLabel" name="peerLastRecv"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1355,14 +1332,14 @@ </property> </widget> </item> - <item row="12" column="0"> + <item row="11" column="0"> <widget class="QLabel" name="label_18"> <property name="text"> <string>Sent</string> </property> </widget> </item> - <item row="12" column="1"> + <item row="11" column="1"> <widget class="QLabel" name="peerBytesSent"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1378,14 +1355,14 @@ </property> </widget> </item> - <item row="13" column="0"> + <item row="12" column="0"> <widget class="QLabel" name="label_20"> <property name="text"> <string>Received</string> </property> </widget> </item> - <item row="13" column="1"> + <item row="12" column="1"> <widget class="QLabel" name="peerBytesRecv"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1401,14 +1378,14 @@ </property> </widget> </item> - <item row="14" column="0"> + <item row="13" column="0"> <widget class="QLabel" name="label_26"> <property name="text"> <string>Ping Time</string> </property> </widget> </item> - <item row="14" column="1"> + <item row="13" column="1"> <widget class="QLabel" name="peerPingTime"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1424,7 +1401,7 @@ </property> </widget> </item> - <item row="15" column="0"> + <item row="14" column="0"> <widget class="QLabel" name="peerPingWaitLabel"> <property name="toolTip"> <string>The duration of a currently outstanding ping.</string> @@ -1434,7 +1411,7 @@ </property> </widget> </item> - <item row="15" column="1"> + <item row="14" column="1"> <widget class="QLabel" name="peerPingWait"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1450,14 +1427,14 @@ </property> </widget> </item> - <item row="16" column="0"> + <item row="15" column="0"> <widget class="QLabel" name="peerMinPingLabel"> <property name="text"> <string>Min Ping</string> </property> </widget> </item> - <item row="16" column="1"> + <item row="15" column="1"> <widget class="QLabel" name="peerMinPing"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1473,14 +1450,14 @@ </property> </widget> </item> - <item row="17" column="0"> + <item row="16" column="0"> <widget class="QLabel" name="label_timeoffset"> <property name="text"> <string>Time Offset</string> </property> </widget> </item> - <item row="17" column="1"> + <item row="16" column="1"> <widget class="QLabel" name="timeoffset"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1496,7 +1473,7 @@ </property> </widget> </item> - <item row="18" column="0"> + <item row="17" column="0"> <widget class="QLabel" name="peerMappedASLabel"> <property name="toolTip"> <string>The mapped Autonomous System used for diversifying peer selection.</string> @@ -1506,7 +1483,7 @@ </property> </widget> </item> - <item row="18" column="1"> + <item row="17" column="1"> <widget class="QLabel" name="peerMappedAS"> <property name="cursor"> <cursorShape>IBeamCursor</cursorShape> @@ -1522,7 +1499,7 @@ </property> </widget> </item> - <item row="19" column="0"> + <item row="18" column="0"> <spacer name="verticalSpacer_3"> <property name="orientation"> <enum>Qt::Vertical</enum> diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 71094f7112..694e1c2eae 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -1126,9 +1126,6 @@ void RPCConsole::updateNodeDetail(const CNodeCombinedStats *stats) // 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(QString("%1").arg(stats->nodeStateStats.nSyncHeight)); diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp index 3d84fa855f..d49b51926c 100644 --- a/src/test/denialofservice_tests.cpp +++ b/src/test/denialofservice_tests.cpp @@ -217,7 +217,7 @@ BOOST_AUTO_TEST_CASE(stale_tip_peer_management) connman->ClearNodes(); } -BOOST_AUTO_TEST_CASE(DoS_banning) +BOOST_AUTO_TEST_CASE(peer_discouragement) { auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); auto connman = MakeUnique<CConnman>(0x1337, 0x1337); @@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning) dummyNode2.fSuccessfullyConnected = true; { LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), 50); + Misbehaving(dummyNode2.GetId(), DISCOURAGEMENT_THRESHOLD - 1); } { LOCK2(cs_main, dummyNode2.cs_sendProcessing); @@ -259,64 +259,20 @@ BOOST_AUTO_TEST_CASE(DoS_banning) BOOST_CHECK(banman->IsDiscouraged(addr1)); // ... but 1 still should be { LOCK(cs_main); - Misbehaving(dummyNode2.GetId(), 50); + Misbehaving(dummyNode2.GetId(), 1); // 2 reaches discouragement threshold } { LOCK2(cs_main, dummyNode2.cs_sendProcessing); BOOST_CHECK(peerLogic->SendMessages(&dummyNode2)); } - 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(dummyNode1.GetId(), dummy); peerLogic->FinalizeNode(dummyNode2.GetId(), dummy); } -BOOST_AUTO_TEST_CASE(DoS_banscore) -{ - auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); - auto connman = MakeUnique<CConnman>(0x1337, 0x1337); - auto peerLogic = MakeUnique<PeerLogicValidation>(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(), "", true); - dummyNode1.SetSendVersion(PROTOCOL_VERSION); - peerLogic->InitializeNode(&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(&dummyNode1)); - } - BOOST_CHECK(!banman->IsDiscouraged(addr1)); - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 10); - } - { - LOCK2(cs_main, dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); - } - BOOST_CHECK(!banman->IsDiscouraged(addr1)); - { - LOCK(cs_main); - Misbehaving(dummyNode1.GetId(), 1); - } - { - LOCK2(cs_main, dummyNode1.cs_sendProcessing); - BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); - } - BOOST_CHECK(banman->IsDiscouraged(addr1)); - - bool dummy; - peerLogic->FinalizeNode(dummyNode1.GetId(), dummy); -} - BOOST_AUTO_TEST_CASE(DoS_bantime) { auto banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index 6cf252e626..fe6e236fc4 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -65,9 +65,10 @@ class CLazyNode(P2PInterface): # Node that never sends a version. We'll use this to send a bunch of messages # 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): @@ -108,7 +109,8 @@ class P2PLeakTest(BitcoinTestFramework): 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(CNodeNoVerackIdle(), wait_for_verack=False) @@ -116,7 +118,7 @@ class P2PLeakTest(BitcoinTestFramework): # 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, timeout=10, lock=mininode_lock) @@ -126,13 +128,13 @@ class P2PLeakTest(BitcoinTestFramework): #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() # Make sure no unexpected messages came in - assert no_version_bannode.unexpected_msg == False + assert no_version_disconnect_node.unexpected_msg == False assert no_version_idlenode.unexpected_msg == False assert no_verack_idlenode.unexpected_msg == False |