test: fix two intermittent failures in wallet_basic.py #32483

1. 
   mzumsande commented at 4:05 pm on May 13, 2025: contributor

   Fixes two rare failures that happened in the CI:

   #27249: There could be a race with outstanding TxAddedToMempool notifications being applied to the soon-to-be created wallet:

   1. importdescriptors during rescan sets status to TxStateConfirmed
   2. old transactionAddedToMempool notification changes status back to TxStateInMempool
   3. If the listunspent call happens here the test will fail
   4. blockConnected notification will change the status back to TxStateConfirmed (so it’s not a persistent failure)

   I could reproduce this by adding a 100 microsecond sleep to AddToWallet(), the fix is to add a sync, so transactionAddedToMempool notifications won’t affect the new wallet anymore.

   #32456: During init, the test framework will start using rpc after the mempool was loaded. It will not wait for start() / postInitProcess or outstanding transactionAddedToMempool notifications (which would both set the status to TxStateInMempool), leading to a possible race, in which listunspent can be called while the tx is still in Inactive status.

   Can be reproduced by adding two sleeps: To init before calling start() for the chain clients, plus to transactionAddedToMempool in wallet.cpp. Prevent this by processing outstanding notifications.

   Fixes #27249 Fixes #32456

2. 
   DrahtBot commented at 4:05 pm on May 13, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32483.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	maflcko
   Concept ACK	fanquake
   Stale ACK	achow101

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. DrahtBot added the label Tests on May 13, 2025
4. mzumsande renamed this:
   test: fix two intermittent timeouts in wallet_basic.py
   test: fix two intermittent failures in wallet_basic.py
   on May 13, 2025
5. test: Fix intermittent failure in wallet_basic.py

   There could be a race with outstanding TxAddedToMempool notifications
   being applied to the soon-to-be created wallet.
   
   Fixes an intermittent timeout reproducable by adding a sleep to
   AddToWallet.

   07350e204d
6. test: fix another intermittent failure in wallet_basic.py

   During init, the test framework will start using rpc after the
   mempool was loaded. It will not wait for postInitProcess or
   outstanding transactionAddedToMempool notifications, leading to
   a possible race, in which listunspent is being called while the
   tx is still in Inactive status. Prevent this by processing
   outstanding notifications.

   e7ad86e1ca
7. mzumsande force-pushed on May 13, 2025
8. 
   achow101 commented at 5:54 pm on May 13, 2025: member

   ACK 4290a86c6022c8f63610b9c1b185ac115cefbd2c
9. fanquake added the label Needs backport (29.x) on May 13, 2025
10. 
    maflcko commented at 11:45 am on May 14, 2025: member

    review ACK e7ad86e1ca3b0b2f2795e91c2f9959486c67dd90 🎩

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: review ACK e7ad86e1ca3b0b2f2795e91c2f9959486c67dd90 🎩
    3WgNwLvB4clQ7IsdGtON1cU1KwtmoEvNlU0K6e9yxlOi3dZc7g52zNZlVg/Qfjnc2K+u1ZreEuSaj1cgnquFpBw==
    

11. DrahtBot requested review from achow101 on May 14, 2025
12. 
    fanquake commented at 12:23 pm on May 14, 2025: member

    @achow101 note that you’ve ACK’d a commit has not in this PR, but for an equivalent change.
13. fanquake merged this on May 14, 2025
14. fanquake closed this on May 14, 2025

    ---

15. fanquake referenced this in commit c966158426 on May 14, 2025
16. fanquake referenced this in commit cf034172bf on May 14, 2025
17. fanquake removed the label Needs backport (29.x) on May 14, 2025
18. 
    fanquake commented at 12:29 pm on May 14, 2025: member

    Backported to 29.x in #32292.
19. mzumsande deleted the branch on May 14, 2025

Contributors
mzumsande DrahtBot achow101 maflcko fanquake

Review Requested
achow101

Labels
Tests