rpc, wallet: Expose database format in getwalletinfo #20125

1. 
   promag commented at 10:26 PM on October 11, 2020: member

   Support for sqlite based wallets was added in #19077. This PR adds the format key in getwalletinfo response, that can be bdb or sqlite.

2. fanquake added the label Wallet on Oct 11, 2020
3. 
   DrahtBot commented at 6:31 AM on October 12, 2020: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #18904 (Don't call lsn_reset in periodic flush by bvbfan)

   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.

4. DrahtBot cross-referenced this on Oct 12, 2020 from issue wallet: Unify wallet directory lock error message by hebasto
5. DrahtBot cross-referenced this on Oct 12, 2020 from issue refactor: Some wallet cleanups by promag
6. DrahtBot cross-referenced this on Oct 12, 2020 from issue wallet: let Listwalletdir do not iterate through our blocksdata. by Saibato
7. DrahtBot cross-referenced this on Oct 12, 2020 from issue refactor: remove ::vpwallets and related global variables by ryanofsky
8. DrahtBot cross-referenced this on Oct 12, 2020 from issue wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets by achow101
9. DrahtBot cross-referenced this on Oct 12, 2020 from issue Don't call lsn_reset in periodic flush by bvbfan
10. DrahtBot cross-referenced this on Oct 12, 2020 from issue build: optionally skip external warnings by vasild
11. DrahtBot cross-referenced this on Oct 12, 2020 from issue Fix crashes and infinite loop in ListWalletDir() by uhliksk
12. DrahtBot cross-referenced this on Oct 12, 2020 from issue net: Add NAT-PMP port forwarding support by hebasto
13. 
    achow101 commented at 3:04 PM on October 12, 2020: member

    Concept ACK

14. hebasto approved
15. 
    hebasto commented at 3:49 PM on October 12, 2020: member

    Approach ACK 56e1b81ef8efdcfe26b6fe22fea5354952afcead

    Mind making the new Format member function const?

16. 
    promag commented at 3:52 PM on October 12, 2020: member

    Mind making the new Format member function const? @ryanofsky fine by you?

17. DrahtBot cross-referenced this on Oct 12, 2020 from issue Wallet: remove db mode string by S3RK
18. DrahtBot cross-referenced this on Oct 12, 2020 from issue Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks by luke-jr
19. rpc, wallet: Expose database format in getwalletinfo 5e737a0092
20. promag force-pushed on Oct 14, 2020
21. 
    jonatack commented at 10:18 PM on October 14, 2020: contributor

    Concept/Approach ACK, this was the first thing I wanted to see when testing #19077. Looks like it's only missing tests. Maybe also add it to -getinfo in single-wallet mode or when -rpcwallet= is passed.

22. 
    jonatack commented at 10:38 PM on October 14, 2020: contributor

    Tested ACK modulo test coverage

    RPC results

    $ ./src/bitcoin-cli -rpcwallet= getwalletinfo
    {
      "walletname": "",
      "walletversion": 169900,
      "format": "bdb",
    ...
    

    $ ./src/bitcoin-cli -rpcwallet=sqlite getwalletinfo
    {
      "walletname": "sqlite",
      "walletversion": 169900,
      "format": "sqlite",
    ...
    

    help

    Result:
    {                                         (json object)
      "walletname" : "str",                   (string) the wallet name
      "walletversion" : n,                    (numeric) the wallet version
      "format" : "str",                       (string) the database format (bdb or sqlite)
    ...
    

23. 
    promag commented at 12:39 AM on October 15, 2020: member

    @jonatack thanks for testing, will pick this after base is merged.

24. promag marked this as ready for review on Oct 15, 2020
25. promag force-pushed on Oct 15, 2020
26. 
    promag commented at 2:20 PM on October 15, 2020: member

    Rebased.

27. test: add coverage for getwalletinfo format field 624bab00dd
28. 
    jonatack commented at 2:49 PM on October 15, 2020: contributor

    Test coverage commit at https://github.com/jonatack/bitcoin/commits/test-getwalletinfo-format-field, feel free to pull in or use.

29. promag force-pushed on Oct 15, 2020
30. 
    promag commented at 3:01 PM on October 15, 2020: member

    Thanks @jonatack, included as is.

31. 
    jonatack commented at 3:10 PM on October 15, 2020: contributor

    Thanks @promag.

    Tested ACK 624bab00dd2cc8e2ebd77dc0a669bc8d507c3721

32. 
    jonatack commented at 3:14 PM on October 15, 2020: contributor

    Will need a release note here or added manually in https://github.com/bitcoin-core/bitcoin-devwiki/wiki.

33. hebasto approved
34. 
    hebasto commented at 4:35 PM on October 15, 2020: member

    ACK 624bab00dd2cc8e2ebd77dc0a669bc8d507c3721, tested on Linux Mint 20 (x86_64).

    Tested in the GUI console :)

    Mind making the new Format member function const?

    @ryanofsky fine by you?

    Didn't get what are concerns. :smiley:

35. 
    promag commented at 4:46 PM on October 15, 2020: member

    He previously said that he dislikes interfaces that enforce const. Probably not relevant in this case.

36. laanwj added this to the milestone 0.21.0 on Oct 15, 2020
37. in src/wallet/rpcwallet.cpp:2469 in 624bab00dd

    2465 | @@ -2465,6 +2466,7 @@ static RPCHelpMan getwalletinfo()
    2466 |      int64_t kp_oldest = pwallet->GetOldestKeyPoolTime();
    2467 |      obj.pushKV("walletname", pwallet->GetName());
    2468 |      obj.pushKV("walletversion", pwallet->GetVersion());
    2469 | +    obj.pushKV("format", pwallet->GetDatabase().Format());
    

    ---

    ---

    

    jonatack commented at 7:38 PM on October 15, 2020:

    Not sure, since it is related to the "descriptors" bool field, maybe place "format" at the end right after "descriptors". IDK.

    ---

    

    promag commented at 7:47 PM on October 15, 2020:

    For now it's related. But note that you can have descriptor wallets in bdb format.

    Actually you can have a bdb descriptors wallet if you use a build before #19077.

38. 
    laanwj commented at 9:52 AM on October 16, 2020: member

    I think this information is mildly useful for troubleshooting user issues, and for testing. Code review ACK 624bab00dd2cc8e2ebd77dc0a669bc8d507c3721.

39. 
    MarcoFalke commented at 9:56 AM on October 16, 2020: member

    How would it be possible to have a legacy-sqlite wallet or a descriptor-bdb wallet? This should only be useful for developers that ran master?

40. 
    MarcoFalke commented at 9:58 AM on October 16, 2020: member

    No objection obviously.

    doesn't hurt ACK 624bab00dd2cc8e2ebd77dc0a669bc8d507c3721

41. 
    meshcollider commented at 10:50 PM on October 19, 2020: contributor

    utACK 624bab00dd2cc8e2ebd77dc0a669bc8d507c3721

42. meshcollider merged this on Oct 19, 2020
43. meshcollider closed this on Oct 19, 2020

    ---

44. promag deleted the branch on Oct 19, 2020
45. sidhujag referenced this in commit b01329c2ec on Oct 19, 2020
46. MarcoFalke cross-referenced this on Oct 21, 2020 from issue Show name, format and if uses descriptors in bitcoin-wallet tool by jonasschnelli
47. bitcoin locked this on Feb 15, 2022

Contributors

promagDrahtBotachow101hebastojonatacklaanwjMarcoFalkemeshcollider

Milestone
0.21.0