Merge #19219: Replace automatic bans with discouragement filter

2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Clean up separated ban/discourage interface (Pieter Wuille)
b691f2df5f7d443c0c9ee056ab94aa0fc19566d5 Replace automatic bans with discouragement filter (Pieter Wuille)

Pull request description:

  This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned since #14929, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full.

  Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers.

  Also change the name of this mechanism to "discouraged" to better reflect reality.

ACKs for top commit:
  naumenkogs:
    utACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8
  amitiuttarwar:
    code review ACK 2ad58381ff
  jonatack:
    ACK 2ad5838 per changes since last review `git range-diff 3276c14 1f7e0ca 2ad5838`
  jnewbery:
    Code review ACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8

Tree-SHA512: 5dedef401d9cbfa026812651303e6286223563dbeed7a10766ed536ac9e3f29ed4bd0df29cc6deadceeb35cbe9f066346add14ef0833958ca9f93d123fe7aab5

15 files changed, 131 insertions, 143 deletions

diff --git a/src/addrdb.h b/src/addrdb.h
index c6d4307d69..8410c3776c 100644
--- a/src/addrdb.h
+++ b/src/addrdb.h
@@ -17,13 +17,6 @@ class CSubNet;
 class CAddrMan;
 class CDataStream;
 
-typedef enum BanReason
-{
-    BanReasonUnknown          = 0,
-    BanReasonNodeMisbehaving  = 1,
-    BanReasonManuallyAdded    = 2
-} BanReason;
-
 class CBanEntry
 {
 public:
@@ -31,7 +24,6 @@ public:
     int nVersion;
     int64_t nCreateTime;
     int64_t nBanUntil;
-    uint8_t banReason;
 
     CBanEntry()
     {
@@ -44,31 +36,17 @@ public:
         nCreateTime = nCreateTimeIn;
     }
 
-    explicit CBanEntry(int64_t n_create_time_in, BanReason ban_reason_in) : CBanEntry(n_create_time_in)
+    SERIALIZE_METHODS(CBanEntry, obj)
     {
-        banReason = ban_reason_in;
+        uint8_t ban_reason = 2; //! For backward compatibility
+        READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, ban_reason);
     }
 
-    SERIALIZE_METHODS(CBanEntry, obj) { READWRITE(obj.nVersion, obj.nCreateTime, obj.nBanUntil, obj.banReason); }
-
     void SetNull()
     {
         nVersion = CBanEntry::CURRENT_VERSION;
         nCreateTime = 0;
         nBanUntil = 0;
-        banReason = BanReasonUnknown;
-    }
-
-    std::string banReasonToString() const
-    {
-        switch (banReason) {
-        case BanReasonNodeMisbehaving:
-            return "node misbehaving";
-        case BanReasonManuallyAdded:
-            return "manually added";
-        default:
-            return "unknown";
-        }
     }
 };
 
diff --git a/src/banman.cpp b/src/banman.cpp
index 422904bb33..2fabacda0e 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -68,28 +68,13 @@ void BanMan::ClearBanned()
     if (m_client_interface) m_client_interface->BannedListChanged();
 }
 
-int BanMan::IsBannedLevel(CNetAddr net_addr)
+bool BanMan::IsDiscouraged(const CNetAddr& net_addr)
 {
-    // Returns the most severe level of banning that applies to this address.
-    // 0 - Not banned
-    // 1 - Automatic misbehavior ban
-    // 2 - Any other ban
-    int level = 0;
-    auto current_time = GetTime();
     LOCK(m_cs_banned);
-    for (const auto& it : m_banned) {
-        CSubNet sub_net = it.first;
-        CBanEntry ban_entry = it.second;
-
-        if (current_time < ban_entry.nBanUntil && sub_net.Match(net_addr)) {
-            if (ban_entry.banReason != BanReasonNodeMisbehaving) return 2;
-            level = 1;
-        }
-    }
-    return level;
+    return m_discouraged.contains(net_addr.GetAddrBytes());
 }
 
-bool BanMan::IsBanned(CNetAddr net_addr)
+bool BanMan::IsBanned(const CNetAddr& net_addr)
 {
     auto current_time = GetTime();
     LOCK(m_cs_banned);
@@ -104,7 +89,7 @@ bool BanMan::IsBanned(CNetAddr net_addr)
     return false;
 }
 
-bool BanMan::IsBanned(CSubNet sub_net)
+bool BanMan::IsBanned(const CSubNet& sub_net)
 {
     auto current_time = GetTime();
     LOCK(m_cs_banned);
@@ -118,15 +103,21 @@ bool BanMan::IsBanned(CSubNet sub_net)
     return false;
 }
 
-void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+void BanMan::Ban(const CNetAddr& net_addr, int64_t ban_time_offset, bool since_unix_epoch)
 {
     CSubNet sub_net(net_addr);
-    Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch);
+    Ban(sub_net, ban_time_offset, since_unix_epoch);
+}
+
+void BanMan::Discourage(const CNetAddr& net_addr)
+{
+    LOCK(m_cs_banned);
+    m_discouraged.insert(net_addr.GetAddrBytes());
 }
 
-void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_unix_epoch)
 {
-    CBanEntry ban_entry(GetTime(), ban_reason);
+    CBanEntry ban_entry(GetTime());
 
     int64_t normalized_ban_time_offset = ban_time_offset;
     bool normalized_since_unix_epoch = since_unix_epoch;
@@ -146,8 +137,8 @@ void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ba
     }
     if (m_client_interface) m_client_interface->BannedListChanged();
 
-    //store banlist to disk immediately if user requested ban
-    if (ban_reason == BanReasonManuallyAdded) DumpBanlist();
+    //store banlist to disk immediately
+    DumpBanlist();
 }
 
 bool BanMan::Unban(const CNetAddr& net_addr)
diff --git a/src/banman.h b/src/banman.h
index 6bea2e75e9..f6bfbd1e49 100644
--- a/src/banman.h
+++ b/src/banman.h
@@ -6,6 +6,7 @@
 #define BITCOIN_BANMAN_H
 
 #include <addrdb.h>
+#include <bloom.h>
 #include <fs.h>
 #include <net_types.h> // For banmap_t
 #include <sync.h>
@@ -23,32 +24,55 @@ class CClientUIInterface;
 class CNetAddr;
 class CSubNet;
 
-// Denial-of-service detection/prevention
-// The idea is to detect peers that are behaving
-// badly and disconnect/ban them, but do it in a
-// one-coding-mistake-won't-shatter-the-entire-network
-// way.
-// IMPORTANT:  There should be nothing I can give a
-// node that it will forward on that will make that
-// node's peers drop it. If there is, an attacker
-// can isolate a node and/or try to split the network.
-// Dropping a node for sending stuff that is invalid
-// now but might be valid in a later version is also
-// dangerous, because it can cause a network split
-// between nodes running old code and nodes running
-// new code.
+// Banman manages two related but distinct concepts:
+//
+// 1. Banning. This is configured manually by the user, through the setban RPC.
+// If an address or subnet is banned, we never accept incoming connections from
+// it and never create outgoing connections to it. We won't gossip its address
+// to other peers in addr messages. Banned addresses and subnets are stored to
+// banlist.dat on shutdown and reloaded on startup. Banning can be used to
+// prevent connections with spy nodes or other griefers.
+//
+// 2. Discouragement. If a peer misbehaves enough (see Misbehaving() in
+// net_processing.cpp), we'll mark that address as discouraged. We still allow
+// incoming connections from them, but they're preferred for eviction when
+// we receive new incoming connections. We never make outgoing connections to
+// them, and do not gossip their address to other peers. This is implemented as
+// a bloom filter. We can (probabilistically) test for membership, but can't
+// list all discouraged addresses or unmark them as discouraged. Discouragement
+// can prevent our limited connection slots being used up by incompatible
+// or broken peers.
+//
+// Neither banning nor discouragement are protections against denial-of-service
+// attacks, since if an attacker has a way to waste our resources and we
+// disconnect from them and ban that address, it's trivial for them to
+// reconnect from another IP address.
+//
+// Attempting to automatically disconnect or ban any class of peer carries the
+// risk of splitting the network. For example, if we banned/disconnected for a
+// transaction that fails a policy check and a future version changes the
+// policy check so the transaction is accepted, then that transaction could
+// cause the network to split between old nodes and new nodes.
 
 class BanMan
 {
 public:
     ~BanMan();
     BanMan(fs::path ban_file, CClientUIInterface* client_interface, int64_t default_ban_time);
-    void Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
-    void Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+    void Ban(const CNetAddr& net_addr, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+    void Ban(const CSubNet& sub_net, int64_t ban_time_offset = 0, bool since_unix_epoch = false);
+    void Discourage(const CNetAddr& net_addr);
     void ClearBanned();
-    int IsBannedLevel(CNetAddr net_addr);
-    bool IsBanned(CNetAddr net_addr);
-    bool IsBanned(CSubNet sub_net);
+
+    //! Return whether net_addr is banned
+    bool IsBanned(const CNetAddr& net_addr);
+
+    //! Return whether sub_net is exactly banned
+    bool IsBanned(const CSubNet& sub_net);
+
+    //! Return whether net_addr is discouraged.
+    bool IsDiscouraged(const CNetAddr& net_addr);
+
     bool Unban(const CNetAddr& net_addr);
     bool Unban(const CSubNet& sub_net);
     void GetBanned(banmap_t& banmap);
@@ -68,6 +92,7 @@ private:
     CClientUIInterface* m_client_interface = nullptr;
     CBanDB m_ban_db;
     const int64_t m_default_ban_time;
+    CRollingBloomFilter m_discouraged GUARDED_BY(m_cs_banned) {50000, 0.000001};
 };
 
 #endif
diff --git a/src/init.cpp b/src/init.cpp
index 0427368e70..2e39bcd9a5 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -431,8 +431,8 @@ void SetupServerArgs(NodeContext& node)
 
     gArgs.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);
     gArgs.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);
-    gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
-    gArgs.AddArg("-bantime=<n>", strprintf("Number of seconds to keep misbehaving peers from reconnecting (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-banscore=<n>", strprintf("Threshold for disconnecting and discouraging misbehaving peers (default: %u)", DEFAULT_BANSCORE_THRESHOLD), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
+    gArgs.AddArg("-bantime=<n>", strprintf("Default duration (in seconds) of manually configured bans (default: %u)", DEFAULT_MISBEHAVING_BANTIME), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
     gArgs.AddArg("-bind=<addr>", "Bind to given address and always listen on it. Use [host]:port notation for IPv6", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::CONNECTION);
     gArgs.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);
     gArgs.AddArg("-discover", "Discover own IP addresses (default: 1 when listening and no -externalip or -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 5b9a15f268..834a16ecf5 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -146,10 +146,10 @@ public:
         }
         return false;
     }
-    bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) override
+    bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) override
     {
         if (m_context.banman) {
-            m_context.banman->Ban(net_addr, reason, ban_time_offset);
+            m_context.banman->Ban(net_addr, ban_time_offset);
             return true;
         }
         return false;
diff --git a/src/interfaces/node.h b/src/interfaces/node.h
index 877a40568f..b88b5bc14e 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -122,7 +122,7 @@ public:
     virtual bool getBanned(banmap_t& banmap) = 0;
 
     //! Ban node.
-    virtual bool ban(const CNetAddr& net_addr, BanReason reason, int64_t ban_time_offset) = 0;
+    virtual bool ban(const CNetAddr& net_addr, int64_t ban_time_offset) = 0;
 
     //! Unban node.
     virtual bool unban(const CSubNet& ip) = 0;
diff --git a/src/net.cpp b/src/net.cpp
index 281232d801..0233b0fc2f 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -1010,17 +1010,24 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     // on all platforms.  Set it again here just to be sure.
     SetSocketNoDelay(hSocket);
 
-    int bannedlevel = m_banman ? m_banman->IsBannedLevel(addr) : 0;
-
-    // Don't accept connections from banned peers, but if our inbound slots aren't almost full, accept
-    // if the only banning reason was an automatic misbehavior ban.
-    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel > ((nInbound + 1 < nMaxInbound) ? 1 : 0))
+    // Don't accept connections from banned peers.
+    bool banned = m_banman->IsBanned(addr);
+    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && banned)
     {
         LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
         CloseSocket(hSocket);
         return;
     }
 
+    // Only accept connections from discouraged peers if our inbound slots aren't (almost) full.
+    bool discouraged = m_banman->IsDiscouraged(addr);
+    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && discouraged)
+    {
+        LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString());
+        CloseSocket(hSocket);
+        return;
+    }
+
     if (nInbound >= nMaxInbound)
     {
         if (!AttemptToEvictConnection()) {
@@ -1044,7 +1051,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     pnode->m_permissionFlags = permissionFlags;
     // If this flag is present, the user probably expect that RPC and QT report it as whitelisted (backward compatibility)
     pnode->m_legacyWhitelisted = legacyWhitelisted;
-    pnode->m_prefer_evict = bannedlevel > 0;
+    pnode->m_prefer_evict = discouraged;
     m_msgproc->InitializeNode(pnode);
 
     LogPrint(BCLog::NET, "connection from %s accepted\n", addr.ToString());
@@ -2045,10 +2052,10 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
         return;
     }
     if (!pszDest) {
-        if (IsLocal(addrConnect) ||
-            FindNode(static_cast<CNetAddr>(addrConnect)) || (m_banman && m_banman->IsBanned(addrConnect)) ||
-            FindNode(addrConnect.ToStringIPPort()))
+        bool banned_or_discouraged = m_banman && (m_banman->IsDiscouraged(addrConnect) || m_banman->IsBanned(addrConnect));
+        if (IsLocal(addrConnect) || FindNode(static_cast<CNetAddr>(addrConnect)) || banned_or_discouraged || FindNode(addrConnect.ToStringIPPort())) {
             return;
+        }
     } else if (FindNode(std::string(pszDest)))
         return;
 
diff --git a/src/net_permissions.h b/src/net_permissions.h
index e004067e75..2cf85120fe 100644
--- a/src/net_permissions.h
+++ b/src/net_permissions.h
@@ -24,7 +24,7 @@ enum NetPermissionFlags
     // Always relay transactions from this peer, even if already in mempool
     // Keep parameter interaction: forcerelay implies relay
     PF_FORCERELAY = (1U << 2) | PF_RELAY,
-    // Can't be banned for misbehavior
+    // Can't be banned/disconnected/discouraged for misbehavior
     PF_NOBAN = (1U << 4),
     // Can query the mempool
     PF_MEMPOOL = (1U << 5),
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index e3053869ae..d0c3c943fc 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -249,8 +249,8 @@ struct CNodeState {
     bool fCurrentlyConnected;
     //! Accumulated misbehaviour score for this peer.
     int nMisbehavior;
-    //! Whether this peer should be disconnected and banned (unless whitelisted).
-    bool fShouldBan;
+    //! Whether this peer should be disconnected and marked as discouraged (unless whitelisted with noban).
+    bool m_should_discourage;
     //! String name of this peer (debugging/logging purposes).
     const std::string name;
     //! The best known block we know this peer has announced.
@@ -401,7 +401,7 @@ struct CNodeState {
     {
         fCurrentlyConnected = false;
         nMisbehavior = 0;
-        fShouldBan = false;
+        m_should_discourage = false;
         pindexBestKnownBlock = nullptr;
         hashLastUnknownBlock.SetNull();
         pindexLastCommonBlock = nullptr;
@@ -1016,7 +1016,7 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans)
 }
 
 /**
- * Mark a misbehaving peer to be banned depending upon the value of `-banscore`.
+ * 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.
  */
 void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 {
@@ -1032,14 +1032,14 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
     std::string message_prefixed = message.empty() ? "" : (": " + message);
     if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore)
     {
-        LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) BAN THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed);
-        state->fShouldBan = true;
+        LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed);
+        state->m_should_discourage = true;
     } else
         LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed);
 }
 
 /**
- * Potentially ban a node based on the contents of a BlockValidationState object
+ * Potentially mark a node discouraged based on the contents of a BlockValidationState object
  *
  * @param[in] via_compact_block this bool is passed in because net_processing should
  * punish peers differently depending on whether the data was provided in a compact
@@ -1069,7 +1069,7 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
                 break;
             }
 
-            // Ban outbound (but not inbound) peers if on an invalid chain.
+            // Discourage outbound (but not inbound) peers if on an invalid chain.
             // Exempt HB compact block peers and manual connections.
             if (!via_compact_block && !node_state->m_is_inbound && !node_state->m_is_manual_connection) {
                 Misbehaving(nodeid, 100, message);
@@ -1104,7 +1104,7 @@ static bool MaybePunishNodeForBlock(NodeId nodeid, const BlockValidationState& s
 }
 
 /**
- * Potentially ban a node based on the contents of a TxValidationState object
+ * Potentially disconnect and discourage a node based on the contents of a TxValidationState object
  *
  * @return Returns true if the peer was punished (probably disconnected)
  */
@@ -1336,7 +1336,7 @@ void PeerLogicValidation::UpdatedBlockTip(const CBlockIndex *pindexNew, const CB
 }
 
 /**
- * Handle invalid block rejection and consequent peer banning, maintain which
+ * Handle invalid block rejection and consequent peer discouragement, maintain which
  * peers announce compact blocks.
  */
 void PeerLogicValidation::BlockChecked(const CBlock& block, const BlockValidationState& state) {
@@ -2473,7 +2473,8 @@ void ProcessMessage(
             if (addr.nTime <= 100000000 || addr.nTime > nNow + 10 * 60)
                 addr.nTime = nNow - 5 * 24 * 60 * 60;
             pfrom.AddAddressKnown(addr);
-            if (banman->IsBanned(addr)) continue; // Do not process banned addresses beyond remembering we received them
+            if (banman->IsDiscouraged(addr)) continue; // Do not process banned/discouraged addresses beyond remembering we received them
+            if (banman->IsBanned(addr)) continue;
             bool fReachable = IsReachable(addr);
             if (addr.nTime > nSince && !pfrom.fGetAddr && vAddr.size() <= 10 && addr.IsRoutable())
             {
@@ -3115,7 +3116,7 @@ void ProcessMessage(
             // relayed before full validation (see BIP 152), so we don't want to disconnect
             // the peer if the header turns out to be for an invalid block.
             // Note that if a peer tries to build on an invalid chain, that
-            // will be detected and the peer will be banned.
+            // will be detected and the peer will be disconnected/discouraged.
             return ProcessHeadersMessage(pfrom, connman, chainman, mempool, {cmpctblock.header}, chainparams, /*via_compact_block=*/true);
         }
 
@@ -3201,7 +3202,7 @@ void ProcessMessage(
                 // 3. the block is otherwise invalid (eg invalid coinbase,
                 //    block is too big, too many legacy sigops, etc).
                 // So if CheckBlock failed, #3 is the only possibility.
-                // Under BIP 152, we don't DoS-ban unless proof of work is
+                // Under BIP 152, we don't discourage the peer unless proof of work is
                 // invalid (we don't require all the stateless checks to have
                 // been run).  This is handled below, so just treat this as
                 // though the block was successfully read, and rely on the
@@ -3326,7 +3327,7 @@ void ProcessMessage(
         std::vector<CAddress> vAddr = connman->GetAddresses();
         FastRandomContext insecure_rand;
         for (const CAddress &addr : vAddr) {
-            if (!banman->IsBanned(addr)) {
+            if (!banman->IsDiscouraged(addr) && !banman->IsBanned(addr)) {
                 pfrom.PushAddress(addr, insecure_rand);
             }
         }
@@ -3561,25 +3562,26 @@ void ProcessMessage(
     return;
 }
 
-bool PeerLogicValidation::CheckIfBanned(CNode& pnode)
+bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode)
 {
     AssertLockHeld(cs_main);
     CNodeState &state = *State(pnode.GetId());
 
-    if (state.fShouldBan) {
-        state.fShouldBan = false;
-        if (pnode.HasPermission(PF_NOBAN))
+    if (state.m_should_discourage) {
+        state.m_should_discourage = false;
+        if (pnode.HasPermission(PF_NOBAN)) {
             LogPrintf("Warning: not punishing whitelisted peer %s!\n", pnode.addr.ToString());
-        else if (pnode.m_manual_connection)
+        } else if (pnode.m_manual_connection) {
             LogPrintf("Warning: not punishing manually-connected peer %s!\n", pnode.addr.ToString());
-        else if (pnode.addr.IsLocal()) {
-            // Disconnect but don't ban _this_ local node
-            LogPrintf("Warning: disconnecting but not banning local peer %s!\n", pnode.addr.ToString());
+        } else if (pnode.addr.IsLocal()) {
+            // Disconnect but don't discourage this local node
+            LogPrintf("Warning: disconnecting but not discouraging local peer %s!\n", pnode.addr.ToString());
             pnode.fDisconnect = true;
         } else {
-            // Disconnect and ban all nodes sharing the address
+            // Disconnect and discourage all nodes sharing the address
+            LogPrintf("Disconnecting and discouraging peer %s!\n", pnode.addr.ToString());
             if (m_banman) {
-                m_banman->Ban(pnode.addr, BanReasonNodeMisbehaving);
+                m_banman->Discourage(pnode.addr);
             }
             connman->DisconnectNode(pnode.addr);
         }
@@ -3679,7 +3681,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
     }
 
     LOCK(cs_main);
-    CheckIfBanned(*pfrom);
+    MaybeDiscourageAndDisconnect(*pfrom);
 
     return fMoreWork;
 }
@@ -3882,7 +3884,7 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
         if (!lockMain)
             return true;
 
-        if (CheckIfBanned(*pto)) return true;
+        if (MaybeDiscourageAndDisconnect(*pto)) return true;
 
         CNodeState &state = *State(pto->GetId());
 
diff --git a/src/net_processing.h b/src/net_processing.h
index 19beca0cc4..eadf29e59f 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -31,7 +31,7 @@ private:
     ChainstateManager& m_chainman;
     CTxMemPool& m_mempool;
 
-    bool CheckIfBanned(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+    bool MaybeDiscourageAndDisconnect(CNode& pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 public:
     PeerLogicValidation(CConnman* connman, BanMan* banman, CScheduler& scheduler, ChainstateManager& chainman, CTxMemPool& pool);
diff --git a/src/netaddress.h b/src/netaddress.h
index e640c07d32..552abc4119 100644
--- a/src/netaddress.h
+++ b/src/netaddress.h
@@ -90,6 +90,7 @@ class CNetAddr
         uint32_t GetMappedAS(const std::vector<bool> &asmap) const;
 
         std::vector<unsigned char> GetGroup(const std::vector<bool> &asmap) const;
+        std::vector<unsigned char> GetAddrBytes() const { return {std::begin(ip), std::end(ip)}; }
         int GetReachabilityFrom(const CNetAddr *paddrPartner = nullptr) const;
 
         explicit CNetAddr(const struct in6_addr& pipv6Addr, const uint32_t scope = 0);
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index dafd517ca8..ab2c37e6f8 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1218,7 +1218,7 @@ void RPCConsole::banSelectedNode(int bantime)
         // Find possible nodes, ban it and clear the selected node
         const CNodeCombinedStats *stats = clientModel->getPeerTableModel()->getNodeStats(detailNodeRow);
         if (stats) {
-            m_node.ban(stats->nodeStats.addr, BanReasonManuallyAdded, bantime);
+            m_node.ban(stats->nodeStats.addr, bantime);
             m_node.disconnectByAddress(stats->nodeStats.addr);
         }
     }
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index df1e0fe623..393442e946 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -614,12 +614,12 @@ static UniValue setban(const JSONRPCRequest& request)
             absolute = true;
 
         if (isSubnet) {
-            node.banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute);
+            node.banman->Ban(subNet, banTime, absolute);
             if (node.connman) {
                 node.connman->DisconnectNode(subNet);
             }
         } else {
-            node.banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
+            node.banman->Ban(netAddr, banTime, absolute);
             if (node.connman) {
                 node.connman->DisconnectNode(netAddr);
             }
@@ -628,7 +628,7 @@ static UniValue setban(const JSONRPCRequest& request)
     else if(strCommand == "remove")
     {
         if (!( isSubnet ? node.banman->Unban(subNet) : node.banman->Unban(netAddr) )) {
-            throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned.");
+            throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously manually banned.");
         }
     }
     return NullUniValue;
@@ -637,7 +637,7 @@ static UniValue setban(const JSONRPCRequest& request)
 static UniValue listbanned(const JSONRPCRequest& request)
 {
             RPCHelpMan{"listbanned",
-                "\nList all banned IPs/Subnets.\n",
+                "\nList all manually banned IPs/Subnets.\n",
                 {},
         RPCResult{RPCResult::Type::ARR, "", "",
             {
@@ -646,7 +646,6 @@ static UniValue listbanned(const JSONRPCRequest& request)
                         {RPCResult::Type::STR, "address", ""},
                         {RPCResult::Type::NUM_TIME, "banned_until", ""},
                         {RPCResult::Type::NUM_TIME, "ban_created", ""},
-                        {RPCResult::Type::STR, "ban_reason", ""},
                     }},
             }},
                 RPCExamples{
@@ -671,7 +670,6 @@ static UniValue listbanned(const JSONRPCRequest& request)
         rec.pushKV("address", entry.first.ToString());
         rec.pushKV("banned_until", banEntry.nBanUntil);
         rec.pushKV("ban_created", banEntry.nCreateTime);
-        rec.pushKV("ban_reason", banEntry.banReasonToString());
 
         bannedAddresses.push_back(rec);
     }
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index 348b170536..1fe01fae04 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -238,8 +238,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
         LOCK2(cs_main, dummyNode1.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
     }
-    BOOST_CHECK(banman->IsBanned(addr1));
-    BOOST_CHECK(!banman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
+    BOOST_CHECK(banman->IsDiscouraged(addr1));
+    BOOST_CHECK(!banman->IsDiscouraged(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
 
     CAddress addr2(ip(0xa0b0c002), NODE_NONE);
     CNode dummyNode2(id++, NODE_NETWORK, 0, INVALID_SOCKET, addr2, 1, 1, CAddress(), "", true);
@@ -255,8 +255,8 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
         LOCK2(cs_main, dummyNode2.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
     }
-    BOOST_CHECK(!banman->IsBanned(addr2)); // 2 not banned yet...
-    BOOST_CHECK(banman->IsBanned(addr1));  // ... but 1 still should be
+    BOOST_CHECK(!banman->IsDiscouraged(addr2)); // 2 not banned yet...
+    BOOST_CHECK(banman->IsDiscouraged(addr1));  // ... but 1 still should be
     {
         LOCK(cs_main);
         Misbehaving(dummyNode2.GetId(), 50);
@@ -265,7 +265,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
         LOCK2(cs_main, dummyNode2.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode2));
     }
-    BOOST_CHECK(banman->IsBanned(addr2));
+    BOOST_CHECK(banman->IsDiscouraged(addr2));
 
     bool dummy;
     peerLogic->FinalizeNode(dummyNode1.GetId(), dummy);
@@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
         LOCK2(cs_main, dummyNode1.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
     }
-    BOOST_CHECK(!banman->IsBanned(addr1));
+    BOOST_CHECK(!banman->IsDiscouraged(addr1));
     {
         LOCK(cs_main);
         Misbehaving(dummyNode1.GetId(), 10);
@@ -303,7 +303,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
         LOCK2(cs_main, dummyNode1.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
     }
-    BOOST_CHECK(!banman->IsBanned(addr1));
+    BOOST_CHECK(!banman->IsDiscouraged(addr1));
     {
         LOCK(cs_main);
         Misbehaving(dummyNode1.GetId(), 1);
@@ -312,7 +312,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
         LOCK2(cs_main, dummyNode1.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode1));
     }
-    BOOST_CHECK(banman->IsBanned(addr1));
+    BOOST_CHECK(banman->IsDiscouraged(addr1));
     gArgs.ForceSetArg("-banscore", ToString(DEFAULT_BANSCORE_THRESHOLD));
 
     bool dummy;
@@ -344,13 +344,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
         LOCK2(cs_main, dummyNode.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
     }
-    BOOST_CHECK(banman->IsBanned(addr));
-
-    SetMockTime(nStartTime+60*60);
-    BOOST_CHECK(banman->IsBanned(addr));
-
-    SetMockTime(nStartTime+60*60*24+1);
-    BOOST_CHECK(!banman->IsBanned(addr));
+    BOOST_CHECK(banman->IsDiscouraged(addr));
 
     bool dummy;
     peerLogic->FinalizeNode(dummyNode.GetId(), dummy);
diff --git a/src/test/fuzz/addrdb.cpp b/src/test/fuzz/addrdb.cpp
index 524cea83fe..ad6461650f 100644
--- a/src/test/fuzz/addrdb.cpp
+++ b/src/test/fuzz/addrdb.cpp
@@ -18,18 +18,11 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
 
     const CBanEntry ban_entry = [&] {
-        switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 3)) {
+        switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 2)) {
         case 0:
             return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>()};
             break;
-        case 1:
-            return CBanEntry{fuzzed_data_provider.ConsumeIntegral<int64_t>(), fuzzed_data_provider.PickValueInArray<BanReason>({
-                                                                                  BanReason::BanReasonUnknown,
-                                                                                  BanReason::BanReasonNodeMisbehaving,
-                                                                                  BanReason::BanReasonManuallyAdded,
-                                                                              })};
-            break;
-        case 2: {
+        case 1: {
             const std::optional<CBanEntry> ban_entry = ConsumeDeserializable<CBanEntry>(fuzzed_data_provider);
             if (ban_entry) {
                 return *ban_entry;
@@ -39,5 +32,4 @@ void test_one_input(const std::vector<uint8_t>& buffer)
         }
         return CBanEntry{};
     }();
-    assert(!ban_entry.banReasonToString().empty());
 }