Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#28540: tests: Fix wallet_resendwallettransactio…
Browse files Browse the repository at this point in the history
…ns.py intermittent failure by using manual bumps instead of bumpfee

b5a9625 tests: Use manual bumps instead of bumpfee for resendwallettransactions (Andrew Chow)

Pull request description:

  Bumpfee will try to increase the entire package to the target feerate, which causes repeated bumpfees to quickly shoot up in fees, causing intermittent failures when the fee is too large. We don't care about this property, just that the child is continuously replaced until we observe it's position in mapWallet is before its parent. Instead of using bumpfee, we can create raw transactions which have only pay (just above) the additional incremental relay fee, thus avoiding this problem.

  Fixes #28491

ACKs for top commit:
  kevkevinpal:
    ACK [b5a9625](bitcoin/bitcoin@b5a9625)
  mzumsande:
    Code review ACK b5a9625
  pablomartin4btc:
    ACK b5a9625 -> adding the `try_rpc` to avoid (skip) any possible failure around the manual bump fee (if we ever reach it as [explained](bitcoin/bitcoin#28540 (comment))) makes a lot of sense as the spirit of the test is the tx (child before parent) sort in the `mapWallet` (as also [explained](bitcoin/bitcoin#28491 (comment))).
  MarcoFalke:
    lgtm ACK b5a9625

Tree-SHA512: f184f11c73be0c30753181901f51a3b4b9c4135e0c4681e9f4ca94692c49bac15c91683c85266a2124333c8593e9919bfd9102724616faab299740f2eb98741f
  • Loading branch information
achow101 committed Sep 28, 2023
2 parents 6619d6a + b5a9625 commit 9d5150a
Showing 1 changed file with 23 additions and 3 deletions.
26 changes: 23 additions & 3 deletions test/functional/wallet_resendwallettransactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
"""Test that the wallet resends transactions periodically."""
import time

from decimal import Decimal

from test_framework.blocktools import (
create_block,
create_coinbase,
Expand All @@ -15,6 +17,8 @@
from test_framework.util import (
assert_equal,
assert_raises_rpc_error,
get_fee,
try_rpc,
)

class ResendWalletTransactionsTest(BitcoinTestFramework):
Expand Down Expand Up @@ -86,18 +90,34 @@ def run_test(self):
# ordering of mapWallet is, if the child is not before the parent, we will create a new
# child (via bumpfee) and remove the old child (via removeprunedfunds) until we get the
# ordering of child before parent.
child_txid = node.send(outputs=[{addr: 0.5}], inputs=[{"txid":txid, "vout":0}])["txid"]
child_inputs = [{"txid": txid, "vout": 0}]
child_txid = node.sendall(recipients=[addr], inputs=child_inputs)["txid"]
# Get the child tx's info for manual bumping
child_tx_info = node.gettransaction(txid=child_txid, verbose=True)
child_output_value = child_tx_info["decoded"]["vout"][0]["value"]
# Include an additional 1 vbyte buffer to handle when we have a smaller signature
additional_child_fee = get_fee(child_tx_info["decoded"]["vsize"] + 1, Decimal(0.00001100))
while True:
txids = node.listreceivedbyaddress(minconf=0, address_filter=addr)[0]["txids"]
if txids == [child_txid, txid]:
break
bumped = node.bumpfee(child_txid)
# Manually bump the tx
# The inputs and the output address stay the same, just changing the amount for the new fee
child_output_value -= additional_child_fee
bumped_raw = node.createrawtransaction(inputs=child_inputs, outputs=[{addr: child_output_value}])
bumped = node.signrawtransactionwithwallet(bumped_raw)
bumped_txid = node.decoderawtransaction(bumped["hex"])["txid"]
# Sometimes we will get a signature that is a little bit shorter than we expect which causes the
# feerate to be a bit higher, then the followup to be a bit lower. This results in a replacement
# that can't be broadcast. We can just skip that and keep grinding.
if try_rpc(-26, "insufficient fee, rejecting replacement", node.sendrawtransaction, bumped["hex"]):
continue
# The scheduler queue creates a copy of the added tx after
# send/bumpfee and re-adds it to the wallet (undoing the next
# removeprunedfunds). So empty the scheduler queue:
node.syncwithvalidationinterfacequeue()
node.removeprunedfunds(child_txid)
child_txid = bumped["txid"]
child_txid = bumped_txid
entry_time = node.getmempoolentry(child_txid)["time"]

block_time = entry_time + 6 * 60
Expand Down

0 comments on commit 9d5150a

Please sign in to comment.