test: Add Wallet Unlock Context Manager #28617

1. 
   BrandonOdiwuor commented at 12:41 PM on October 9, 2023: contributor

   Fixes #28601, see #28403 (review)

   Add Context Manager to manage the locking and unlocking of locked wallets with a passphrase during testing.

2. 
   DrahtBot commented at 12:41 PM on October 9, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	kevkevinpal, maflcko

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

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #28528 (test: Use test framework utils in functional tests by osagie98)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot added the label Tests on Oct 9, 2023
4. in test/functional/test_framework/wallet_util.py:126 in a6bded90a9 outdated

    121 | @@ -122,3 +122,21 @@ def generate_keypair(compressed=True, wif=False):
    122 |      if wif:
    123 |          privkey = bytes_to_wif(privkey.get_bytes(), compressed)
    124 |      return privkey, pubkey
    125 | +
    126 | +class WalletUnlockContextManager():
   

   ---

   ---

   

   maflcko commented at 12:54 PM on October 9, 2023:

   class WalletUnlock():
   

   nit: I don't think other context managers have it in their name, so could remove this as well, but no strong opinion, either is fine.

   ---

   

   BrandonOdiwuor commented at 1:00 PM on October 10, 2023:

   done

5. in test/functional/test_framework/wallet_util.py:131 in a6bded90a9 outdated

    126 | +class WalletUnlockContextManager():
    127 | +    """
    128 | +    A context manager for unlocking a wallet with a passphrase and automatically locking it afterward.
    129 | +    """
    130 | +
    131 | +    TIMEOUT = 999000 # WALLET_PASSPHRASE_TIMEOUT
   

   ---

   ---

   

   maflcko commented at 12:59 PM on October 9, 2023:

   What does the comment mean?

   nit: Could use the maximum allowed value?

   ---

   

   BrandonOdiwuor commented at 12:59 PM on October 10, 2023:

   done

6. maflcko approved
7. DrahtBot added the label CI failed on Oct 9, 2023
8. BrandonOdiwuor force-pushed on Oct 10, 2023
9. 
   maflcko commented at 11:22 AM on October 10, 2023: member

   I think there is one more that can be using the context:

   test/functional/wallet_fundrawtransaction.py:        wallet.walletpassphrase("test", 999000)
   

10. BrandonOdiwuor force-pushed on Oct 10, 2023
11. BrandonOdiwuor force-pushed on Oct 10, 2023
12. test: Add Wallet Unlock Context Manager

    Add Context Manager to manage wallet locking/unlocking with passphrase

    004903ebad
13. BrandonOdiwuor force-pushed on Oct 10, 2023
14. DrahtBot removed the label CI failed on Oct 10, 2023
15. BrandonOdiwuor requested review from maflcko on Oct 10, 2023
16. 
    BrandonOdiwuor commented at 12:43 PM on October 11, 2023: contributor

    @maflcko could you please review the changes

17. 
    kevkevinpal commented at 3:42 AM on October 19, 2023: contributor

    lgtm ACK 004903e

18. 
    maflcko commented at 8:38 AM on October 19, 2023: member

    lgtm ACK 004903ebade38ba47c5ddc17b756d605b963528e

19. DrahtBot removed review request from maflcko on Oct 19, 2023
20. maflcko added this to the milestone 26.0 on Oct 19, 2023
21. fanquake merged this on Oct 19, 2023
22. fanquake closed this on Oct 19, 2023

    ---

23. Frank-GER referenced this in commit f1482876a7 on Oct 21, 2023
24. 
    kashifs commented at 2:49 PM on October 22, 2023: contributor

    Hey @BrandonOdiwuor, nice work on these changes. I know that this PR has been closed, but I finally finished my review of all the files. Apologies for the delay. I'm trying to get up to speed and have to work very slowly at the moment. I learned a ton from your work and have already gotten some value from the code review, but I'm not sure of the best way to contribute these changes. They can be found here:

    https://github.com/bitcoin/bitcoin/compare/master...kashifs:bitcoin:master

    Please let me know what you think. Thanks!

25. BrandonOdiwuor deleted the branch on Nov 21, 2023
26. bitcoin locked this on Nov 20, 2024

Contributors

BrandonOdiwuorDrahtBotmaflckokevkevinpalkashifs

Milestone
26.0