wallet: No BDB creation, unless -deprecatedrpc=create_bdb #28597

1. 
   maflcko commented at 1:50 pm on October 5, 2023: member

   With BDB being removed soon, it seems confusing and harmful to allow users to create fresh BDB wallets going forward, as it would load them with an additional burden of having to migrate them soon after.

   Also, it would be good to allow for one release for test (and external) scripts to adapt.

   Fix all issues by introducing the -deprecatedrpc=create_bdb setting.

2. wallet: No BDB creation, unless -deprecatedrpc=create_bdb fa071aeb61
3. 
   DrahtBot commented at 1:50 pm on October 5, 2023: contributor

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

   Code Coverage

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	Sjors, furszy, achow101
   Concept ACK	hebasto

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #27788 (rpc: Be able to access RPC parameters by name by achow101)

   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 added the label Wallet on Oct 5, 2023
5. maflcko added this to the milestone 26.0 on Oct 5, 2023
6. maflcko force-pushed on Oct 5, 2023
7. 
   furszy commented at 2:07 pm on October 5, 2023: member

   Concept ACK.

   Maybe add a small test creating a legacy wallet without the “create_bdb” flag? asserting the error is being properly thrown.

8. 
   maflcko commented at 2:16 pm on October 5, 2023: member

   Maybe add a small test creating a legacy wallet without the “create_bdb” flag? asserting the error is being properly thrown.

   Yes, happy to push a commit or patch, if someone writes one. Otherwise, it should be possible to test locally:

   0$ ./src/bitcoin-cli -datadir=/tmp -regtest createwallet name false false "" false false
   1error code: -4
   2error message:
   3BDB wallet creation is deprecated and will be removed in a future release. In this release it can be re-enabled temporarily with the -deprecatedrpc=create_bdb setting.
   

9. 
   Sjors commented at 2:31 pm on October 5, 2023: member

   Concept ACK
10. 
    hebasto commented at 2:35 pm on October 5, 2023: member

    Concept ACK.
11. 
    Sjors commented at 3:22 pm on October 5, 2023: member

    tACK fa071aeb61dcc42cd122d3fb1abe4b9c238f8010
12. fanquake requested review from achow101 on Oct 5, 2023
13. 
    furszy commented at 4:37 pm on October 5, 2023: member

    utACK fa071aeb
14. 
    furszy commented at 4:46 pm on October 5, 2023: member

    On a second thought, probably better to call it “create_legacy_wallet” instead of “create_bdb”. Users might not be aware of the legacy wallet db engine name.
15. 
    maflcko commented at 4:49 pm on October 5, 2023: member

    Users might not be aware of the legacy wallet db engine name.

    I think BDB is mentioned in all compile docs, as well as the GUI. Maybe not in the RPC interface, but it is explained in this pull.

    Though, happy to change.

16. 
    achow101 commented at 7:29 pm on October 5, 2023: member

    ACK fa071aeb61dcc42cd122d3fb1abe4b9c238f8010
17. DrahtBot removed review request from achow101 on Oct 5, 2023
18. achow101 merged this on Oct 5, 2023
19. achow101 closed this on Oct 5, 2023

    ---

20. fanquake added the label Needs release note on Oct 6, 2023
21. maflcko deleted the branch on Oct 6, 2023
22. 
    pablomartin4btc commented at 10:02 pm on October 6, 2023: member

    post-merge ACK fa071aeb61dcc42cd122d3fb1abe4b9c238f8010
23. 
    kristapsk commented at 12:25 pm on December 7, 2023: contributor

    This should have had release notes.
24. 
    maflcko commented at 12:59 pm on December 7, 2023: member

    This should have had release notes.

    A label was added after merge, but it looks like no one went through those. Currently there are 4 tagged: https://github.com/bitcoin/bitcoin/issues?q=label%3A%22Needs+release+note%22+is%3Aclosed

25. fanquake removed the label Needs release note on Dec 7, 2023
26. 
    fanquake commented at 3:33 pm on December 7, 2023: member

    Release note being added in #29023.

Contributors
maflcko DrahtBot furszy Sjors hebasto achow101 pablomartin4btc kristapsk fanquake

Labels
Wallet

Milestone
26.0