Merge bitcoin/bitcoin#27195: bumpfee: allow send coins back to yourself

be72663a1521bc6cdf16d43a4feae7c5b57735c0 test: bumpfee, add coverage for "send coins back to yourself" (furszy)
7bffec6715a75bb2c8631177d39f984aabc656ba bumpfee: enable send coins back to yourself (furszy)

Pull request description:

  Simple example:

  1) User_1 sends 0.1 btc to user_2 on a low fee transaction.
  2) After few hours, the tx is still in the mempool, user_2
     is not interested anymore, so user_1 decides to cancel
     it by sending coins back to himself.
  3) User_1 has the bright idea of opening the explorer and
     copy the change output address of the transaction. Then
     call bumpfee providing such output (in the "outputs" arg).

  Currently, this is not possible. The wallet fails with
  "Unable to create transaction. Transaction must have at least
  one recipient" error.
  The error reason is because we discard the provided output
  from the recipients list and set it inside the coin control
  so the process adds it later (when the change is calculated).
  But.. there is no later if the tx has no outputs.

ACKs for top commit:
  ishaanam:
    reACK be72663a1521bc6cdf16d43a4feae7c5b57735c0
  achow101:
    ACK be72663a1521bc6cdf16d43a4feae7c5b57735c0

Tree-SHA512: c2c38290a998f9b426a830d9624c7feb730158980ac186f8fb0138d5e200935d6538307bc60a2c3d0b7b6ee2b4ffb77a1e98baf8feb1d20a7d825f6055ac377f

1 files changed, 56 insertions, 8 deletions

diff --git a/test/functional/wallet_bumpfee.py b/test/functional/wallet_bumpfee.py
index 5ed7b0ccaa..19c8022600 100755
--- a/test/functional/wallet_bumpfee.py
+++ b/test/functional/wallet_bumpfee.py
@@ -41,6 +41,10 @@ NORMAL       =    100
 HIGH         =    500
 TOO_HIGH     = 100000
 
+def get_change_address(tx, node):
+    tx_details = node.getrawtransaction(tx, 1)
+    txout_addresses = [txout['scriptPubKey']['address'] for txout in tx_details["vout"]]
+    return [address for address in txout_addresses if node.getaddressinfo(address)["ischange"]]
 
 class BumpFeeTest(BitcoinTestFramework):
     def add_options(self, parser):
@@ -104,6 +108,7 @@ class BumpFeeTest(BitcoinTestFramework):
         # These tests wipe out a number of utxos that are expected in other tests
         test_small_output_with_feerate_succeeds(self, rbf_node, dest_address)
         test_no_more_inputs_fails(self, rbf_node, dest_address)
+        self.test_bump_back_to_yourself()
 
         # Context independent tests
         test_feerate_checks_replaced_outputs(self, rbf_node, peer_node)
@@ -171,6 +176,54 @@ class BumpFeeTest(BitcoinTestFramework):
 
         self.clear_mempool()
 
+    def test_bump_back_to_yourself(self):
+        self.log.info("Test that bumpfee can send coins back to yourself")
+        node = self.nodes[1]
+
+        node.createwallet("back_to_yourself")
+        wallet = node.get_wallet_rpc("back_to_yourself")
+
+        # Make 3 UTXOs
+        addr = wallet.getnewaddress()
+        for _ in range(3):
+            self.nodes[0].sendtoaddress(addr, 5)
+        self.generate(self.nodes[0], 1)
+
+        # Create a tx with two outputs. recipient and change.
+        tx = wallet.send(outputs={wallet.getnewaddress(): 9}, fee_rate=2)
+        tx_info = wallet.gettransaction(txid=tx["txid"], verbose=True)
+        assert_equal(len(tx_info["decoded"]["vout"]), 2)
+        assert_equal(len(tx_info["decoded"]["vin"]), 2)
+
+        # Bump tx, send coins back to change address.
+        change_addr = get_change_address(tx["txid"], wallet)[0]
+        out_amount = 10
+        bumped = wallet.bumpfee(txid=tx["txid"], options={"fee_rate": 20, "outputs": [{change_addr: out_amount}]})
+        bumped_tx = wallet.gettransaction(txid=bumped["txid"], verbose=True)
+        assert_equal(len(bumped_tx["decoded"]["vout"]), 1)
+        assert_equal(len(bumped_tx["decoded"]["vin"]), 2)
+        assert_equal(bumped_tx["decoded"]["vout"][0]["value"] + bumped["fee"], out_amount)
+
+        # Bump tx again, now test send fewer coins back to change address.
+        out_amount = 6
+        bumped = wallet.bumpfee(txid=bumped["txid"], options={"fee_rate": 40, "outputs": [{change_addr: out_amount}]})
+        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 any(txout['value'] == out_amount - bumped["fee"] and txout['scriptPubKey']['address'] == change_addr for txout in bumped_tx['decoded']['vout'])
+        # Check that total out amount is still equal to the previously bumped tx
+        assert_equal(bumped_tx["decoded"]["vout"][0]["value"] + bumped_tx["decoded"]["vout"][1]["value"] + bumped["fee"], 10)
+
+        # Bump tx again, send more coins back to change address. The process will add another input to cover the target.
+        out_amount = 12
+        bumped = wallet.bumpfee(txid=bumped["txid"], options={"fee_rate": 80, "outputs": [{change_addr: out_amount}]})
+        bumped_tx = wallet.gettransaction(txid=bumped["txid"], verbose=True)
+        assert_equal(len(bumped_tx["decoded"]["vout"]), 2)
+        assert_equal(len(bumped_tx["decoded"]["vin"]), 3)
+        assert any(txout['value'] == out_amount - bumped["fee"] and txout['scriptPubKey']['address'] == change_addr for txout in bumped_tx['decoded']['vout'])
+        assert_equal(bumped_tx["decoded"]["vout"][0]["value"] + bumped_tx["decoded"]["vout"][1]["value"] + bumped["fee"], 15)
+
+        node.unloadwallet("back_to_yourself")
 
 def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
     self.log.info('Test simple bumpfee: {}'.format(mode))
@@ -635,21 +688,16 @@ def test_locked_wallet_fails(self, rbf_node, dest_address):
 def test_change_script_match(self, rbf_node, dest_address):
     self.log.info('Test that the same change addresses is used for the replacement transaction when possible')
 
-    def get_change_address(tx):
-        tx_details = rbf_node.getrawtransaction(tx, 1)
-        txout_addresses = [txout['scriptPubKey']['address'] for txout in tx_details["vout"]]
-        return [address for address in txout_addresses if rbf_node.getaddressinfo(address)["ischange"]]
-
     # Check that there is only one change output
     rbfid = spend_one_input(rbf_node, dest_address)
-    change_addresses = get_change_address(rbfid)
+    change_addresses = get_change_address(rbfid, rbf_node)
     assert_equal(len(change_addresses), 1)
 
     # Now find that address in each subsequent tx, and no other change
     bumped_total_tx = rbf_node.bumpfee(rbfid, {"fee_rate": ECONOMICAL})
-    assert_equal(change_addresses, get_change_address(bumped_total_tx['txid']))
+    assert_equal(change_addresses, get_change_address(bumped_total_tx['txid'], rbf_node))
     bumped_rate_tx = rbf_node.bumpfee(bumped_total_tx["txid"])
-    assert_equal(change_addresses, get_change_address(bumped_rate_tx['txid']))
+    assert_equal(change_addresses, get_change_address(bumped_rate_tx['txid'], rbf_node))
     self.clear_mempool()