wallet: change upgradewallet return type to be an object #20282

1. 
   jnewbery commented at 8:40 am on November 2, 2020: member

   Change the return type of upgradewallet to be an object for future extensibility.

   Also return any error string returned from the UpgradeWallet() function.

2. [wallet] Return object from upgradewallet RPC 2ead31fb1b
3. fanquake added the label Wallet on Nov 2, 2020
4. fanquake added this to the milestone 0.21.0 on Nov 2, 2020
5. fanquake requested review from MarcoFalke on Nov 2, 2020
6. jnewbery renamed this:
   Wallet: Change upgradewallet return type to be an object
   wallet: Change upgradewallet return type to be an object
   on Nov 2, 2020
7. jnewbery renamed this:
   wallet: Change upgradewallet return type to be an object
   wallet: change upgradewallet return type to be an object
   on Nov 2, 2020
8. 
   MarcoFalke commented at 9:13 am on November 2, 2020: member

   ACK 2ead31fb1b17c9b183a4b81f0ae4f48e5cf67d64

   Thanks for picking this up to avoid a delay in the 0.21 release!

9. MarcoFalke added the label RPC/REST/ZMQ on Nov 2, 2020
10. 
    jonatack commented at 1:19 pm on November 2, 2020: member

    ACK, tested the upgradewallet help and output manually both for successful and failure cases. Two thoughts: (a) it might be user-friendly to provide a message that the upgrade was successful rather than an empty JSON object with a newline in the middle, e.g. {"result": "the wallet was upgraded successfully to version 16990"}, and (b) could add a functional test case for the result in case of an error.

    0$ bitcoin-cli -regtest upgradewallet 
    1{
    2}
    

    0$ bitcoin-cli -regtest upgradewallet
    1{
    2  "error": "Cannot downgrade wallet"
    3}
    

     0$ bitcoin-cli -regtest help upgradewallet 
     1upgradewallet ( version )
     2
     3Upgrade the wallet. Upgrades to the latest version if no version number is specified
     4New keys may be generated and a new wallet backup will need to be made.
     5Arguments:
     61. version    (numeric, optional, default=169900) The version number to upgrade to. Default is the latest wallet version
     7
     8Result:
     9{                     (json object)
    10  "error" : "str"     (string, optional) Error message (if there is one)
    11}
    12
    13Examples:
    14> bitcoin-cli upgradewallet 169900
    15> curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "upgradewallet", "params": [169900]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
    

11. 
    MarcoFalke commented at 1:22 pm on November 2, 2020: member

    a) seems like a feature, so should be a separate pull b) seems like a test. Can go in anytime (in this pull or any other pull, no strong opinion)
12. 
    DrahtBot commented at 3:00 pm on November 2, 2020: member

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

    Conflicts

    No conflicts as of last run.

13. 
    meshcollider commented at 1:50 am on November 4, 2020: contributor

    Tested ACK 2ead31fb1b17c9b183a4b81f0ae4f48e5cf67d64
14. meshcollider merged this on Nov 4, 2020
15. meshcollider closed this on Nov 4, 2020

    ---

16. sidhujag referenced this in commit 6d085c8930 on Nov 4, 2020
17. jnewbery deleted the branch on Nov 4, 2020
18. 
    jonatack commented at 6:23 pm on November 16, 2020: member

    a) seems like a feature, so should be a separate pull

    Done in #20403, which also fixes an issue of upgradewallet silently not upgrading.

19. MarcoFalke referenced this in commit afdfd3c8c1 on Nov 25, 2020
20. DrahtBot locked this on Feb 15, 2022

Contributors
jnewbery MarcoFalke jonatack DrahtBot meshcollider

Review Requested
MarcoFalke

Labels
Wallet RPC/REST/ZMQ

Milestone
0.21.0