test: [bugfix] Replace bumpfee with prioritisetransaction #26085

1. 
   MarcoFalke commented at 8:31 AM on September 14, 2022: member

   Fixes #26071 by using virtual fee instead of real fee, to bypass -maxtxfee.

2. test: [bugfix] Replace bumpfee with prioritisetransaction fae33acd50
3. test/doc: Remove unused syncwithvalidationinterfacequeue

   See https://github.com/bitcoin/bitcoin/pull/25768#discussion_r958562071
   
   Also fix doc typo from https://github.com/bitcoin/bitcoin/pull/25768#discussion_r958571943

   fa44670f9a
4. MarcoFalke added this to the milestone 24.0 on Sep 14, 2022
5. fanquake added the label Tests on Sep 14, 2022
6. 
   MarcoFalke commented at 8:41 AM on September 14, 2022: member

   Diff on master to increase the failure rate:

   diff --git a/test/functional/wallet_resendwallettransactions.py b/test/functional/wallet_resendwallettransactions.py
   index 26df0841d8..b187f08b85 100755
   --- a/test/functional/wallet_resendwallettransactions.py
   +++ b/test/functional/wallet_resendwallettransactions.py
   @@ -20,6 +20,7 @@ from test_framework.util import (
    class ResendWalletTransactionsTest(BitcoinTestFramework):
        def set_test_params(self):
            self.num_nodes = 1
   +        self.extra_args=[["-maxtxfee=0.00004450"]]
    
        def skip_test_if_missing_module(self):
            self.skip_if_no_wallet()
   

   So I guess a simpler alternative would be to increase -maxtxfee, but I am not sure how fast the wallet would run out of funds then.

7. glozow requested review from achow101 on Sep 14, 2022
8. in test/functional/wallet_resendwallettransactions.py:91 in fa44670f9a

     88 | +        # child (via nSequence-=1 and fee_delta+=200) and remove the old child (via removeprunedfunds) until we get the
     89 |          # ordering of child before parent.
     90 | -        child_txid = node.send(outputs=[{addr: 0.5}], options={"inputs": [{"txid":txid, "vout":0}]})["txid"]
     91 | +        # It is not possible to use bumpfee, as in very unlikely cases
     92 | +        # repeatedly calling it will overflow the maximum allowed -maxtxfee or
     93 | +        # run the wallet out of funds.
   

   ---

   ---

   

   glozow commented at 11:49 AM on September 14, 2022:

           # RBF the child using prioritisetransaction instead of adding fees because, in very unlikely cases,
           # it will overflow the maximum allowed -maxtxfee or
           # run the wallet out of funds.
   

9. 
   glozow commented at 11:53 AM on September 14, 2022: member

   ~cr ACK fa44670f9a2e42a9680e438f834bd8943ae58bed~

   ~Approach seems fine to me. Also think -maxtxfee would work since you'd need to bump to many BTCs of fees before you run out. At some point you run out of final nSequence numbers too, no?~

   Edit: I now see this approach doesn't really resolve the underlying issue, i.e. that it loops infinitely in the first place. It just prevents the failure for exceeding maxtxfee, which is bad, since it keeps going for longer.

10. 
    MarcoFalke commented at 12:31 PM on September 14, 2022: member

    Actually I don't like this fix. The failing test runs for 10 minutes, generating txs in a loop, https://cirrus-ci.com/task/5603470202896384?logs=ci#L3002 (wallet_resendwallettransactions.py --descriptors | ✖ Failed | 625 s).

    It might be best to just remove the while True loop completely, instead of running "forever" and then crash at the end.

11. MarcoFalke closed this on Sep 14, 2022

    ---

12. MarcoFalke deleted the branch on Sep 14, 2022
13. fanquake removed this from the milestone 24.0 on Sep 14, 2022
14. bitcoin locked this on Sep 14, 2023

Review Requested
achow101