bumpfee: be able to bump fee of a tx with external inputs

In some cases, notably psbtbumpfee, it is okay, and potentially desired,
to be able to bump the fee of a transaction which contains external
inputs.

Author: achow101

src/wallet/feebumper.cpp

@@ -20,7 +20,7 @@
 namespace wallet {
 //! Check whether transaction has descendant in wallet or mempool, or has been
 //! mined, or conflicts with a mined transaction. Return a feebumper::Result.
-static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
+static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWalletTx& wtx, bool require_mine, std::vector<bilingual_str>& errors) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
     if (wallet.HasWalletSpend(wtx.tx)) {
         errors.push_back(Untranslated("Transaction has descendants in the wallet"));
@@ -49,15 +49,16 @@ static feebumper::Result PreconditionChecks(const CWallet& wallet, const CWallet
         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)
-    isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
-    if (!AllInputsMine(wallet, *wtx.tx, filter)) {
-        errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
-        return feebumper::Result::WALLET_ERROR;
+    if (require_mine) {
+        // 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)
+        isminefilter filter = wallet.GetLegacyScriptPubKeyMan() && wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) ? ISMINE_WATCH_ONLY : ISMINE_SPENDABLE;
+        if (!AllInputsMine(wallet, *wtx.tx, filter)) {
+            errors.push_back(Untranslated("Transaction contains inputs that don't belong to this wallet"));
+            return feebumper::Result::WALLET_ERROR;
+        }
     }
 
-
     return feebumper::Result::OK;
 }
 
@@ -149,12 +150,12 @@ bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid)
     if (wtx == nullptr) return false;
 
     std::vector<bilingual_str> errors_dummy;
-    feebumper::Result res = PreconditionChecks(wallet, *wtx, errors_dummy);
+    feebumper::Result res = PreconditionChecks(wallet, *wtx, /* require_mine=*/ true, errors_dummy);
     return res == feebumper::Result::OK;
 }
 
 Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCoinControl& coin_control, std::vector<bilingual_str>& errors,
-                                 CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx)
+                                 CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx, bool require_mine)
 {
     // We are going to modify coin control later, copy to re-use
     CCoinControl new_coin_control(coin_control);
@@ -216,7 +217,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
         }
     }
 
-    Result result = PreconditionChecks(wallet, wtx, errors);
+    Result result = PreconditionChecks(wallet, wtx, require_mine, errors);
     if (result != Result::OK) {
         return result;
     }
@@ -309,7 +310,7 @@ Result CommitTransaction(CWallet& wallet, const uint256& txid, CMutableTransacti
     const CWalletTx& oldWtx = it->second;
 
     // make sure the transaction still has no descendants and hasn't been mined in the meantime
-    Result result = PreconditionChecks(wallet, oldWtx, errors);
+    Result result = PreconditionChecks(wallet, oldWtx, /* require_mine=*/ false, errors);
     if (result != Result::OK) {
         return result;
     }

src/wallet/feebumper.h

@@ -33,14 +33,25 @@ enum class Result
 //! Return whether transaction can be bumped.
 bool TransactionCanBeBumped(const CWallet& wallet, const uint256& txid);
 
-//! Create bumpfee transaction based on feerate estimates.
+/** Create bumpfee transaction based on feerate estimates.
+ *
+ * @param[in] wallet The wallet to use for this bumping
+ * @param[in] txid The txid of the transaction to bump
+ * @param[in] coin_control A CCoinControl object which provides feerates and other information used for coin selection
+ * @param[out] errors Errors
+ * @param[out] old_fee The fee the original transaction pays
+ * @param[out] new_fee the fee that the bump transaction pays
+ * @param[out] mtx The bump transaction itself
+ * @param[in] require_mine Whether the original transaction must consist of inputs that can be spent by the wallet
+ */
 Result CreateRateBumpTransaction(CWallet& wallet,
     const uint256& txid,
     const CCoinControl& coin_control,
     std::vector<bilingual_str>& errors,
     CAmount& old_fee,
     CAmount& new_fee,
-    CMutableTransaction& mtx);
+    CMutableTransaction& mtx,
+    bool require_mine);
 
 //! Sign the new transaction,
 //! @return false if the tx couldn't be found or if it was

src/wallet/interfaces.cpp

@@ -291,7 +291,7 @@ class WalletImpl : public Wallet
         CAmount& new_fee,
         CMutableTransaction& mtx) override
     {
-        return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx) == feebumper::Result::OK;
+        return feebumper::CreateRateBumpTransaction(*m_wallet.get(), txid, coin_control, errors, old_fee, new_fee, mtx, /* require_mine= */ true) == feebumper::Result::OK;
     }
     bool signBumpTransaction(CMutableTransaction& mtx) override { return feebumper::SignTransaction(*m_wallet.get(), mtx); }
     bool commitBumpTransaction(const uint256& txid,

src/wallet/rpc/spend.cpp

@@ -1045,7 +1045,7 @@ static RPCHelpMan bumpfee_helper(std::string method_name)
     CMutableTransaction mtx;
     feebumper::Result res;
     // Targeting feerate bump.
-    res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx);
+    res = feebumper::CreateRateBumpTransaction(*pwallet, hash, coin_control, errors, old_fee, new_fee, mtx, /*require_mine=*/ !want_psbt);
     if (res != feebumper::Result::OK) {
         switch(res) {
             case feebumper::Result::INVALID_ADDRESS_OR_KEY: