Merge #19512: p2p: banscore updates to gui, tests, release notes

fa108d6a757838225179a8df942cfb6d99c98c90 test: update tests for peer discouragement (Jon Atack)
1a9f462caa63fa16d7b4415312d2032a42b3fe0b gui, doc: rm Ban Score in GUI Peers window/release notes updates (Jon Atack)

Pull request description:

  This is the third `-banscore` PR in the mini-series described in #19464. See that PR for the intention and reasoning.

  - no longer display "Ban Score" in the GUI peers window and add a release note, plus release note fixups per https://github.com/bitcoin/bitcoin/pull/19464#pullrequestreview-447452052
  - update tests (`src/test/denialofservice_tests.cpp` and `test/functional/p2p_leak.py`) from banning to discouragement and per https://github.com/bitcoin/bitcoin/pull/19464#issuecomment-658052518

ACKs for top commit:
  jnewbery:
    ACK fa108d6a757838225179a8df942cfb6d99c98c90
  laanwj:
    ACK fa108d6a757838225179a8df942cfb6d99c98c90

Tree-SHA512: 58a449b3f47b8cb5490b34e4442ee8675bfad1ce48af4e4fd5c67715b0c1a596fb8e731d42e576b4c3b64627f76e0a68cbb1da9ea9f588a5932fe119baf40d50

3 files changed, 25 insertions, 95 deletions

diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui
index 1217ca3e2..4747dd6d2 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 71094f711..694e1c2ea 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 3d84fa855..d49b51926 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);