From 0596a6eeb5ddb84c7095aed71bb7e6645c275f07 Mon Sep 17 00:00:00 2001
From: MIZUTA Takeshi <mizuta.takeshi@fujitsu.com>
Date: Mon, 8 Jun 2020 16:37:59 +0900
Subject: [PATCH 01/15] util: Don't reference errno when pthread fails.

Pthread library does not set errno.
Pthread library's errno is returned by return value.

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

Github-Pull: #19194
Rebased-From: cb38b069b0f41b1a26264784b1c1303c8ac6ab08
---
 src/util/system.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/util/system.cpp b/src/util/system.cpp
index b0a538b5277..b6a7f3926d5 100644
--- a/src/util/system.cpp
+++ b/src/util/system.cpp
@@ -1155,8 +1155,9 @@ void ScheduleBatchPriority()
 {
 #ifdef SCHED_BATCH
     const static sched_param param{};
-    if (pthread_setschedparam(pthread_self(), SCHED_BATCH, &param) != 0) {
-        LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(errno));
+    const int rc = pthread_setschedparam(pthread_self(), SCHED_BATCH, &param);
+    if (rc != 0) {
+        LogPrintf("Failed to pthread_setschedparam: %s\n", strerror(rc));
     }
 #endif
 }

From c219d21634b3eff537471c4d52e631679965d8d8 Mon Sep 17 00:00:00 2001
From: sachinkm77 <sachinkm77>
Date: Wed, 3 Jun 2020 04:06:36 -0400
Subject: [PATCH 02/15] build: improved output of configure for build OS

Github-Pull: #19152
Rebased-From: 0fef60c63d6d2f4df8e698936221e2330ef3a244
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index c48a9a02a93..e61b65f7fa5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1668,7 +1668,7 @@ echo "  gprof enabled = $enable_gprof"
 echo "  werror        = $enable_werror"
 echo
 echo "  target os     = $TARGET_OS"
-echo "  build os      = $BUILD_OS"
+echo "  build os      = $build_os"
 echo
 echo "  CC            = $CC"
 echo "  CFLAGS        = $CFLAGS"

From 5c7151a60468bf3c603ffe0490fbbe95736fdd69 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Sat, 23 May 2020 18:22:29 +0800
Subject: [PATCH 03/15] gui: update Qt base translations for macOS release

These haven't been updated since their addition, so this updates the list that
controls which qt base translations are bundled with the macOS binary, to all the
languages that are available with qt 5.9.8.

This could probably be improved in some way, however qt updates are infrequent,
and I didn't want to spend any more time looking at this. Also given that no-one
seems to have noticed and/or reported this it wouldn't seem high-priority.

Could be backported to 0.20.1.

Github-Pull: #19059
Rebased-From: 69bfcac27a83440092bc6e61904ded910ed4baf4
---
 Makefile.am            | 2 +-
 depends/packages/qt.mk | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index c35f5080aa2..70d4f1253a1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -41,7 +41,7 @@ OSX_DEPLOY_SCRIPT=$(top_srcdir)/contrib/macdeploy/macdeployqtplus
 OSX_FANCY_PLIST=$(top_srcdir)/contrib/macdeploy/fancy.plist
 OSX_INSTALLER_ICONS=$(top_srcdir)/src/qt/res/icons/bitcoin.icns
 OSX_PLIST=$(top_builddir)/share/qt/Info.plist #not installed
-OSX_QT_TRANSLATIONS = da,de,es,hu,ru,uk,zh_CN,zh_TW
+OSX_QT_TRANSLATIONS = ar,bg,ca,cs,da,de,es,fa,fi,fr,gd,gl,he,hu,it,ja,ko,lt,lv,pl,pt,ru,sk,sl,sv,uk,zh_CN,zh_TW
 
 DIST_CONTRIB = \
 	       $(top_srcdir)/contrib/linearize/linearize-data.py \
diff --git a/depends/packages/qt.mk b/depends/packages/qt.mk
index 366b1d0c423..631851855a0 100644
--- a/depends/packages/qt.mk
+++ b/depends/packages/qt.mk
@@ -10,6 +10,7 @@ $(package)_build_subdir=qtbase
 $(package)_qt_libs=corelib network widgets gui plugins testlib
 $(package)_patches=fix_qt_pkgconfig.patch mac-qmake.conf fix_configure_mac.patch fix_no_printer.patch fix_rcc_determinism.patch fix_riscv64_arch.patch xkb-default.patch no-xlib.patch fix_android_qmake_conf.patch fix_android_jni_static.patch
 
+# Update OSX_QT_TRANSLATIONS when this is updated
 $(package)_qttranslations_file_name=qttranslations-$($(package)_suffix)
 $(package)_qttranslations_sha256_hash=fb5a47799754af73d3bf501fe513342cfe2fc37f64e80df5533f6110e804220c
 

From febebc4ea68104bba9ad2cf4468fc50e6136f803 Mon Sep 17 00:00:00 2001
From: Samuel Dobson <dobsonsa68@gmail.com>
Date: Sun, 19 Apr 2020 11:07:14 +1200
Subject: [PATCH 04/15] Fix WSL file locking by using flock instead of fcntl

Co-authored-by: sipa <pieter@wuille.net>

Github-Pull: #18700
Rebased-From: e8fa0a3d2025509fcddc59fc618e91371542cf87
---
 src/fs.cpp | 37 +++++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/src/fs.cpp b/src/fs.cpp
index 066c6c10d30..e68c97b3caf 100644
--- a/src/fs.cpp
+++ b/src/fs.cpp
@@ -6,6 +6,9 @@
 
 #ifndef WIN32
 #include <fcntl.h>
+#include <string>
+#include <sys/file.h>
+#include <sys/utsname.h>
 #else
 #ifndef NOMINMAX
 #define NOMINMAX
@@ -47,20 +50,38 @@ FileLock::~FileLock()
     }
 }
 
+static bool IsWSL()
+{
+    struct utsname uname_data;
+    return uname(&uname_data) == 0 && std::string(uname_data.version).find("Microsoft") != std::string::npos;
+}
+
 bool FileLock::TryLock()
 {
     if (fd == -1) {
         return false;
     }
-    struct flock lock;
-    lock.l_type = F_WRLCK;
-    lock.l_whence = SEEK_SET;
-    lock.l_start = 0;
-    lock.l_len = 0;
-    if (fcntl(fd, F_SETLK, &lock) == -1) {
-        reason = GetErrorReason();
-        return false;
+
+    // Exclusive file locking is broken on WSL using fcntl (issue #18622)
+    // This workaround can be removed once the bug on WSL is fixed
+    static const bool is_wsl = IsWSL();
+    if (is_wsl) {
+        if (flock(fd, LOCK_EX | LOCK_NB) == -1) {
+            reason = GetErrorReason();
+            return false;
+        }
+    } else {
+        struct flock lock;
+        lock.l_type = F_WRLCK;
+        lock.l_whence = SEEK_SET;
+        lock.l_start = 0;
+        lock.l_len = 0;
+        if (fcntl(fd, F_SETLK, &lock) == -1) {
+            reason = GetErrorReason();
+            return false;
+        }
     }
+
     return true;
 }
 #else

From 654420d6dfb455ca4030055881db4e3aa9ec6e8b Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Fri, 15 May 2020 09:23:55 -0400
Subject: [PATCH 05/15] wallet: Minimal fix to restore conflicted transaction
 notifications

This fix is a based on the fix by Antoine Riard <ariard@student.42.fr> in
https://github.com/bitcoin/bitcoin/pull/18600.

Unlike that PR, which implements some new behavior, this just restores previous
wallet notification and status behavior for transactions removed from the
mempool because they conflict with transactions in a block. The behavior was
accidentally changed in two `CWallet::BlockConnected` updates:
a31be09bfd77eed497a8e251d31358e16e2f2eb1 and
7e89994133725125dddbfa8d45484e3b9ed51c6e from
https://github.com/bitcoin/bitcoin/pull/16624, causing issue
https://github.com/bitcoin/bitcoin/issues/18325.

The change here could be improved and replaced with a more comprehensive
cleanup, so it includes a detailed comment explaining future considerations.

Fixes #18325

Co-authored-by: Antoine Riard <ariard@student.42.fr>

Github-Pull: #18982
Rebased-From: b604c5c8b5892842f13dee89ae31812a28ab25d1
---
 doc/release-notes-18982.md               |  8 ++++++
 src/interfaces/chain.cpp                 |  4 +--
 src/interfaces/chain.h                   |  3 +-
 src/txmempool.cpp                        |  2 +-
 src/validationinterface.cpp              |  6 ++--
 src/validationinterface.h                |  5 ++--
 src/wallet/wallet.cpp                    | 35 ++++++++++++++++++++++--
 src/wallet/wallet.h                      |  2 +-
 test/functional/feature_notifications.py |  9 ++----
 9 files changed, 55 insertions(+), 19 deletions(-)
 create mode 100644 doc/release-notes-18982.md

diff --git a/doc/release-notes-18982.md b/doc/release-notes-18982.md
new file mode 100644
index 00000000000..d1b8528d137
--- /dev/null
+++ b/doc/release-notes-18982.md
@@ -0,0 +1,8 @@
+Notification changes
+--------------------
+
+`-walletnotify` notifications are now sent for wallet transactions that are
+removed from the mempool because they conflict with a new block. These
+notifications were sent previously before the v0.19 release, but had been
+broken since that release (bug
+[#18325](https://github.com/bitcoin/bitcoin/issues/18325)).
diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
index 0b3cd08e222..7c58273f82c 100644
--- a/src/interfaces/chain.cpp
+++ b/src/interfaces/chain.cpp
@@ -158,9 +158,9 @@ public:
     {
         m_notifications->transactionAddedToMempool(tx);
     }
-    void TransactionRemovedFromMempool(const CTransactionRef& tx) override
+    void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override
     {
-        m_notifications->transactionRemovedFromMempool(tx);
+        m_notifications->transactionRemovedFromMempool(tx, reason);
     }
     void BlockConnected(const std::shared_ptr<const CBlock>& block, const CBlockIndex* index) override
     {
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index e1bc9bbbf3a..538aaae23be 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -20,6 +20,7 @@ class CRPCCommand;
 class CScheduler;
 class Coin;
 class uint256;
+enum class MemPoolRemovalReason;
 enum class RBFTransactionState;
 struct CBlockLocator;
 struct FeeCalculation;
@@ -221,7 +222,7 @@ public:
     public:
         virtual ~Notifications() {}
         virtual void transactionAddedToMempool(const CTransactionRef& tx) {}
-        virtual void transactionRemovedFromMempool(const CTransactionRef& ptx) {}
+        virtual void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}
         virtual void blockConnected(const CBlock& block, int height) {}
         virtual void blockDisconnected(const CBlock& block, int height) {}
         virtual void updatedBlockTip() {}
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 47b0d39ea4c..c14417a8473 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -410,7 +410,7 @@ void CTxMemPool::removeUnchecked(txiter it, MemPoolRemovalReason reason)
         // for any reason except being included in a block. Clients interested
         // in transactions included in blocks can subscribe to the BlockConnected
         // notification.
-        GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx());
+        GetMainSignals().TransactionRemovedFromMempool(it->GetSharedTx(), reason);
     }
 
     const uint256 hash = it->GetTx().GetHash();
diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 11000774c09..81a8268684f 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -199,9 +199,9 @@ void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) {
                           ptx->GetWitnessHash().ToString());
 }
 
-void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx) {
-    auto event = [ptx, this] {
-        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(ptx); });
+void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
+    auto event = [ptx, reason, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(ptx, reason); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
                           ptx->GetHash().ToString(),
diff --git a/src/validationinterface.h b/src/validationinterface.h
index cb0204a555d..2c03d8cd850 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -21,6 +21,7 @@ class CConnman;
 class CValidationInterface;
 class uint256;
 class CScheduler;
+enum class MemPoolRemovalReason;
 
 // These functions dispatch to one or all registered wallets
 
@@ -129,7 +130,7 @@ protected:
      *
      * Called on a background thread.
      */
-    virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx) {}
+    virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {}
     /**
      * Notifies listeners of a block being connected.
      * Provides a vector of transactions evicted from the mempool as a result.
@@ -197,7 +198,7 @@ public:
 
     void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
     void TransactionAddedToMempool(const CTransactionRef &);
-    void TransactionRemovedFromMempool(const CTransactionRef &);
+    void TransactionRemovedFromMempool(const CTransactionRef &, MemPoolRemovalReason);
     void BlockConnected(const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
     void ChainStateFlushed(const CBlockLocator &);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 45f5542cadd..80b4b1f4bf8 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -21,6 +21,7 @@
 #include <script/descriptor.h>
 #include <script/script.h>
 #include <script/signingprovider.h>
+#include <txmempool.h>
 #include <util/bip32.h>
 #include <util/error.h>
 #include <util/fees.h>
@@ -1105,12 +1106,42 @@ void CWallet::transactionAddedToMempool(const CTransactionRef& ptx) {
     }
 }
 
-void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx) {
+void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
     LOCK(cs_wallet);
     auto it = mapWallet.find(ptx->GetHash());
     if (it != mapWallet.end()) {
         it->second.fInMempool = false;
     }
+    // Handle transactions that were removed from the mempool because they
+    // conflict with transactions in a newly connected block.
+    if (reason == MemPoolRemovalReason::CONFLICT) {
+        // Call SyncNotifications, so external -walletnotify notifications will
+        // be triggered for these transactions. Set Status::UNCONFIRMED instead
+        // of Status::CONFLICTED for a few reasons:
+        //
+        // 1. The transactionRemovedFromMempool callback does not currently
+        //    provide the conflicting block's hash and height, and for backwards
+        //    compatibility reasons it may not be not safe to store conflicted
+        //    wallet transactions with a null block hash. See
+        //    https://github.com/bitcoin/bitcoin/pull/18600#discussion_r420195993.
+        // 2. For most of these transactions, the wallet's internal conflict
+        //    detection in the blockConnected handler will subsequently call
+        //    MarkConflicted and update them with CONFLICTED status anyway. This
+        //    applies to any wallet transaction that has inputs spent in the
+        //    block, or that has ancestors in the wallet with inputs spent by
+        //    the block.
+        // 3. Longstanding behavior since the sync implementation in
+        //    https://github.com/bitcoin/bitcoin/pull/9371 and the prior sync
+        //    implementation before that was to mark these transactions
+        //    unconfirmed rather than conflicted.
+        //
+        // Nothing described above should be seen as an unchangeable requirement
+        // when improving this code in the future. The wallet's heuristics for
+        // distinguishing between conflicted and unconfirmed transactions are
+        // imperfect, and could be improved in general, see
+        // https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking
+        SyncTransaction(ptx, {CWalletTx::Status::UNCONFIRMED, /* block height  */ 0, /* block hash */ {}, /* index */ 0});
+    }
 }
 
 void CWallet::blockConnected(const CBlock& block, int height)
@@ -1124,7 +1155,7 @@ void CWallet::blockConnected(const CBlock& block, int height)
     for (size_t index = 0; index < block.vtx.size(); index++) {
         CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index);
         SyncTransaction(block.vtx[index], confirm);
-        transactionRemovedFromMempool(block.vtx[index]);
+        transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK);
     }
 }
 
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 7446a4889a4..20f0306c63b 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -910,7 +910,7 @@ public:
         uint256 last_failed_block;
     };
     ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate);
-    void transactionRemovedFromMempool(const CTransactionRef &ptx) override;
+    void transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
     void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void ResendWalletTransactions();
     struct Balance {
diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py
index 47200b6cc6f..fb0c7ceed4c 100755
--- a/test/functional/feature_notifications.py
+++ b/test/functional/feature_notifications.py
@@ -125,12 +125,7 @@ class NotificationsTest(BitcoinTestFramework):
 
             # Bump tx2 as bump2 and generate a block on node 0 while
             # disconnected, then reconnect and check for notifications on node 1
-            # about newly confirmed bump2 and newly conflicted tx2. Currently
-            # only the bump2 notification is sent. Ideally, notifications would
-            # be sent both for bump2 and tx2, which was the previous behavior
-            # before being broken by an accidental change in PR
-            # https://github.com/bitcoin/bitcoin/pull/16624. The bug is reported
-            # in issue https://github.com/bitcoin/bitcoin/issues/18325.
+            # about newly confirmed bump2 and newly conflicted tx2.
             disconnect_nodes(self.nodes[0], 1)
             bump2 = self.nodes[0].bumpfee(tx2)["txid"]
             self.nodes[0].generatetoaddress(1, ADDRESS_BCRT1_UNSPENDABLE)
@@ -138,7 +133,7 @@ class NotificationsTest(BitcoinTestFramework):
             assert_equal(tx2 in self.nodes[1].getrawmempool(), True)
             connect_nodes(self.nodes[0], 1)
             self.sync_blocks()
-            self.expect_wallet_notify([bump2])
+            self.expect_wallet_notify([bump2, tx2])
             assert_equal(self.nodes[1].gettransaction(bump2)["confirmations"], 1)
 
         # TODO: add test for `-alertnotify` large fork notifications

From 27786d072dbcf65cba9110f1dd171f94b6abc107 Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Fri, 22 May 2020 16:30:07 -0400
Subject: [PATCH 06/15] trivial: Suggested cleanups to surrounding code

https://github.com/bitcoin/bitcoin/pull/18982#pullrequestreview-416974841

Github-Pull: #18982
Rebased-From: 7eaf86d3bfc83f2beb3ef449707d5156853126fb
---
 src/validationinterface.cpp | 20 ++++++++++----------
 src/validationinterface.h   |  8 ++++----
 src/wallet/wallet.cpp       | 23 +++++++++--------------
 src/wallet/wallet.h         |  2 +-
 4 files changed, 24 insertions(+), 29 deletions(-)

diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp
index 81a8268684f..6a2c151806c 100644
--- a/src/validationinterface.cpp
+++ b/src/validationinterface.cpp
@@ -190,22 +190,22 @@ void CMainSignals::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockInd
                           fInitialDownload);
 }
 
-void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) {
-    auto event = [ptx, this] {
-        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(ptx); });
+void CMainSignals::TransactionAddedToMempool(const CTransactionRef& tx) {
+    auto event = [tx, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionAddedToMempool(tx); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
-                          ptx->GetHash().ToString(),
-                          ptx->GetWitnessHash().ToString());
+                          tx->GetHash().ToString(),
+                          tx->GetWitnessHash().ToString());
 }
 
-void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
-    auto event = [ptx, reason, this] {
-        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(ptx, reason); });
+void CMainSignals::TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {
+    auto event = [tx, reason, this] {
+        m_internals->Iterate([&](CValidationInterface& callbacks) { callbacks.TransactionRemovedFromMempool(tx, reason); });
     };
     ENQUEUE_AND_LOG_EVENT(event, "%s: txid=%s wtxid=%s", __func__,
-                          ptx->GetHash().ToString(),
-                          ptx->GetWitnessHash().ToString());
+                          tx->GetHash().ToString(),
+                          tx->GetWitnessHash().ToString());
 }
 
 void CMainSignals::BlockConnected(const std::shared_ptr<const CBlock> &pblock, const CBlockIndex *pindex) {
diff --git a/src/validationinterface.h b/src/validationinterface.h
index 2c03d8cd850..3ab82306a6f 100644
--- a/src/validationinterface.h
+++ b/src/validationinterface.h
@@ -97,7 +97,7 @@ protected:
      *
      * Called on a background thread.
      */
-    virtual void TransactionAddedToMempool(const CTransactionRef &ptxn) {}
+    virtual void TransactionAddedToMempool(const CTransactionRef& tx) {}
     /**
      * Notifies listeners of a transaction leaving mempool.
      *
@@ -130,7 +130,7 @@ protected:
      *
      * Called on a background thread.
      */
-    virtual void TransactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {}
+    virtual void TransactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {}
     /**
      * Notifies listeners of a block being connected.
      * Provides a vector of transactions evicted from the mempool as a result.
@@ -197,8 +197,8 @@ public:
 
 
     void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload);
-    void TransactionAddedToMempool(const CTransactionRef &);
-    void TransactionRemovedFromMempool(const CTransactionRef &, MemPoolRemovalReason);
+    void TransactionAddedToMempool(const CTransactionRef&);
+    void TransactionRemovedFromMempool(const CTransactionRef&, MemPoolRemovalReason);
     void BlockConnected(const std::shared_ptr<const CBlock> &, const CBlockIndex *pindex);
     void BlockDisconnected(const std::shared_ptr<const CBlock> &, const CBlockIndex* pindex);
     void ChainStateFlushed(const CBlockLocator &);
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 80b4b1f4bf8..17eb02b3d52 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -1094,21 +1094,19 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio
     MarkInputsDirty(ptx);
 }
 
-void CWallet::transactionAddedToMempool(const CTransactionRef& ptx) {
-    auto locked_chain = chain().lock();
+void CWallet::transactionAddedToMempool(const CTransactionRef& tx) {
     LOCK(cs_wallet);
-    CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
-    SyncTransaction(ptx, confirm);
+    SyncTransaction(tx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0});
 
-    auto it = mapWallet.find(ptx->GetHash());
+    auto it = mapWallet.find(tx->GetHash());
     if (it != mapWallet.end()) {
         it->second.fInMempool = true;
     }
 }
 
-void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) {
+void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) {
     LOCK(cs_wallet);
-    auto it = mapWallet.find(ptx->GetHash());
+    auto it = mapWallet.find(tx->GetHash());
     if (it != mapWallet.end()) {
         it->second.fInMempool = false;
     }
@@ -1140,7 +1138,7 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolR
         // distinguishing between conflicted and unconfirmed transactions are
         // imperfect, and could be improved in general, see
         // https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking
-        SyncTransaction(ptx, {CWalletTx::Status::UNCONFIRMED, /* block height  */ 0, /* block hash */ {}, /* index */ 0});
+        SyncTransaction(tx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0});
     }
 }
 
@@ -1153,8 +1151,7 @@ void CWallet::blockConnected(const CBlock& block, int height)
     m_last_block_processed_height = height;
     m_last_block_processed = block_hash;
     for (size_t index = 0; index < block.vtx.size(); index++) {
-        CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index);
-        SyncTransaction(block.vtx[index], confirm);
+        SyncTransaction(block.vtx[index], {CWalletTx::Status::CONFIRMED, height, block_hash, (int)index});
         transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK);
     }
 }
@@ -1171,8 +1168,7 @@ void CWallet::blockDisconnected(const CBlock& block, int height)
     m_last_block_processed_height = height - 1;
     m_last_block_processed = block.hashPrevBlock;
     for (const CTransactionRef& ptx : block.vtx) {
-        CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
-        SyncTransaction(ptx, confirm);
+        SyncTransaction(ptx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0});
     }
 }
 
@@ -1721,8 +1717,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
                 break;
             }
             for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
-                CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, *block_height, block_hash, posInBlock);
-                SyncTransaction(block.vtx[posInBlock], confirm, fUpdate);
+                SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, *block_height, block_hash, (int)posInBlock}, fUpdate);
             }
             // scan succeeded, record block as most recent successfully scanned
             result.last_scanned_block = block_hash;
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index 20f0306c63b..9ca8e9e3091 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -910,7 +910,7 @@ public:
         uint256 last_failed_block;
     };
     ScanResult ScanForWalletTransactions(const uint256& first_block, const uint256& last_block, const WalletRescanReserver& reserver, bool fUpdate);
-    void transactionRemovedFromMempool(const CTransactionRef &ptx, MemPoolRemovalReason reason) override;
+    void transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason) override;
     void ReacceptWalletTransactions() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
     void ResendWalletTransactions();
     struct Balance {

From 68e0e6f85247c8b9e0b0f039bc31c434632c2982 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Thu, 4 Jun 2020 23:43:25 -0400
Subject: [PATCH 07/15] rpc: show both UTXOs in decodepsbt

Github-Pull: #19215
Rebased-From: 72f6bec1da198764d4648a10a61c485e7ab65e9e
---
 src/rpc/rawtransaction.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 3c0b0604398..746900fdab1 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -1116,6 +1116,7 @@ UniValue decodepsbt(const JSONRPCRequest& request)
         const PSBTInput& input = psbtx.inputs[i];
         UniValue in(UniValue::VOBJ);
         // UTXOs
+        bool have_a_utxo = false;
         if (!input.witness_utxo.IsNull()) {
             const CTxOut& txout = input.witness_utxo;
 
@@ -1133,7 +1134,9 @@ UniValue decodepsbt(const JSONRPCRequest& request)
             ScriptToUniv(txout.scriptPubKey, o, true);
             out.pushKV("scriptPubKey", o);
             in.pushKV("witness_utxo", out);
-        } else if (input.non_witness_utxo) {
+            have_a_utxo = true;
+        }
+        if (input.non_witness_utxo) {
             UniValue non_wit(UniValue::VOBJ);
             TxToUniv(*input.non_witness_utxo, uint256(), non_wit, false);
             in.pushKV("non_witness_utxo", non_wit);
@@ -1144,7 +1147,9 @@ UniValue decodepsbt(const JSONRPCRequest& request)
                 // Hack to just not show fee later
                 have_all_utxos = false;
             }
-        } else {
+            have_a_utxo = true;
+        }
+        if (!have_a_utxo) {
             have_all_utxos = false;
         }
 

From ed5ec3080419445c0af7410c0af7a421704841a6 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Thu, 4 Jun 2020 23:43:39 -0400
Subject: [PATCH 08/15] psbt: Allow both non_witness_utxo and witness_utxo

Github-Pull: #19215
Rebased-From: 5279d8bc07d601fe6a67ad665fbc7591fe73c7de
---
 src/psbt.cpp                   | 29 -----------------------------
 src/psbt.h                     |  7 -------
 src/test/fuzz/psbt.cpp         |  2 --
 src/wallet/scriptpubkeyman.cpp |  5 -----
 src/wallet/wallet.cpp          |  5 -----
 5 files changed, 48 deletions(-)

diff --git a/src/psbt.cpp b/src/psbt.cpp
index e6b6285652b..ff17981fd8c 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -35,14 +35,6 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
     return true;
 }
 
-bool PartiallySignedTransaction::IsSane() const
-{
-    for (PSBTInput input : inputs) {
-        if (!input.IsSane()) return false;
-    }
-    return true;
-}
-
 bool PartiallySignedTransaction::AddInput(const CTxIn& txin, PSBTInput& psbtin)
 {
     if (std::find(tx->vin.begin(), tx->vin.end(), txin) != tx->vin.end()) {
@@ -158,18 +150,6 @@ void PSBTInput::Merge(const PSBTInput& input)
     if (final_script_witness.IsNull() && !input.final_script_witness.IsNull()) final_script_witness = input.final_script_witness;
 }
 
-bool PSBTInput::IsSane() const
-{
-    // Cannot have both witness and non-witness utxos
-    if (!witness_utxo.IsNull() && non_witness_utxo) return false;
-
-    // If we have a witness_script or a scriptWitness, we must also have a witness utxo
-    if (!witness_script.empty() && witness_utxo.IsNull()) return false;
-    if (!final_script_witness.IsNull() && witness_utxo.IsNull()) return false;
-
-    return true;
-}
-
 void PSBTOutput::FillSignatureData(SignatureData& sigdata) const
 {
     if (!redeem_script.empty()) {
@@ -250,11 +230,6 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction&
     bool require_witness_sig = false;
     CTxOut utxo;
 
-    // Verify input sanity, which checks that at most one of witness or non-witness utxos is provided.
-    if (!input.IsSane()) {
-        return false;
-    }
-
     if (input.non_witness_utxo) {
         // If we're taking our information from a non-witness UTXO, verify that it matches the prevout.
         COutPoint prevout = tx.vin[index].prevout;
@@ -345,10 +320,6 @@ TransactionError CombinePSBTs(PartiallySignedTransaction& out, const std::vector
             return TransactionError::PSBT_MISMATCH;
         }
     }
-    if (!out.IsSane()) {
-        return TransactionError::INVALID_PSBT;
-    }
-
     return TransactionError::OK;
 }
 
diff --git a/src/psbt.h b/src/psbt.h
index dfba2619617..1f7f84af6b2 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -58,7 +58,6 @@ struct PSBTInput
     void FillSignatureData(SignatureData& sigdata) const;
     void FromSignatureData(const SignatureData& sigdata);
     void Merge(const PSBTInput& input);
-    bool IsSane() const;
     PSBTInput() {}
 
     template <typename Stream>
@@ -280,7 +279,6 @@ struct PSBTOutput
     void FillSignatureData(SignatureData& sigdata) const;
     void FromSignatureData(const SignatureData& sigdata);
     void Merge(const PSBTOutput& output);
-    bool IsSane() const;
     PSBTOutput() {}
 
     template <typename Stream>
@@ -397,7 +395,6 @@ struct PartiallySignedTransaction
     /** Merge psbt into this. The two psbts must have the same underlying CTransaction (i.e. the
       * same actual Bitcoin transaction.) Returns true if the merge succeeded, false otherwise. */
     NODISCARD bool Merge(const PartiallySignedTransaction& psbt);
-    bool IsSane() const;
     bool AddInput(const CTxIn& txin, PSBTInput& psbtin);
     bool AddOutput(const CTxOut& txout, const PSBTOutput& psbtout);
     PartiallySignedTransaction() {}
@@ -547,10 +544,6 @@ struct PartiallySignedTransaction
         if (outputs.size() != tx->vout.size()) {
             throw std::ios_base::failure("Outputs provided does not match the number of outputs in transaction.");
         }
-        // Sanity check
-        if (!IsSane()) {
-            throw std::ios_base::failure("PSBT is not sane.");
-        }
     }
 
     template <typename Stream>
diff --git a/src/test/fuzz/psbt.cpp b/src/test/fuzz/psbt.cpp
index ca3e0b85869..fe8bd208167 100644
--- a/src/test/fuzz/psbt.cpp
+++ b/src/test/fuzz/psbt.cpp
@@ -39,7 +39,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     }
 
     (void)psbt.IsNull();
-    (void)psbt.IsSane();
 
     Optional<CMutableTransaction> tx = psbt.tx;
     if (tx) {
@@ -50,7 +49,6 @@ void test_one_input(const std::vector<uint8_t>& buffer)
     for (const PSBTInput& input : psbt.inputs) {
         (void)PSBTInputSigned(input);
         (void)input.IsNull();
-        (void)input.IsSane();
     }
 
     for (const PSBTOutput& output : psbt.outputs) {
diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp
index b96cb0aa1a3..8863e394ebd 100644
--- a/src/wallet/scriptpubkeyman.cpp
+++ b/src/wallet/scriptpubkeyman.cpp
@@ -536,11 +536,6 @@ TransactionError LegacyScriptPubKeyMan::FillPSBT(PartiallySignedTransaction& psb
             continue;
         }
 
-        // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness.
-        if (!input.IsSane()) {
-            return TransactionError::INVALID_PSBT;
-        }
-
         // Get the Sighash type
         if (sign && input.sighash_type > 0 && input.sighash_type != sighash_type) {
             return TransactionError::SIGHASH_MISMATCH;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 17eb02b3d52..a6962ba428b 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2517,11 +2517,6 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp
             continue;
         }
 
-        // Verify input looks sane. This will check that we have at most one uxto, witness or non-witness.
-        if (!input.IsSane()) {
-            return TransactionError::INVALID_PSBT;
-        }
-
         // If we have no utxo, grab it from the wallet.
         if (!input.non_witness_utxo && input.witness_utxo.IsNull()) {
             const uint256& txhash = txin.prevout.hash;

From 3228b59b1761a80bca6aa7309cb46db5acafc905 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Thu, 4 Jun 2020 23:43:43 -0400
Subject: [PATCH 09/15] psbt: always put a non_witness_utxo and don't remove it

Offline signers will always need a non_witness_utxo so make sure it is
there.

Github-Pull: #19215
Rebased-From: 46004790588c24174a0bec49b540d158ce163ffd
---
 src/psbt.cpp                          | 7 ++++---
 src/psbt.h                            | 4 ++--
 src/wallet/test/psbt_wallet_tests.cpp | 2 +-
 src/wallet/wallet.cpp                 | 2 +-
 test/functional/rpc_psbt.py           | 4 +++-
 5 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/src/psbt.cpp b/src/psbt.cpp
index ff17981fd8c..bfa9e3c667c 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -136,8 +136,8 @@ void PSBTInput::Merge(const PSBTInput& input)
 {
     if (!non_witness_utxo && input.non_witness_utxo) non_witness_utxo = input.non_witness_utxo;
     if (witness_utxo.IsNull() && !input.witness_utxo.IsNull()) {
+        // TODO: For segwit v1, we will want to clear out the non-witness utxo when setting a witness one. For v0 and non-segwit, this is not safe
         witness_utxo = input.witness_utxo;
-        non_witness_utxo = nullptr; // Clear out any non-witness utxo when we set a witness one.
     }
 
     partial_sigs.insert(input.partial_sigs.begin(), input.partial_sigs.end());
@@ -263,10 +263,11 @@ bool SignPSBTInput(const SigningProvider& provider, PartiallySignedTransaction&
     if (require_witness_sig && !sigdata.witness) return false;
     input.FromSignatureData(sigdata);
 
-    // If we have a witness signature, use the smaller witness UTXO.
+    // If we have a witness signature, put a witness UTXO.
+    // TODO: For segwit v1, we should remove the non_witness_utxo
     if (sigdata.witness) {
         input.witness_utxo = utxo;
-        input.non_witness_utxo = nullptr;
+        // input.non_witness_utxo = nullptr;
     }
 
     // Fill in the missing info
diff --git a/src/psbt.h b/src/psbt.h
index 1f7f84af6b2..6eb411e3a43 100644
--- a/src/psbt.h
+++ b/src/psbt.h
@@ -63,12 +63,12 @@ struct PSBTInput
     template <typename Stream>
     inline void Serialize(Stream& s) const {
         // Write the utxo
-        // If there is a non-witness utxo, then don't add the witness one.
         if (non_witness_utxo) {
             SerializeToVector(s, PSBT_IN_NON_WITNESS_UTXO);
             OverrideStream<Stream> os(&s, s.GetType(), s.GetVersion() | SERIALIZE_TRANSACTION_NO_WITNESS);
             SerializeToVector(os, non_witness_utxo);
-        } else if (!witness_utxo.IsNull()) {
+        }
+        if (!witness_utxo.IsNull()) {
             SerializeToVector(s, PSBT_IN_WITNESS_UTXO);
             SerializeToVector(s, witness_utxo);
         }
diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp
index 8b7b7af21d5..f751ba5a3a6 100644
--- a/src/wallet/test/psbt_wallet_tests.cpp
+++ b/src/wallet/test/psbt_wallet_tests.cpp
@@ -66,7 +66,7 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test)
     CDataStream ssTx(SER_NETWORK, PROTOCOL_VERSION);
     ssTx << psbtx;
     std::string final_hex = HexStr(ssTx.begin(), ssTx.end());
-    BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f0000008000000080010000800001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008000");
+    BOOST_CHECK_EQUAL(final_hex, "70736274ff01009a020000000258e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd750000000000ffffffff838d0427d0ec650a68aa46bb0b098aea4422c071b2ca78352a077959d07cea1d0100000000ffffffff0270aaf00800000000160014d85c2b71d0060b09c9886aeb815e50991dda124d00e1f5050000000016001400aea9a2e5f0f876a588df5546e8742d1d87008f00000000000100bb0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f6187650000000104475221029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f2102dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d752ae2206029583bf39ae0a609747ad199addd634fa6108559d6c5cd39b4c2183f1ab96e07f10d90c6a4f000000800000008000000080220602dab61ff49a14db6a7d02b0cd1fbb78fc4b18312b5b4e54dae4dba2fbfef536d710d90c6a4f0000008000000080010000800001008a020000000158e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd7501000000171600145f275f436b09a8cc9a2eb2a2f528485c68a56323feffffff02d8231f1b0100000017a914aed962d6654f9a2b36608eb9d64d2b260db4f1118700c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e8876500000001012000c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88701042200208c2353173743b595dfb4a07b72ba8e42e3797da74e87fe7d9d7497e3b2028903010547522103089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc21023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7352ae2206023add904f3d6dcf59ddb906b0dee23529b7ffb9ed50e5e86151926860221f0e7310d90c6a4f000000800000008003000080220603089dc10c7ac6db54f91329af617333db388cead0c231f723379d1b99030b02dc10d90c6a4f00000080000000800200008000220203a9a4c37f5996d3aa25dbac6b570af0650394492942460b354753ed9eeca5877110d90c6a4f000000800000008004000080002202027f6399757d2eff55a136ad02c684b1838b6556e5f1b6b34282a94b6b5005109610d90c6a4f00000080000000800500008000");
 
     // Mutate the transaction so that one of the inputs is invalid
     psbtx.tx->vin[0].prevout.n = 2;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index a6962ba428b..4394b2748a8 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2518,7 +2518,7 @@ TransactionError CWallet::FillPSBT(PartiallySignedTransaction& psbtx, bool& comp
         }
 
         // If we have no utxo, grab it from the wallet.
-        if (!input.non_witness_utxo && input.witness_utxo.IsNull()) {
+        if (!input.non_witness_utxo) {
             const uint256& txhash = txin.prevout.hash;
             const auto it = mapWallet.find(txhash);
             if (it != mapWallet.end()) {
diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 3a633775454..285c38161cb 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -37,6 +37,7 @@ class PSBTTest(BitcoinTestFramework):
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
 
+    # TODO: Re-enable this test with segwit v1
     def test_utxo_conversion(self):
         mining_node = self.nodes[2]
         offline_node = self.nodes[0]
@@ -326,7 +327,8 @@ class PSBTTest(BitcoinTestFramework):
         for i, signer in enumerate(signers):
             self.nodes[2].unloadwallet("wallet{}".format(i))
 
-        self.test_utxo_conversion()
+        # TODO: Re-enable this for segwit v1
+        # self.test_utxo_conversion()
 
         # Test that psbts with p2pkh outputs are created properly
         p2pkh = self.nodes[0].getnewaddress(address_type='legacy')

From cf0b5a933da540416b530ca6eb0259ab18cc6548 Mon Sep 17 00:00:00 2001
From: Andrew Chow <achow101-github@achow101.com>
Date: Mon, 8 Jun 2020 19:27:16 -0400
Subject: [PATCH 10/15] tests: Check that segwit inputs in psbt have both UTXO
 types

Github-Pull: #19215
Rebased-From: 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3
---
 test/functional/rpc_psbt.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py
index 285c38161cb..b8586f255ce 100755
--- a/test/functional/rpc_psbt.py
+++ b/test/functional/rpc_psbt.py
@@ -134,6 +134,10 @@ class PSBTTest(BitcoinTestFramework):
         # spend single key from node 1
         rawtx = self.nodes[1].walletcreatefundedpsbt([{"txid":txid,"vout":p2wpkh_pos},{"txid":txid,"vout":p2sh_p2wpkh_pos},{"txid":txid,"vout":p2pkh_pos}], {self.nodes[1].getnewaddress():29.99})['psbt']
         walletprocesspsbt_out = self.nodes[1].walletprocesspsbt(rawtx)
+        # Make sure it has both types of UTXOs
+        decoded = self.nodes[1].decodepsbt(walletprocesspsbt_out['psbt'])
+        assert 'non_witness_utxo' in decoded['inputs'][0]
+        assert 'witness_utxo' in decoded['inputs'][0]
         assert_equal(walletprocesspsbt_out['complete'], True)
         self.nodes[1].sendrawtransaction(self.nodes[1].finalizepsbt(walletprocesspsbt_out['psbt'])['hex'])
 

From c9b49d28563487e76169c7749b309244cabc70bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= <joao.paulo.barbosa@gmail.com>
Date: Wed, 17 Jun 2020 10:47:29 +0100
Subject: [PATCH 11/15] wallet: Handle concurrent wallet loading

Github-Pull: #19300
Rebased-From: b9971ae5853c1d62e09d976a8705f4f731290d85
---
 src/wallet/wallet.cpp | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 4394b2748a8..af3f9583497 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -101,9 +101,11 @@ std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet)
     return interfaces::MakeHandler([it] { LOCK(cs_wallets); g_load_wallet_fns.erase(it); });
 }
 
+static Mutex g_loading_wallet_mutex;
 static Mutex g_wallet_release_mutex;
 static std::condition_variable g_wallet_release_cv;
-static std::set<std::string> g_unloading_wallet_set;
+static std::set<std::string> g_loading_wallet_set GUARDED_BY(g_loading_wallet_mutex);
+static std::set<std::string> g_unloading_wallet_set GUARDED_BY(g_wallet_release_mutex);
 
 // Custom deleter for shared_ptr<CWallet>.
 static void ReleaseWallet(CWallet* wallet)
@@ -147,7 +149,8 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet)
     }
 }
 
-std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector<std::string>& warnings)
+namespace {
+std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector<std::string>& warnings)
 {
     try {
         if (!CWallet::Verify(chain, location, false, error, warnings)) {
@@ -168,6 +171,19 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocati
         return nullptr;
     }
 }
+} // namespace
+
+std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector<std::string>& warnings)
+{
+    auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(location.GetName()));
+    if (!result.second) {
+        error = "Wallet already being loading.";
+        return nullptr;
+    }
+    auto wallet = LoadWalletInternal(chain, location, error, warnings);
+    WITH_LOCK(g_loading_wallet_mutex, g_loading_wallet_set.erase(result.first));
+    return wallet;
+}
 
 std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector<std::string>& warnings)
 {

From eb6b82a5586a0cea5b72c376ff377a7322acb588 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= <joao.paulo.barbosa@gmail.com>
Date: Wed, 17 Jun 2020 14:00:25 +0100
Subject: [PATCH 12/15] qa: Test concurrent wallet loading

Github-Pull: #19300
Rebased-From: 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7
---
 test/functional/wallet_multiwallet.py | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py
index a2c502f280c..294de76b7bd 100755
--- a/test/functional/wallet_multiwallet.py
+++ b/test/functional/wallet_multiwallet.py
@@ -6,19 +6,36 @@
 
 Verify that a bitcoind node can load multiple wallet files
 """
+from threading import Thread
 import os
 import shutil
 import time
 
+from test_framework.authproxy import JSONRPCException
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.test_node import ErrorMatch
 from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
+    get_rpc_proxy,
 )
 
 FEATURE_LATEST = 169900
 
+got_loading_error = False
+def test_load_unload(node, name):
+    global got_loading_error
+    for i in range(10):
+        if got_loading_error:
+            return
+        try:
+            node.loadwallet(name)
+            node.unloadwallet(name)
+        except JSONRPCException as e:
+            if e.error['code'] == -4 and 'Wallet already being loading' in e.error['message']:
+                got_loading_error = True
+                return
+
 
 class MultiWalletTest(BitcoinTestFramework):
     def set_test_params(self):
@@ -219,6 +236,18 @@ class MultiWalletTest(BitcoinTestFramework):
         w2 = node.get_wallet_rpc(wallet_names[1])
         w2.getwalletinfo()
 
+        self.log.info("Concurrent wallet loading")
+        threads = []
+        for _ in range(3):
+            n = node.cli if self.options.usecli else get_rpc_proxy(node.url, 1, timeout=600, coveragedir=node.coverage_dir)
+            t = Thread(target=test_load_unload, args=(n, wallet_names[2], ))
+            t.start()
+            threads.append(t)
+        for t in threads:
+            t.join()
+        global got_loading_error
+        assert_equal(got_loading_error, True)
+
         self.log.info("Load remaining wallets")
         for wallet_name in wallet_names[2:]:
             loadwallet_name = self.nodes[0].loadwallet(wallet_name)

From e7f06f9b0e84a65812d24ff6efa4bc2d3d818590 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Fri, 3 Jul 2020 09:59:33 +0800
Subject: [PATCH 13/15] test: remove Cirrus CI FreeBSD job

This has been removed from the master branch, and always seems to fail
at the installing packages phase. i.e:
```bash
pkg install -y autoconf automake boost-libs git gmake libevent libtool pkgconf python3 ccache
Updating FreeBSD repository catalogue...
Fetching meta.txz: . done
pkg: repository meta /var/db/pkg/FreeBSD.meta has wrong version 2
repository FreeBSD has no meta file, using default settings
Fetching packagesite.txz: .......... done
pkg: repository meta /var/db/pkg/FreeBSD.meta has wrong version 2
pkg: Repository FreeBSD load error: meta cannot be loaded No error: 0
Unable to open created repository FreeBSD
Unable to update repository FreeBSD
Error updating repositories!

Exit status: 3
```
---
 .cirrus.yml | 30 ------------------------------
 1 file changed, 30 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index f9c3d844bef..f680715f2fe 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -1,33 +1,3 @@
-task:
-  name: "FreeBsd 12.0 amd64  [GOAL: install]  [no depends, only system libs]"
-  freebsd_instance:
-    image: freebsd-12-0-release-amd64
-    cpu: 8
-    memory: 8G
-  timeout_in: 60m
-  env:
-    MAKEJOBS: "-j9"
-    CONFIGURE_OPTS: "--disable-dependency-tracking"
-    GOAL: "install"
-    TEST_RUNNER_PORT_MIN: "14000"  # Must be larger than 12321, which is used for the http cache. See https://cirrus-ci.org/guide/writing-tasks/#http-cache
-    CCACHE_SIZE: "200M"
-    CCACHE_COMPRESS: 1
-    CCACHE_DIR: "/tmp/ccache_dir"
-  ccache_cache:
-    folder: "/tmp/ccache_dir"
-  install_script:
-    - pkg install -y autoconf automake boost-libs git gmake libevent libtool pkgconf python3 ccache
-    - ./contrib/install_db4.sh $(pwd)
-    - ccache --max-size=${CCACHE_SIZE}
-  configure_script:
-    - ./autogen.sh
-    - ./configure ${CONFIGURE_OPTS} BDB_LIBS="-L$(pwd)/db4/lib -ldb_cxx-4.8" BDB_CFLAGS="-I$(pwd)/db4/include" || ( cat config.log && false)
-  make_script:
-    - gmake ${MAKEJOBS} ${GOAL} || ( echo "Build failure. Verbose build follows." && gmake ${GOAL} V=1 ; false )
-  check_script:
-    - gmake check ${MAKEJOBS} VERBOSE=1
-  functional_test_script:
-    - ./test/functional/test_runner.py --jobs 9 --ci --extended --exclude feature_dbcrash --combinedlogslen=1000 --quiet --failfast
 task:
   name: "x86_64 Linux  [GOAL: install]  [bionic]  [Using ./ci/ system]"
   container:

From 04773480575ac79f238ac5764247dddd0cae5051 Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter@wuille.net>
Date: Mon, 8 Jun 2020 18:46:53 -0700
Subject: [PATCH 14/15] Replace automatic bans with discouragement filter

This patch improves performance and resource usage around IP
addresses that are banned for misbehavior. They're already not
actually banned, 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 better reflect reality;
they're not banned, just discouraged.

Contains release notes and several interface improvements by
John Newbery.

Github-Pull: #19219
Rebased-From: b691f2df5f7d443c0c9ee056ab94aa0fc19566d5
---
 doc/release-notes-19219.md         | 23 +++++++++++++++
 src/banman.cpp                     | 13 ++++++---
 src/banman.h                       | 45 ++++++++++++++++++++----------
 src/init.cpp                       |  4 +--
 src/net.cpp                        | 13 +++++++--
 src/net_processing.cpp             | 21 +++++++-------
 src/netaddress.h                   |  1 +
 src/rpc/net.cpp                    |  9 +++---
 src/test/denialofservice_tests.cpp |  6 ----
 9 files changed, 91 insertions(+), 44 deletions(-)
 create mode 100644 doc/release-notes-19219.md

diff --git a/doc/release-notes-19219.md b/doc/release-notes-19219.md
new file mode 100644
index 00000000000..b5ee885ddc3
--- /dev/null
+++ b/doc/release-notes-19219.md
@@ -0,0 +1,23 @@
+#### Changes regarding misbehaving peers
+
+Peers that misbehave (e.g. send us invalid blocks) are now referred to as
+discouraged nodes in log output, as they're not (and weren't) strictly banned:
+incoming connections are still allowed from them, but they're preferred for
+eviction.
+
+Furthermore, a few additional changes are introduced to how discouraged
+addresses are treated:
+
+- Discouraging an address does not time out automatically after 24 hours
+  (or the `-bantime` setting). Depending on traffic from other peers,
+  discouragement may time out at an indeterminate time.
+
+- Discouragement is not persisted over restarts.
+
+- There is no method to list discouraged addresses. They are not returned by
+  the `listbanned` RPC. That RPC also no longer reports the `ban_reason`
+  field, as `"manually added"` is the only remaining option.
+
+- Discouragement cannot be removed with the `setban remove` RPC command.
+  If you need to remove a discouragement, you can remove all discouragements by
+  stop-starting your node.
diff --git a/src/banman.cpp b/src/banman.cpp
index 9cc584f0e49..16bd452704f 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -74,7 +74,6 @@ int BanMan::IsBannedLevel(CNetAddr net_addr)
     // 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) {
@@ -82,17 +81,17 @@ int BanMan::IsBannedLevel(CNetAddr net_addr)
         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 2;
         }
     }
-    return level;
+    return m_discouraged.contains(net_addr.GetAddrBytes()) ? 1 : 0;
 }
 
 bool BanMan::IsBanned(CNetAddr net_addr)
 {
     auto current_time = GetTime();
     LOCK(m_cs_banned);
+    if (m_discouraged.contains(net_addr.GetAddrBytes())) return true;
     for (const auto& it : m_banned) {
         CSubNet sub_net = it.first;
         CBanEntry ban_entry = it.second;
@@ -120,12 +119,18 @@ bool BanMan::IsBanned(CSubNet sub_net)
 
 void BanMan::Ban(const CNetAddr& net_addr, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
 {
+    if (ban_reason == BanReasonNodeMisbehaving) {
+        LOCK(m_cs_banned);
+        m_discouraged.insert(net_addr.GetAddrBytes());
+        return;
+    }
     CSubNet sub_net(net_addr);
     Ban(sub_net, ban_reason, ban_time_offset, since_unix_epoch);
 }
 
 void BanMan::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
 {
+    assert(ban_reason == BanReasonManuallyAdded);
     CBanEntry ban_entry(GetTime(), ban_reason);
 
     int64_t normalized_ban_time_offset = ban_time_offset;
diff --git a/src/banman.h b/src/banman.h
index 6bea2e75e94..aad73c514f0 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,20 +24,35 @@ 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
 {
@@ -68,6 +84,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 d015fb4b6b3..f381a53b1b0 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -422,8 +422,8 @@ void SetupServerArgs()
 
     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/net.cpp b/src/net.cpp
index dcc613ba880..ce85dc1d79e 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -998,15 +998,22 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
 
     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.
+    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel == 2)
     {
         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.
+    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && bannedlevel >= 1)
+    {
+        LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString());
+        CloseSocket(hSocket);
+        return;
+    }
+
     if (nInbound >= nMaxInbound)
     {
         if (!AttemptToEvictConnection()) {
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 2a61f84a085..71d206bc1a9 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -963,7 +963,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)
 {
@@ -979,14 +979,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);
+        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->fShouldBan = 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
@@ -1016,7 +1016,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);
@@ -1051,7 +1051,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)
  */
@@ -1278,7 +1278,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) {
@@ -2833,7 +2833,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             // 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, mempool, {cmpctblock.header}, chainparams, /*via_compact_block=*/true);
         }
 
@@ -2919,7 +2919,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
                 // 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
@@ -3267,11 +3267,12 @@ bool PeerLogicValidation::CheckIfBanned(CNode* pnode)
         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());
+            // 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
+            LogPrintf("Disconnecting and discouraging peer %s!\n", pnode->addr.ToString());
             if (m_banman) {
                 m_banman->Ban(pnode->addr, BanReasonNodeMisbehaving);
             }
diff --git a/src/netaddress.h b/src/netaddress.h
index b7381c1eb40..0c8d600a4a7 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/rpc/net.cpp b/src/rpc/net.cpp
index 10562126dba..3886efcadf9 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -591,7 +591,8 @@ static UniValue setban(const JSONRPCRequest& request)
 
     if (strCommand == "add")
     {
-        if (isSubnet ? g_rpc_node->banman->IsBanned(subNet) : g_rpc_node->banman->IsBanned(netAddr)) {
+        if ((isSubnet && g_rpc_node->banman->IsBanned(subNet)) ||
+            (!isSubnet && g_rpc_node->banman->IsBannedLevel(netAddr) == BanReasonManuallyAdded)) {
             throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: IP/Subnet already banned");
         }
 
@@ -618,7 +619,7 @@ static UniValue setban(const JSONRPCRequest& request)
     else if(strCommand == "remove")
     {
         if (!( isSubnet ? g_rpc_node->banman->Unban(subNet) : g_rpc_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;
@@ -627,7 +628,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, "", "",
             {
@@ -636,7 +637,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{
@@ -660,7 +660,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 6314c1a42f0..bbe4df8550a 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -346,12 +346,6 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     }
     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));
-
     bool dummy;
     peerLogic->FinalizeNode(dummyNode.GetId(), dummy);
 }

From 2b79ac740622218a52e2cf3ae1755dd3a8d4847a Mon Sep 17 00:00:00 2001
From: Pieter Wuille <pieter@wuille.net>
Date: Wed, 10 Jun 2020 17:11:38 -0700
Subject: [PATCH 15/15] Clean up separated ban/discourage interface

Github-Pull: #19219
Rebased-From: 2ad58381fffb33d611abf900b73d9e6b5a4e35f8
---
 src/addrdb.h                       | 28 ++----------------
 src/banman.cpp                     | 46 +++++++++++-------------------
 src/banman.h                       | 18 ++++++++----
 src/interfaces/node.cpp            |  4 +--
 src/interfaces/node.h              |  2 +-
 src/net.cpp                        | 16 +++++------
 src/net_permissions.h              |  2 +-
 src/net_processing.cpp             | 33 ++++++++++-----------
 src/net_processing.h               |  2 +-
 src/qt/rpcconsole.cpp              |  2 +-
 src/rpc/net.cpp                    |  7 ++---
 src/test/denialofservice_tests.cpp | 18 ++++++------
 src/test/fuzz/addrdb.cpp           | 12 ++------
 13 files changed, 77 insertions(+), 113 deletions(-)

diff --git a/src/addrdb.h b/src/addrdb.h
index c6d4307d694..8410c3776c4 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 16bd452704f..a1bb5be903f 100644
--- a/src/banman.cpp
+++ b/src/banman.cpp
@@ -68,30 +68,16 @@ 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
-    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)) {
-            return 2;
-        }
-    }
-    return m_discouraged.contains(net_addr.GetAddrBytes()) ? 1 : 0;
+    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);
-    if (m_discouraged.contains(net_addr.GetAddrBytes())) return true;
     for (const auto& it : m_banned) {
         CSubNet sub_net = it.first;
         CBanEntry ban_entry = it.second;
@@ -103,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);
@@ -117,21 +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)
 {
-    if (ban_reason == BanReasonNodeMisbehaving) {
-        LOCK(m_cs_banned);
-        m_discouraged.insert(net_addr.GetAddrBytes());
-        return;
-    }
     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::Ban(const CSubNet& sub_net, const BanReason& ban_reason, int64_t ban_time_offset, bool since_unix_epoch)
+void BanMan::Discourage(const CNetAddr& net_addr)
 {
-    assert(ban_reason == BanReasonManuallyAdded);
-    CBanEntry ban_entry(GetTime(), ban_reason);
+    LOCK(m_cs_banned);
+    m_discouraged.insert(net_addr.GetAddrBytes());
+}
+
+void BanMan::Ban(const CSubNet& sub_net, int64_t ban_time_offset, bool since_unix_epoch)
+{
+    CBanEntry ban_entry(GetTime());
 
     int64_t normalized_ban_time_offset = ban_time_offset;
     bool normalized_since_unix_epoch = since_unix_epoch;
@@ -151,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 aad73c514f0..f6bfbd1e493 100644
--- a/src/banman.h
+++ b/src/banman.h
@@ -59,12 +59,20 @@ 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);
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 905173d20b7..3bd32909bfc 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -136,10 +136,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 53a20886cd1..61acde0523e 100644
--- a/src/interfaces/node.h
+++ b/src/interfaces/node.h
@@ -118,7 +118,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 ce85dc1d79e..09ddcb163d6 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -996,10 +996,9 @@ 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.
-    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && bannedlevel == 2)
+    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);
@@ -1007,7 +1006,8 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     }
 
     // Only accept connections from discouraged peers if our inbound slots aren't (almost) full.
-    if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::PF_NOBAN) && nInbound + 1 >= nMaxInbound && bannedlevel >= 1)
+    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);
@@ -1037,7 +1037,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());
@@ -1994,10 +1994,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 ad748483478..a1b45c40099 100644
--- a/src/net_permissions.h
+++ b/src/net_permissions.h
@@ -18,7 +18,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 71d206bc1a9..ddc516ce0f8 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -215,8 +215,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.
@@ -367,7 +367,7 @@ struct CNodeState {
     {
         fCurrentlyConnected = false;
         nMisbehavior = 0;
-        fShouldBan = false;
+        m_should_discourage = false;
         pindexBestKnownBlock = nullptr;
         hashLastUnknownBlock.SetNull();
         pindexLastCommonBlock = nullptr;
@@ -980,7 +980,7 @@ void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIV
     if (state->nMisbehavior >= banscore && state->nMisbehavior - howmuch < banscore)
     {
         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->fShouldBan = true;
+        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);
 }
@@ -2191,7 +2191,8 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
             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())
             {
@@ -3044,7 +3045,7 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
         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);
             }
         }
@@ -3255,26 +3256,26 @@ bool ProcessMessage(CNode* pfrom, const std::string& msg_type, CDataStream& vRec
     return true;
 }
 
-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()) {
+        } 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);
         }
@@ -3381,7 +3382,7 @@ bool PeerLogicValidation::ProcessMessages(CNode* pfrom, std::atomic<bool>& inter
     }
 
     LOCK(cs_main);
-    CheckIfBanned(pfrom);
+    MaybeDiscourageAndDisconnect(pfrom);
 
     return fMoreWork;
 }
@@ -3584,7 +3585,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 3d9bc65a3ac..ddb17801480 100644
--- a/src/net_processing.h
+++ b/src/net_processing.h
@@ -28,7 +28,7 @@ private:
     BanMan* const m_banman;
     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, CTxMemPool& pool);
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 0ffdc892c5a..01e387f74b8 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1216,7 +1216,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 3886efcadf9..8c400c05a02 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -591,8 +591,7 @@ static UniValue setban(const JSONRPCRequest& request)
 
     if (strCommand == "add")
     {
-        if ((isSubnet && g_rpc_node->banman->IsBanned(subNet)) ||
-            (!isSubnet && g_rpc_node->banman->IsBannedLevel(netAddr) == BanReasonManuallyAdded)) {
+        if (isSubnet ? g_rpc_node->banman->IsBanned(subNet) : g_rpc_node->banman->IsBanned(netAddr)) {
             throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: IP/Subnet already banned");
         }
 
@@ -605,12 +604,12 @@ static UniValue setban(const JSONRPCRequest& request)
             absolute = true;
 
         if (isSubnet) {
-            g_rpc_node->banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute);
+            g_rpc_node->banman->Ban(subNet, banTime, absolute);
             if (g_rpc_node->connman) {
                 g_rpc_node->connman->DisconnectNode(subNet);
             }
         } else {
-            g_rpc_node->banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
+            g_rpc_node->banman->Ban(netAddr, banTime, absolute);
             if (g_rpc_node->connman) {
                 g_rpc_node->connman->DisconnectNode(netAddr);
             }
diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index bbe4df8550a..41d10ae3217 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,7 +344,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
         LOCK2(cs_main, dummyNode.cs_sendProcessing);
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
     }
-    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 f21ff3fac38..a7e6e8961a8 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 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());
 }