test: add functional test for upgradewallet rpc #32803

1. 
   Prabhat1308 commented at 3:20 pm on June 24, 2025: contributor

   followups up from #32438 (comment) to add coverage for upgradewallet rpc. PR adds 2 test cases , trying to upgrade from the latest version and asserting it stays the same and trying to downgrade from the latest version. Test case to actually upgrade is not possible currently since the createwallet rpc by default creates wallet with version FEATURE_LATEST which is the highest.
2. DrahtBot added the label Tests on Jun 24, 2025
3. 
   DrahtBot commented at 3:20 pm on June 24, 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/32803.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

4. Prabhat1308 force-pushed on Jun 24, 2025
5. DrahtBot added the label CI failed on Jun 24, 2025
6. 
   DrahtBot commented at 3:28 pm on June 24, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/44696601189 LLM reason (✨ experimental): Lint check failed due to trailing whitespace and missing trailing newline in the code.

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

7. Prabhat1308 force-pushed on Jun 24, 2025
8. 
   maflcko commented at 3:34 pm on June 24, 2025: member

   are there any plans that the rpc will be used anytime soon in the next couple of years? If not, it could make sense to just remove it, and add it back, in the unlikely case it will be used?
9. 
   Prabhat1308 commented at 4:23 pm on June 24, 2025: contributor

   are there any plans that the rpc will be used anytime soon in the next couple of years? If not, it could make sense to just remove it, and add it back, in the unlikely case it will be used?

   Looking through the code this is pretty much a dead rpc where no one can use it to actually upgrade wallet and there is only 1 check that multiwallet can’t call it before this PR . I am not very sure about what the future plans are for wallet but the only case where I see this being used is if there is a need to upgradewallet version to quantum-compatible wallets. Other than that I see no future use case of the rpc in the near future.

10. 
    Prabhat1308 commented at 4:59 pm on June 24, 2025: contributor

    I am not so sure why the CI fails to call createwallet for the added test while works on other rpc tests . Will try to debug
11. 
    pablomartin4btc commented at 6:03 pm on June 24, 2025: member

    You need to add this (CI “no wallet…” failure) to test/functional/rpc_upgradewallet.py:

    0    def skip_test_if_missing_module(self):
    1        self.skip_if_no_wallet()
    

12. Prabhat1308 force-pushed on Jun 24, 2025
13. Prabhat1308 force-pushed on Jun 24, 2025
14. add functional test for upgradewallet rpc

    Adds functional test to add coverage for upgradewallet rpc .

    cead0fa5fa
15. Prabhat1308 force-pushed on Jun 24, 2025
16. 
    pablomartin4btc commented at 6:44 pm on June 24, 2025: member

    As @maflcko mentioned, not sure how useful would be to keep the RPC, and maybe you can create the PR for its removal. I thought perhaps you could move this test to wallet_backwards_compatibility.py, but again not sure about its purpose.

    cc @achow101, @furszy

17. DrahtBot removed the label CI failed on Jun 24, 2025

Contributors
Prabhat1308 DrahtBot maflcko pablomartin4btc

Labels
Tests