bumpfee: Special case for reducing single output tx

When choosing the only output of a single output tx as the output to
reduce the fee from, we need to deal with it specially as the normal
transcation creation code cannot handle the fact that there are no
specified recipients.

Author: achow101

src/wallet/feebumper.cpp

@@ -247,16 +247,17 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
 
     old_fee = input_value - output_value;
 
+    // We need to make a temporary transaction with no input witnesses as the dummy signer expects them to be empty for external inputs
+    CMutableTransaction temp_mtx{*wtx.tx};
+    for (auto& txin : temp_mtx.vin) {
+        txin.scriptSig.clear();
+        txin.scriptWitness.SetNull();
+    }
+    const int64_t maxTxSize{CalculateMaximumSignedTxSize(CTransaction(temp_mtx), &wallet, &new_coin_control).vsize};
+
     if (coin_control.m_feerate) {
         // The user provided a feeRate argument.
         // We calculate this here to avoid compiler warning on the cs_wallet lock
-        // We need to make a temporary transaction with no input witnesses as the dummy signer expects them to be empty for external inputs
-        CMutableTransaction mtx{*wtx.tx};
-        for (auto& txin : mtx.vin) {
-            txin.scriptSig.clear();
-            txin.scriptWitness.SetNull();
-        }
-        const int64_t maxTxSize{CalculateMaximumSignedTxSize(CTransaction(mtx), &wallet, &new_coin_control).vsize};
         Result res = CheckFeeRate(wallet, wtx, *new_coin_control.m_feerate, maxTxSize, old_fee, errors);
         if (res != Result::OK) {
             return res;
@@ -281,21 +282,35 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo
     // We cannot source new unconfirmed inputs(bip125 rule 2)
     new_coin_control.m_min_depth = 1;
 
-    constexpr int RANDOM_CHANGE_POSITION = -1;
-    auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false);
-    if (!res) {
-        errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res));
-        return Result::WALLET_ERROR;
-    }
+    if (recipients.size() > 0) {
+        constexpr int RANDOM_CHANGE_POSITION = -1;
+        auto res = CreateTransaction(wallet, recipients, RANDOM_CHANGE_POSITION, new_coin_control, false);
+        if (!res) {
+            errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res));
+            return Result::WALLET_ERROR;
+        }
 
-    const auto& txr = *res;
-    // Write back new fee if successful
-    new_fee = txr.fee;
+        const auto& txr = *res;
+        // Write back new fee if successful
+        new_fee = txr.fee;
 
-    // Write back transaction
-    mtx = CMutableTransaction(*txr.tx);
+        // Write back transaction
+        mtx = CMutableTransaction(*txr.tx);
 
-    return Result::OK;
+        return Result::OK;
+    } else {
+        // The transaction only has one output, and it is either detected as change,
+        // or selected as the one to take the fee from.
+        // In that case, just reduce its value to meet the new feerate.
+        new_fee = new_coin_control.m_feerate->GetFee(maxTxSize);
+        temp_mtx.vout[0].nValue += old_fee - new_fee;
+        if (IsDust(temp_mtx.vout[0], wallet.chain().relayDustFee())) {
+            errors.push_back(Untranslated("Single output amount too small"));
+            return Result::MISC_ERROR;
+        }
+        mtx = temp_mtx;
+        return Result::OK;
+    }
 }
 
 bool SignTransaction(CWallet& wallet, CMutableTransaction& mtx) {

test/functional/wallet_bumpfee.py

@@ -28,6 +28,7 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
+    assert_fee_amount,
     assert_greater_than,
     assert_raises_rpc_error,
     find_vout_for_address,
@@ -105,6 +106,7 @@ def run_test(self):
         test_small_output_with_feerate_succeeds(self, rbf_node, dest_address)
         test_no_more_inputs_fails(self, rbf_node, dest_address)
         self.test_provided_change_pos(rbf_node)
+        self.test_single_output()
 
     def test_invalid_parameters(self, rbf_node, peer_node, dest_address):
         self.log.info('Test invalid parameters')
@@ -195,6 +197,42 @@ def test_provided_change_pos(self, rbf_node):
         assert_greater_than(change_value, new_change_value)
 
 
+    def test_single_output(self):
+        self.log.info("Test that single output txs can be bumped")
+        node = self.nodes[1]
+
+        node.createwallet("single_out_rbf")
+        wallet = node.get_wallet_rpc("single_out_rbf")
+
+        addr = wallet.getnewaddress()
+        amount = Decimal("0.001")
+        # Make 2 UTXOs
+        self.nodes[0].sendtoaddress(addr, amount)
+        self.nodes[0].sendtoaddress(addr, amount)
+        self.generate(self.nodes[0], 1)
+        utxos = wallet.listunspent()
+
+        tx = wallet.sendall(recipients=[wallet.getnewaddress()], fee_rate=2, options={"inputs": [utxos[0]]})
+
+        # Reduce the only output with a crazy high feerate, should fail as the output would be dust
+        assert_raises_rpc_error(-1, "Single output amount too small", wallet.bumpfee, txid=tx["txid"], options={"fee_rate": 1100, "reduce_output": 0})
+
+        # Reduce the only output successfully
+        bumped = wallet.bumpfee(txid=tx["txid"], options={"fee_rate": 10, "reduce_output": 0})
+        bumped_tx = wallet.gettransaction(txid=bumped["txid"], verbose=True)
+        assert_equal(len(bumped_tx["decoded"]["vout"]), 1)
+        assert_equal(len(bumped_tx["decoded"]["vin"]), 1)
+        assert_equal(bumped_tx["decoded"]["vout"][0]["value"] + bumped["fee"], amount)
+        assert_fee_amount(bumped["fee"], bumped_tx["decoded"]["vsize"], Decimal(10) / Decimal(1e8) * 1000)
+
+        # Bumping without reducing adds a new input and output
+        bumped = wallet.bumpfee(txid=bumped["txid"], options={"fee_rate": 20})
+        bumped_tx = wallet.gettransaction(txid=bumped["txid"], verbose=True)
+        assert_equal(len(bumped_tx["decoded"]["vout"]), 2)
+        assert_equal(len(bumped_tx["decoded"]["vin"]), 2)
+        assert_fee_amount(bumped["fee"], bumped_tx["decoded"]["vsize"], Decimal(20) / Decimal(1e8) * 1000)
+
+
 def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
     self.log.info('Test simple bumpfee: {}'.format(mode))
     rbfid = spend_one_input(rbf_node, dest_address)