Merge #9697: [Qt] simple fee bumper with user verification

a38783747 Make sure we re-check the conditions of a feebump during commit (Jonas Schnelli)
9b9ca538c Only update the transactionrecord if the fee bump has been commited (Jonas Schnelli)
6ed4368f1 Make sure we use nTxConfirmTarget during Qt fee bumps (Jonas Schnelli)
be08fc39d Make sure we always update the table row after a bumpfee call (Jonas Schnelli)
2678d3dc6 Show old-fee, increase a new-fee in Qt fee bumper confirmation dialog (Jonas Schnelli)
2ec911f60 Add cs_wallet lock assertion to SignTransaction() (Jonas Schnelli)
fbf385cc8 [Qt] simple fee bumper with user verification (Jonas Schnelli)

Tree-SHA512: a3ce626201abf64cee496dd1d83870de51ba633de40c48eb0219c3eba5085c038af34c284512130d2544de20c1bff9fea1b78f92e3574c21dd4e96c11b8e7d76

3 files changed, 36 insertions, 19 deletions

diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp
index b3cb6a718c..c10a9eccd9 100644
--- a/src/wallet/feebumper.cpp
+++ b/src/wallet/feebumper.cpp
@@ -41,6 +41,31 @@ int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *pWal
     return GetVirtualTransactionSize(txNew);
 }
 
+bool CFeeBumper::preconditionChecks(const CWallet *pWallet, const CWalletTx& wtx) {
+    if (pWallet->HasWalletSpend(wtx.GetHash())) {
+        vErrors.push_back("Transaction has descendants in the wallet");
+        currentResult = BumpFeeResult::INVALID_PARAMETER;
+        return false;
+    }
+
+    {
+        LOCK(mempool.cs);
+        auto it_mp = mempool.mapTx.find(wtx.GetHash());
+        if (it_mp != mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1) {
+            vErrors.push_back("Transaction has descendants in the mempool");
+            currentResult = BumpFeeResult::INVALID_PARAMETER;
+            return false;
+        }
+    }
+
+    if (wtx.GetDepthInMainChain() != 0) {
+        vErrors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
+        currentResult = BumpFeeResult::WALLET_ERROR;
+        return false;
+    }
+    return true;
+}
+
 CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConfirmTarget, bool specifiedConfirmTarget, CAmount totalFee, bool newTxReplaceable)
     :
     txid(std::move(txidIn)),
@@ -58,25 +83,7 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, int newConf
     auto it = pWallet->mapWallet.find(txid);
     const CWalletTx& wtx = it->second;
 
-    if (pWallet->HasWalletSpend(txid)) {
-        vErrors.push_back("Transaction has descendants in the wallet");
-        currentResult = BumpFeeResult::INVALID_PARAMETER;
-        return;
-    }
-
-    {
-        LOCK(mempool.cs);
-        auto it_mp = mempool.mapTx.find(txid);
-        if (it_mp != mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1) {
-            vErrors.push_back("Transaction has descendants in the mempool");
-            currentResult = BumpFeeResult::INVALID_PARAMETER;
-            return;
-        }
-    }
-
-    if (wtx.GetDepthInMainChain() != 0) {
-        vErrors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
-        currentResult = BumpFeeResult::WALLET_ERROR;
+    if (!preconditionChecks(pWallet, wtx)) {
         return;
     }
 
@@ -248,6 +255,11 @@ bool CFeeBumper::commit(CWallet *pWallet)
     }
     CWalletTx& oldWtx = pWallet->mapWallet[txid];
 
+    // make sure the transaction still has no descendants and hasen't been mined in the meantime
+    if (!preconditionChecks(pWallet, oldWtx)) {
+        return false;
+    }
+
     CWalletTx wtxBumped(pWallet, MakeTransactionRef(std::move(mtx)));
     // commit/broadcast the tx
     CReserveKey reservekey(pWallet);
diff --git a/src/wallet/feebumper.h b/src/wallet/feebumper.h
index 681f55e4e5..f40d05da28 100644
--- a/src/wallet/feebumper.h
+++ b/src/wallet/feebumper.h
@@ -8,6 +8,7 @@
 #include <primitives/transaction.h>
 
 class CWallet;
+class CWalletTx;
 class uint256;
 
 enum class BumpFeeResult
@@ -44,6 +45,8 @@ public:
     bool commit(CWallet *pWalletNonConst);
 
 private:
+    bool preconditionChecks(const CWallet *pWallet, const CWalletTx& wtx);
+
     const uint256 txid;
     uint256 bumpedTxid;
     CMutableTransaction mtx;
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index dcb452a009..9eba8ad9fe 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -2311,6 +2311,8 @@ bool CWallet::SelectCoins(const std::vector<COutput>& vAvailableCoins, const CAm
 
 bool CWallet::SignTransaction(CMutableTransaction &tx)
 {
+    AssertLockHeld(cs_wallet); // mapWallet
+
     // sign the new tx
     CTransaction txNewConst(tx);
     int nIn = 0;