Merge #12296: wallet: Only fee-bump non-conflicted/non-confirmed txes

faca18d feebumper: Use PreconditionChecks to determine bump eligibility (MarcoFalke)
718f05c move more bumpfee prechecks to feebumper::PreconditionChecks (Gregory Sanders)

Pull request description:

  This only affects the gui.

  Fee-bumping of transactions that are already confirmed or are already conflicted by other transactions should not be offered by the gui.

Tree-SHA512: 4acf8087c69fbe5bd67be0485cdb4055e985bbf84acc420aa786ad31e2dc6c2572baaac1d359af10a6907790f626edca690285d9a46ae5440900ea12624c634f

Author: jonasschnelli

src/wallet/feebumper.cpp

@@ -65,16 +65,39 @@ static feebumper::Result PreconditionChecks(const CWallet* wallet, const CWallet
         errors.push_back("Transaction has been mined, or is conflicted with a mined transaction");
         return feebumper::Result::WALLET_ERROR;
     }
+
+    if (!SignalsOptInRBF(*wtx.tx)) {
+        errors.push_back("Transaction is not BIP 125 replaceable");
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    if (wtx.mapValue.count("replaced_by_txid")) {
+        errors.push_back(strprintf("Cannot bump transaction %s which was already bumped by transaction %s", wtx.GetHash().ToString(), wtx.mapValue.at("replaced_by_txid")));
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+    // check that original tx consists entirely of our inputs
+    // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
+    if (!wallet->IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) {
+        errors.push_back("Transaction contains inputs that don't belong to this wallet");
+        return feebumper::Result::WALLET_ERROR;
+    }
+
+
     return feebumper::Result::OK;
 }
 
 namespace feebumper {
 
-bool TransactionCanBeBumped(CWallet* wallet, const uint256& txid)
+bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid)
 {
     LOCK2(cs_main, wallet->cs_wallet);
     const CWalletTx* wtx = wallet->GetWalletTx(txid);
-    return wtx && SignalsOptInRBF(*wtx->tx) && !wtx->mapValue.count("replaced_by_txid");
+    if (wtx == nullptr) return false;
+
+    std::vector<std::string> errors_dummy;
+    feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy);
+    return res == feebumper::Result::OK;
 }
 
 Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoinControl& coin_control, CAmount total_fee, std::vector<std::string>& errors,
@@ -94,23 +117,6 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
         return result;
     }
 
-    if (!SignalsOptInRBF(*wtx.tx)) {
-        errors.push_back("Transaction is not BIP 125 replaceable");
-        return Result::WALLET_ERROR;
-    }
-
-    if (wtx.mapValue.count("replaced_by_txid")) {
-        errors.push_back(strprintf("Cannot bump transaction %s which was already bumped by transaction %s", txid.ToString(), wtx.mapValue.at("replaced_by_txid")));
-        return Result::WALLET_ERROR;
-    }
-
-    // check that original tx consists entirely of our inputs
-    // if not, we can't bump the fee, because the wallet has no way of knowing the value of the other inputs (thus the fee)
-    if (!wallet->IsAllFromMe(*wtx.tx, ISMINE_SPENDABLE)) {
-        errors.push_back("Transaction contains inputs that don't belong to this wallet");
-        return Result::WALLET_ERROR;
-    }
-
     // figure out which output was change
     // if there was no change output or multiple change outputs, fail
     int nOutput = -1;
@@ -228,6 +234,7 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
         }
     }
 
+
     return Result::OK;
 }
 

src/wallet/feebumper.h

@@ -26,7 +26,7 @@ enum class Result
 };
 
 //! Return whether transaction can be bumped.
-bool TransactionCanBeBumped(CWallet* wallet, const uint256& txid);
+bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid);
 
 //! Create bumpfee transaction.
 Result CreateTransaction(const CWallet* wallet,