test: add functional test for upgradewallet rpc #32803

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.

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

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

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

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

<!--5faf32d7da4f0f540f40219e4f7537a3-->

<!--85328a0da195eb286784d51f73fa0af9-->

🚧 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.}

<details><summary>Hints</summary>

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.

</details>

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?

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.

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

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

    def skip_test_if_missing_module(self):
        self.skip_if_no_wallet()

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

closing in favour of #32944