Merge bitcoin/bitcoin#26467: bumpfee: Allow the user to choose which output is change

e8c31f135c tests: Test for bumping single output transaction (Andrew Chow)
4f4d4407e3 test: Test bumpfee reduce_output (Andrew Chow)
7d83502d3d bumpfee: Allow original change position to be specified (Andrew Chow)

Pull request description:

  When bumping the transaction fee, we will try to find the change output of the transaction in order to have an output whose value we can modify so that we can meet the target feerate. However we do not always find the change output which can cause us to unnecessarily add an additional output to the transaction. We can avoid this issue by outsourcing the determination of change to the user if they so desire.

  This PR adds a `orig_change_pos` option to bumpfee which the user can use to specify the index of the change output.

  Fixes #11233
  Fixes #20795

ACKs for top commit:
  ismaelsadeeq:
    re ACK e8c31f135c
  pinheadmz:
    re-ACK e8c31f135c
  furszy:
    Code review ACK e8c31f13

Tree-SHA512: 3a230655934af17f7c1a5953fafb5ef0d687c21355cf284d5e98fece411f589cd69ea505f06d6bdcf82836b08d268c366ad2dd30ae3d71541c9cdf94d1f698ee
This commit is contained in:
fanquake
2023-07-20 09:38:05 +01:00
5 changed files with 113 additions and 9 deletions

View File

@@ -24,9 +24,11 @@ from test_framework.messages import (
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_fee_amount,
assert_greater_than,
assert_raises_rpc_error,
get_fee,
find_vout_for_address,
)
from test_framework.wallet import MiniWallet
@@ -109,6 +111,8 @@ class BumpFeeTest(BitcoinTestFramework):
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()
self.test_provided_change_pos(rbf_node)
self.test_single_output()
# Context independent tests
test_feerate_checks_replaced_outputs(self, rbf_node, peer_node)
@@ -174,6 +178,13 @@ class BumpFeeTest(BitcoinTestFramework):
assert_raises_rpc_error(-8, "Invalid parameter, duplicate key: data",
rbf_node.bumpfee, rbfid, {"outputs": [{"data": "deadbeef"}, {"data": "deadbeef"}]})
self.log.info("Test reduce_output option")
assert_raises_rpc_error(-1, "JSON integer out of range", rbf_node.bumpfee, rbfid, {"reduce_output": -1})
assert_raises_rpc_error(-8, "Change position is out of range", rbf_node.bumpfee, rbfid, {"reduce_output": 2})
self.log.info("Test outputs and reduce_output cannot both be provided")
assert_raises_rpc_error(-8, "Cannot specify both new outputs to use and an output index to reduce", rbf_node.bumpfee, rbfid, {"reduce_output": 2, "outputs": [{dest_address: 0.1}]})
self.clear_mempool()
def test_bump_back_to_yourself(self):
@@ -225,6 +236,72 @@ class BumpFeeTest(BitcoinTestFramework):
node.unloadwallet("back_to_yourself")
def test_provided_change_pos(self, rbf_node):
self.log.info("Test the reduce_output option")
change_addr = rbf_node.getnewaddress()
dest_addr = rbf_node.getnewaddress()
assert_equal(rbf_node.getaddressinfo(change_addr)["ischange"], False)
assert_equal(rbf_node.getaddressinfo(dest_addr)["ischange"], False)
send_res = rbf_node.send(outputs=[{dest_addr: 1}], options={"change_address": change_addr})
assert send_res["complete"]
txid = send_res["txid"]
tx = rbf_node.gettransaction(txid=txid, verbose=True)
assert_equal(len(tx["decoded"]["vout"]), 2)
change_pos = find_vout_for_address(rbf_node, txid, change_addr)
change_value = tx["decoded"]["vout"][change_pos]["value"]
bumped = rbf_node.bumpfee(txid, {"reduce_output": change_pos})
new_txid = bumped["txid"]
new_tx = rbf_node.gettransaction(txid=new_txid, verbose=True)
assert_equal(len(new_tx["decoded"]["vout"]), 2)
new_change_pos = find_vout_for_address(rbf_node, new_txid, change_addr)
new_change_value = new_tx["decoded"]["vout"][new_change_pos]["value"]
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(-4, "The transaction amount is too small to pay the fee", 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)
wallet.unloadwallet()
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)