mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-02-02 09:46:52 -05:00
Merge #18311: Bumpfee test fix
f1b4503114bumpfee test: exit loop at proper time with new fee value being compared (Gregory Sanders)2e4edc68f9Add some test logging to wallet_bumpfee.py (Gregory Sanders) Pull request description: In the loop we accidentally used `origfee` which is not the value to check, and also allowed the loop to exit too early since the new fee must be strictly greater than `0.0005`. Also converted/added a bunch of logging from comments. Resolves https://github.com/bitcoin/bitcoin/issues/17716 ACKs for top commit: MarcoFalke: ACKf1b4503114🏈 Tree-SHA512: eb73297fc82b09b9ec08d85ba3f0bec662119d0ff63ccf5d978a7bad6a674b5915f5ed021ec42f72a732c9ee7af43212d1de87361f50a970df7755caec96f6d8
This commit is contained in:
MarcoFalke
commit
b5c7665e30
1 changed files with 62 additions and 46 deletions
|
|
@ -71,28 +71,29 @@ class BumpFeeTest(BitcoinTestFramework):
|
|||
test_simple_bumpfee_succeeds(self, "default", rbf_node, peer_node, dest_address)
|
||||
test_simple_bumpfee_succeeds(self, "fee_rate", rbf_node, peer_node, dest_address)
|
||||
test_feerate_args(self, rbf_node, peer_node, dest_address)
|
||||
test_segwit_bumpfee_succeeds(rbf_node, dest_address)
|
||||
test_nonrbf_bumpfee_fails(peer_node, dest_address)
|
||||
test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address)
|
||||
test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address)
|
||||
test_small_output_fails(rbf_node, dest_address)
|
||||
test_dust_to_fee(rbf_node, dest_address)
|
||||
test_settxfee(rbf_node, dest_address)
|
||||
test_segwit_bumpfee_succeeds(self, rbf_node, dest_address)
|
||||
test_nonrbf_bumpfee_fails(self, peer_node, dest_address)
|
||||
test_notmine_bumpfee_fails(self, rbf_node, peer_node, dest_address)
|
||||
test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_address)
|
||||
test_small_output_fails(self, rbf_node, dest_address)
|
||||
test_dust_to_fee(self, rbf_node, dest_address)
|
||||
test_settxfee(self, rbf_node, dest_address)
|
||||
test_watchonly_psbt(self, peer_node, rbf_node, dest_address)
|
||||
test_rebumping(rbf_node, dest_address)
|
||||
test_rebumping_not_replaceable(rbf_node, dest_address)
|
||||
test_unconfirmed_not_spendable(rbf_node, rbf_node_address)
|
||||
test_bumpfee_metadata(rbf_node, dest_address)
|
||||
test_locked_wallet_fails(rbf_node, dest_address)
|
||||
test_change_script_match(rbf_node, dest_address)
|
||||
test_rebumping(self, rbf_node, dest_address)
|
||||
test_rebumping_not_replaceable(self, rbf_node, dest_address)
|
||||
test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address)
|
||||
test_bumpfee_metadata(self, rbf_node, dest_address)
|
||||
test_locked_wallet_fails(self, rbf_node, dest_address)
|
||||
test_change_script_match(self, rbf_node, dest_address)
|
||||
test_maxtxfee_fails(self, rbf_node, dest_address)
|
||||
# These tests wipe out a number of utxos that are expected in other tests
|
||||
test_small_output_with_feerate_succeeds(rbf_node, dest_address)
|
||||
test_no_more_inputs_fails(rbf_node, dest_address)
|
||||
test_small_output_with_feerate_succeeds(self, rbf_node, dest_address)
|
||||
test_no_more_inputs_fails(self, rbf_node, dest_address)
|
||||
self.log.info("Success")
|
||||
|
||||
|
||||
def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
|
||||
self.log.info('Test simple bumpfee')
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
rbftx = rbf_node.gettransaction(rbfid)
|
||||
self.sync_mempools((rbf_node, peer_node))
|
||||
|
|
@ -119,6 +120,7 @@ def test_simple_bumpfee_succeeds(self, mode, rbf_node, peer_node, dest_address):
|
|||
assert_equal(bumpedwtx["replaces_txid"], rbfid)
|
||||
|
||||
def test_feerate_args(self, rbf_node, peer_node, dest_address):
|
||||
self.log.info('Test feerate args')
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
self.sync_mempools((rbf_node, peer_node))
|
||||
assert rbfid in rbf_node.getrawmempool() and rbfid in peer_node.getrawmempool()
|
||||
|
|
@ -135,7 +137,8 @@ def test_feerate_args(self, rbf_node, peer_node, dest_address):
|
|||
assert_raises_rpc_error(-4, "is too high (cannot be higher than", rbf_node.bumpfee, rbfid, {"fee_rate":1})
|
||||
|
||||
|
||||
def test_segwit_bumpfee_succeeds(rbf_node, dest_address):
|
||||
def test_segwit_bumpfee_succeeds(self, rbf_node, dest_address):
|
||||
self.log.info('Test that segwit-sourcing bumpfee works')
|
||||
# Create a transaction with segwit output, then create an RBF transaction
|
||||
# which spends it, and make sure bumpfee can be called on it.
|
||||
|
||||
|
|
@ -165,14 +168,14 @@ def test_segwit_bumpfee_succeeds(rbf_node, dest_address):
|
|||
assert rbfid not in rbf_node.getrawmempool()
|
||||
|
||||
|
||||
def test_nonrbf_bumpfee_fails(peer_node, dest_address):
|
||||
# cannot replace a non RBF transaction (from node which did not enable RBF)
|
||||
def test_nonrbf_bumpfee_fails(self, peer_node, dest_address):
|
||||
self.log.info('Test that we cannot replace a non RBF transaction')
|
||||
not_rbfid = peer_node.sendtoaddress(dest_address, Decimal("0.00090000"))
|
||||
assert_raises_rpc_error(-4, "not BIP 125 replaceable", peer_node.bumpfee, not_rbfid)
|
||||
|
||||
|
||||
def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address):
|
||||
# cannot bump fee unless the tx has only inputs that we own.
|
||||
def test_notmine_bumpfee_fails(self, rbf_node, peer_node, dest_address):
|
||||
self.log.info('Test that it cannot bump fee if non-owned inputs are included')
|
||||
# here, the rbftx has a peer_node coin and then adds a rbf_node input
|
||||
# Note that this test depends upon the RPC code checking input ownership prior to change outputs
|
||||
# (since it can't use fundrawtransaction, it lacks a proper change output)
|
||||
|
|
@ -192,8 +195,8 @@ def test_notmine_bumpfee_fails(rbf_node, peer_node, dest_address):
|
|||
rbf_node.bumpfee, rbfid)
|
||||
|
||||
|
||||
def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address):
|
||||
# cannot bump fee if the transaction has a descendant
|
||||
def test_bumpfee_with_descendant_fails(self, rbf_node, rbf_node_address, dest_address):
|
||||
self.log.info('Test that fee cannot be bumped when it has descendant')
|
||||
# parent is send-to-self, so we don't have to check which output is change when creating the child tx
|
||||
parent_id = spend_one_input(rbf_node, rbf_node_address)
|
||||
tx = rbf_node.createrawtransaction([{"txid": parent_id, "vout": 0}], {dest_address: 0.00020000})
|
||||
|
|
@ -201,7 +204,8 @@ def test_bumpfee_with_descendant_fails(rbf_node, rbf_node_address, dest_address)
|
|||
rbf_node.sendrawtransaction(tx["hex"])
|
||||
assert_raises_rpc_error(-8, "Transaction has descendants in the wallet", rbf_node.bumpfee, parent_id)
|
||||
|
||||
def test_small_output_fails(rbf_node, dest_address):
|
||||
def test_small_output_fails(self, rbf_node, dest_address):
|
||||
self.log.info('Test totalFee bump with small output fails')
|
||||
# cannot bump fee with a too-small output
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
rbf_node.bumpfee(rbfid, {"totalFee": 50000})
|
||||
|
|
@ -209,7 +213,8 @@ def test_small_output_fails(rbf_node, dest_address):
|
|||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
assert_raises_rpc_error(-4, "Change output is too small", rbf_node.bumpfee, rbfid, {"totalFee": 50001})
|
||||
|
||||
def test_small_output_with_feerate_succeeds(rbf_node, dest_address):
|
||||
def test_small_output_with_feerate_succeeds(self, rbf_node, dest_address):
|
||||
self.log.info('Testing small output with feerate bump succeeds')
|
||||
|
||||
# Make sure additional inputs exist
|
||||
rbf_node.generatetoaddress(101, rbf_node.getnewaddress())
|
||||
|
|
@ -217,9 +222,9 @@ def test_small_output_with_feerate_succeeds(rbf_node, dest_address):
|
|||
input_list = rbf_node.getrawtransaction(rbfid, 1)["vin"]
|
||||
assert_equal(len(input_list), 1)
|
||||
original_txin = input_list[0]
|
||||
# Keep bumping until we out-spend change output
|
||||
self.log.info('Keep bumping until transaction fee out-spends non-destination value')
|
||||
tx_fee = 0
|
||||
while tx_fee < Decimal("0.0005"):
|
||||
while True:
|
||||
input_list = rbf_node.getrawtransaction(rbfid, 1)["vin"]
|
||||
new_item = list(input_list)[0]
|
||||
assert_equal(len(input_list), 1)
|
||||
|
|
@ -231,7 +236,11 @@ def test_small_output_with_feerate_succeeds(rbf_node, dest_address):
|
|||
assert rbfid not in raw_pool
|
||||
assert rbfid_new in raw_pool
|
||||
rbfid = rbfid_new
|
||||
tx_fee = rbfid_new_details["origfee"]
|
||||
tx_fee = rbfid_new_details["fee"]
|
||||
|
||||
# Total value from input not going to destination
|
||||
if tx_fee > Decimal('0.00050000'):
|
||||
break
|
||||
|
||||
# input(s) have been added
|
||||
final_input_list = rbf_node.getrawtransaction(rbfid, 1)["vin"]
|
||||
|
|
@ -244,8 +253,8 @@ def test_small_output_with_feerate_succeeds(rbf_node, dest_address):
|
|||
rbf_node.generatetoaddress(1, rbf_node.getnewaddress())
|
||||
assert_equal(rbf_node.gettransaction(rbfid)["confirmations"], 1)
|
||||
|
||||
def test_dust_to_fee(rbf_node, dest_address):
|
||||
# check that if output is reduced to dust, it will be converted to fee
|
||||
def test_dust_to_fee(self, rbf_node, dest_address):
|
||||
self.log.info('Test that bumped output that is dust is dropped to fee')
|
||||
# the bumped tx sets fee=49,900, but it converts to 50,000
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
fulltx = rbf_node.getrawtransaction(rbfid, 1)
|
||||
|
|
@ -257,7 +266,8 @@ def test_dust_to_fee(rbf_node, dest_address):
|
|||
assert_equal(len(full_bumped_tx["vout"]), 1) # change output is eliminated
|
||||
|
||||
|
||||
def test_settxfee(rbf_node, dest_address):
|
||||
def test_settxfee(self, rbf_node, dest_address):
|
||||
self.log.info('Test settxfee')
|
||||
assert_raises_rpc_error(-8, "txfee cannot be less than min relay tx fee", rbf_node.settxfee, Decimal('0.000005'))
|
||||
assert_raises_rpc_error(-8, "txfee cannot be less than wallet min fee", rbf_node.settxfee, Decimal('0.000015'))
|
||||
# check that bumpfee reacts correctly to the use of settxfee (paytxfee)
|
||||
|
|
@ -272,17 +282,19 @@ def test_settxfee(rbf_node, dest_address):
|
|||
rbf_node.settxfee(Decimal("0.00000000")) # unset paytxfee
|
||||
|
||||
|
||||
def test_maxtxfee_fails(test, rbf_node, dest_address):
|
||||
def test_maxtxfee_fails(self, rbf_node, dest_address):
|
||||
self.log.info('Test that bumpfee fails when it hits -matxfee')
|
||||
# size of bumped transaction (p2wpkh, 1 input, 2 outputs): 141 vbytes
|
||||
# expected bumping feerate of 20 sats/vbyte => 141*20 sats = 0.00002820 btc
|
||||
test.restart_node(1, ['-maxtxfee=0.000025'] + test.extra_args[1])
|
||||
self.restart_node(1, ['-maxtxfee=0.000025'] + self.extra_args[1])
|
||||
rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
assert_raises_rpc_error(-4, "Unable to create transaction: Fee exceeds maximum configured by -maxtxfee", rbf_node.bumpfee, rbfid)
|
||||
test.restart_node(1, test.extra_args[1])
|
||||
self.restart_node(1, self.extra_args[1])
|
||||
rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
|
||||
|
||||
def test_watchonly_psbt(test, peer_node, rbf_node, dest_address):
|
||||
def test_watchonly_psbt(self, peer_node, rbf_node, dest_address):
|
||||
self.log.info('Test that PSBT is returned for bumpfee in watchonly wallets')
|
||||
priv_rec_desc = "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/0/*)#rweraev0"
|
||||
pub_rec_desc = rbf_node.getdescriptorinfo(priv_rec_desc)["descriptor"]
|
||||
priv_change_desc = "wpkh([00000001/84'/1'/0']tprv8ZgxMBicQKsPd7Uf69XL1XwhmjHopUGep8GuEiJDZmbQz6o58LninorQAfcKZWARbtRtfnLcJ5MQ2AtHcQJCCRUcMRvmDUjyEmNUWwx8UbK/1/*)#j6uzqvuh"
|
||||
|
|
@ -334,7 +346,7 @@ def test_watchonly_psbt(test, peer_node, rbf_node, dest_address):
|
|||
funding_address2 = watcher.getnewaddress(address_type='bech32')
|
||||
peer_node.sendmany("", {funding_address1: 0.001, funding_address2: 0.001})
|
||||
peer_node.generate(1)
|
||||
test.sync_all()
|
||||
self.sync_all()
|
||||
|
||||
# Create single-input PSBT for transaction to be bumped
|
||||
psbt = watcher.walletcreatefundedpsbt([], {dest_address:0.0005}, 0, {"feeRate": 0.00001}, True)['psbt']
|
||||
|
|
@ -363,24 +375,24 @@ def test_watchonly_psbt(test, peer_node, rbf_node, dest_address):
|
|||
rbf_node.unloadwallet("watcher")
|
||||
rbf_node.unloadwallet("signer")
|
||||
|
||||
def test_rebumping(rbf_node, dest_address):
|
||||
# check that re-bumping the original tx fails, but bumping the bumper succeeds
|
||||
def test_rebumping(self, rbf_node, dest_address):
|
||||
self.log.info('Test that re-bumping the original tx fails, but bumping successor works')
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 2000})
|
||||
assert_raises_rpc_error(-4, "already bumped", rbf_node.bumpfee, rbfid, {"totalFee": 3000})
|
||||
rbf_node.bumpfee(bumped["txid"], {"totalFee": 3000})
|
||||
|
||||
|
||||
def test_rebumping_not_replaceable(rbf_node, dest_address):
|
||||
# check that re-bumping a non-replaceable bump tx fails
|
||||
def test_rebumping_not_replaceable(self, rbf_node, dest_address):
|
||||
self.log.info('Test that re-bumping non-replaceable fails')
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
bumped = rbf_node.bumpfee(rbfid, {"totalFee": 10000, "replaceable": False})
|
||||
assert_raises_rpc_error(-4, "Transaction is not BIP 125 replaceable", rbf_node.bumpfee, bumped["txid"],
|
||||
{"totalFee": 20000})
|
||||
|
||||
|
||||
def test_unconfirmed_not_spendable(rbf_node, rbf_node_address):
|
||||
# check that unconfirmed outputs from bumped transactions are not spendable
|
||||
def test_unconfirmed_not_spendable(self, rbf_node, rbf_node_address):
|
||||
self.log.info('Test that unconfirmed outputs from bumped txns are not spendable')
|
||||
rbfid = spend_one_input(rbf_node, rbf_node_address)
|
||||
rbftx = rbf_node.gettransaction(rbfid)["hex"]
|
||||
assert rbfid in rbf_node.getrawmempool()
|
||||
|
|
@ -418,7 +430,8 @@ def test_unconfirmed_not_spendable(rbf_node, rbf_node_address):
|
|||
if t["txid"] == rbfid and t["address"] == rbf_node_address and t["spendable"]), 1)
|
||||
|
||||
|
||||
def test_bumpfee_metadata(rbf_node, dest_address):
|
||||
def test_bumpfee_metadata(self, rbf_node, dest_address):
|
||||
self.log.info('Test that bumped txn metadata persists to new txn record')
|
||||
assert(rbf_node.getbalance() < 49)
|
||||
rbf_node.generatetoaddress(101, rbf_node.getnewaddress())
|
||||
rbfid = rbf_node.sendtoaddress(dest_address, 49, "comment value", "to value")
|
||||
|
|
@ -428,15 +441,17 @@ def test_bumpfee_metadata(rbf_node, dest_address):
|
|||
assert_equal(bumped_wtx["to"], "to value")
|
||||
|
||||
|
||||
def test_locked_wallet_fails(rbf_node, dest_address):
|
||||
def test_locked_wallet_fails(self, rbf_node, dest_address):
|
||||
self.log.info('Test that locked wallet cannot bump txn')
|
||||
rbfid = spend_one_input(rbf_node, dest_address)
|
||||
rbf_node.walletlock()
|
||||
assert_raises_rpc_error(-13, "Please enter the wallet passphrase with walletpassphrase first.",
|
||||
rbf_node.bumpfee, rbfid)
|
||||
rbf_node.walletpassphrase(WALLET_PASSPHRASE, WALLET_PASSPHRASE_TIMEOUT)
|
||||
|
||||
def test_change_script_match(rbf_node, dest_address):
|
||||
"""Test that the same change addresses is used for the replacement transaction when possible."""
|
||||
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']['addresses'][0] for txout in tx_details["vout"]]
|
||||
|
|
@ -480,7 +495,8 @@ def submit_block_with_tx(node, tx):
|
|||
node.submitblock(block.serialize().hex())
|
||||
return block
|
||||
|
||||
def test_no_more_inputs_fails(rbf_node, dest_address):
|
||||
def test_no_more_inputs_fails(self, rbf_node, dest_address):
|
||||
self.log.info('Test that bumpfee fails when there are no available confirmed outputs')
|
||||
# feerate rbf requires confirmed outputs when change output doesn't exist or is insufficient
|
||||
rbf_node.generatetoaddress(1, dest_address)
|
||||
# spend all funds, no change output
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue