diff options
| -rw-r--r-- | src/wallet/feebumper.cpp | 16 | ||||
| -rwxr-xr-x | test/functional/wallet_bumpfee.py | 64 |
2 files changed, 72 insertions, 8 deletions
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index d127c41c43..b6b1fa1d3e 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -231,6 +231,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // is one). If outputs vector is non-empty, replace original // outputs with its contents, otherwise use original outputs. std::vector<CRecipient> recipients; + CAmount new_outputs_value = 0; const auto& txouts = outputs.empty() ? wtx.tx->vout : outputs; for (const auto& output : txouts) { if (!OutputIsChange(wallet, output)) { @@ -241,6 +242,21 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo ExtractDestination(output.scriptPubKey, change_dest); new_coin_control.destChange = change_dest; } + new_outputs_value += output.nValue; + } + + // If no recipients, means that we are sending coins to a change address + if (recipients.empty()) { + // Just as a sanity check, ensure that the change address exist + if (std::get_if<CNoDestination>(&new_coin_control.destChange)) { + errors.emplace_back(Untranslated("Unable to create transaction. Transaction must have at least one recipient")); + return Result::INVALID_PARAMETER; + } + + // Add change as recipient with SFFO flag enabled, so fees are deduced from it. + // If the output differs from the original tx output (because the user customized it) a new change output will be created. + recipients.emplace_back(CRecipient{GetScriptForDestination(new_coin_control.destChange), new_outputs_value, /*fSubtractFeeFromAmount=*/true}); + new_coin_control.destChange = CNoDestination(); } if (coin_control.m_feerate) { 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() |