Make descriptor wallets by default #23002

pull achow101 wants to merge 2 commits into bitcoin:master from achow101:default-desc-wallets changing 5 files +25 −6
  1. achow101 commented at 11:11 PM on September 16, 2021: member

    Changes the default wallet type from legacy to descriptors. Descriptor wallets will now by the default type. Additionally, descriptor wallets will no longer be marked as experimental.

    This follows the timeline proposed in #20160

  2. rpc, wallet: Descriptor wallets are no longer experimental f19ad40463
  3. fanquake added the label Wallet on Sep 16, 2021
  4. fanquake commented at 1:34 AM on September 17, 2021: member

    Concept ACK

  5. fanquake added this to the milestone 23.0 on Sep 17, 2021
  6. in src/wallet/rpcwallet.cpp:2769 in 723e860ddc outdated 
    2765 | @@ -2766,12 +2766,11 @@ static RPCHelpMan createwallet()
2766 |      if (!request.params[4].isNull() && request.params[4].get_bool()) {
2767 |          flags |= WALLET_FLAG_AVOID_REUSE;
2768 |      }
2769 | -    if (!request.params[5].isNull() && request.params[5].get_bool()) {
2770 | +    if (request.params[5].isNull() || (!request.params[5].isNull() && request.params[5].get_bool())) {
    MarcoFalke commented at 6:02 AM on September 17, 2021:

    nit:

        if (request.params[5].isNull() || request.params[5].get_bool())) {
    achow101 commented at 4:28 PM on September 17, 2021:

    Done

  7. laanwj commented at 8:14 AM on September 17, 2021: member

    Concept ACK

  8. prusnak commented at 3:17 PM on September 17, 2021: contributor

    Concept ACK

  9. sipa commented at 3:18 PM on September 17, 2021: member

    Concept ACK

  10. achow101 force-pushed on Sep 17, 2021
  11. wallet: Default new wallets to descriptor wallets 9c1052a521
  12. in src/wallet/wallettool.cpp:142 in 71e0af8e6d outdated 
     138 | +        if (make_legacy && make_descriptors) {
 139 | +            tfm::format(std::cerr, "Only one of -legacy or -descriptors can be set to true, not both\n");
 140 | +            return false;
 141 | +        }
 142 | +        if (!make_legacy && !make_descriptors) {
 143 | +            tfm::format(std::cerr, "One of -legacy or -descriptors must be set to true (or ommitted)\n");
    jonatack commented at 4:43 PM on September 17, 2021:

    s/ommitted/omitted/

    achow101 commented at 5:32 PM on September 17, 2021:

    Done

  13. achow101 force-pushed on Sep 17, 2021
  14. DrahtBot commented at 2:40 AM on September 18, 2021: 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:

    • #22766 (refactor: Clarify and disable unused ArgsManager flags by ryanofsky)

    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.

  15. DrahtBot cross-referenced this on Sep 18, 2021 from issue refactor: Clarify and disable unused ArgsManager flags by ryanofsky
  16. in src/qt/forms/createwalletdialog.ui:110 in 9c1052a521 
     106 | @@ -107,6 +107,9 @@
 107 |          <property name="text">
 108 |           <string>Descriptor Wallet</string>
 109 |          </property>
 110 | +        <property name="checked">
    instagibbs commented at 5:17 AM on September 21, 2021:

    maybe switch this around and have an unchecked "legacy wallet" option here instead?

    achow101 commented at 4:47 PM on September 21, 2021:

    I think that may be a little confusing?

    laanwj commented at 11:51 AM on September 23, 2021:

    I think there's something to be said for both options, but I think the current name is slightly better. It highlights what is new instead of what is old.

    laanwj commented at 11:52 AM on September 23, 2021:

    I suppose you could have a tooltip text that explains in more detail, that there are "legacy wallets" and "descriptor wallets" and what is the difference from a user point of view.

  17. darosior commented at 5:25 PM on September 21, 2021: member

    Concept ACK. I've been using descriptor wallets in my application for a year without issue.

  18. lsilva01 approved
  19. lsilva01 commented at 6:25 PM on September 21, 2021: contributor

    Tested ACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411 on Ubuntu 20.04

  20. ghost commented at 6:26 PM on September 21, 2021: none

    Concept ACK. Tested on Pop!_OS and had only one issue.

    1. Descriptor wallet is created by default
    $ bitcoin-cli createwallet "D1"
{
  "name": "D1",
  "warning": ""
}

$ bitcoin-cli -rpcwallet=D1 getwalletinfo
{
  "walletname": "D1",
  "walletversion": 169900,
  "format": "sqlite",
  "balance": 0.00000000,
  "unconfirmed_balance": 0.00000000,
  "immature_balance": 0.00000000,
  "txcount": 0,
  "keypoolsize": 3000,
  "keypoolsize_hd_internal": 3000,
  "paytxfee": 0.00000000,
  "private_keys_enabled": true,
  "avoid_reuse": false,
  "scanning": false,
  "descriptors": true
}
    1. Descriptor wallet option is checked in GUI

    image

    1. Not sure where should I use -legacy? I tried using it with bitcoind, in bitcoin.conf and in bitcoin-cli -named createwallet. Nothing works. :warning:
  21. achow101 commented at 6:35 PM on September 21, 2021: member 
    1. Not sure where should I use `-legacy`? I tried using it with `bitcoind`, in bitcoin.conf and in `bitcoin-cli -named createwallet`. Nothing works.

    It is part of the wallet tool, bitcoin-wallet.

  22. lsilva01 commented at 6:42 PM on September 21, 2021: contributor
    1. Not sure where should I use -legacy? I tried using it with bitcoind, in bitcoin.conf and in bitcoin-cli -named createwallet. Nothing works.

    You can test with the following command:

    src/bitcoin-wallet -legacy -wallet="test_legacy" create
  23. unknown approved
  25. meshcollider commented at 2:02 PM on September 22, 2021: contributor

    Concept ACK

  26. meshcollider commented at 2:12 AM on September 30, 2021: contributor

    Code review ACK 9c1052a5218e191fd23c0d9fc06f2fca34b03411

  27. achow101 cross-referenced this on Oct 6, 2021 from issue Proposed Timeline for Legacy Wallet and BDB removal by achow101
  28. theStack commented at 10:43 AM on October 11, 2021: contributor

    Concept ACK

  29. Sjors commented at 12:10 PM on October 20, 2021: member

    Concept ACK, but I'd like to see #22364 land first, so we have tr() descriptors for all new users, and don't have to worry about upgrade paths for a while.

  30. MarcoFalke commented at 11:23 AM on October 22, 2021: member

    As long as #22364 lands before the next major release, it should be fine to merge this. master shouldn't be used for anything other than testing, so we don't have to worry about upgrade paths.

    Edit: Also, (experimental) descriptor wallets already exist, so this discussion seems unrelated to this pull request either way?

  31. MarcoFalke added the label Needs release note on Oct 22, 2021
  32. MarcoFalke commented at 11:31 AM on October 22, 2021: member

    Changing the default might need release notes, but this can be done later.

  33. MarcoFalke merged this on Oct 22, 2021
  34. MarcoFalke closed this on Oct 22, 2021
  35. sidhujag referenced this in commit 47c21fec08 on Oct 22, 2021
  36. in src/wallet/wallettool.cpp:136 in 9c1052a521 
     129 | @@ -126,7 +130,19 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command)
 130 |      if (command == "create") {
 131 |          DatabaseOptions options;
 132 |          options.require_create = true;
 133 | -        if (args.GetBoolArg("-descriptors", false)) {
 134 | +        // If -legacy is set, use it. Otherwise default to false.
 135 | +        bool make_legacy = args.GetBoolArg("-legacy", false);
 136 | +        // If neither -legacy nor -descriptors is set, default to true. If -descriptors is set, use its value.
 137 | +        bool make_descriptors = (!args.IsArgSet("-descriptors") && !args.IsArgSet("-legacy")) || (args.IsArgSet("-descriptors") && args.GetBoolArg("-descriptors", true));
    ryanofsky commented at 6:57 PM on October 25, 2021:

    This IsArgSet logic is very strange. Why is this not just:

    bool make_descriptors = args.GetBoolArg("-descriptors", true);
    achow101 commented at 9:18 PM on October 25, 2021:

    To avoid setting it to true if -legacy is set.

  37. laanwj cross-referenced this on Nov 5, 2021 from issue doc: Document use of legacy wallet in USAGE.md by laanwj
  38. bitcoinhodler referenced this in commit 2a8cb264a6 on Feb 7, 2022
  39. bitcoinhodler cross-referenced this on Feb 7, 2022 from issue docs: Update to match new default wallet type by bitcoinhodler
  40. bitcoinhodler referenced this in commit 8e9699cb10 on Feb 9, 2022
  41. unknown cross-referenced this on Feb 16, 2022 from issue wallet: ensure wallet files are not reused across chains by rodentrabies
  42. achow101 referenced this in commit 3a618c1e3b on Feb 17, 2022
  43. hmel referenced this in commit 552a03f90e on Feb 20, 2022
  44. jamesob cross-referenced this on Mar 2, 2022 from issue 0.3.0 release by jamesob
  45. jonatack cross-referenced this on Mar 10, 2022 from issue doc: update multisig-tutorial.md to descriptor wallet by default by jonatack
  46. theStack cross-referenced this on Mar 13, 2022 from issue remove explicit `descriptors=true` parameter for createwallet RPCs in functional tests by theStack
  47. prusnak referenced this in commit ade16d75d8 on Mar 13, 2022
  48. prusnak cross-referenced this on Mar 13, 2022 from issue test: remove explicit descriptors=True for createwallet RPC calls by prusnak
  49. prusnak referenced this in commit e334426816 on Mar 14, 2022
  50. achow101 referenced this in commit 114754adf4 on Mar 16, 2022
  51. stickies-v cross-referenced this on Mar 28, 2022 from issue Connection via Bitcoin RPC fails in Bitcoin v23 rc2 by ziggie1984
  52. chappjc cross-referenced this on Apr 29, 2022 from issue client/asset/btc: enable descriptor wallets for bitcoin core v23 by chappjc
  53. fanquake removed the label Needs release note on Sep 15, 2022
  54. fanquake commented at 3:17 PM on September 15, 2022: member

    This was part of 23.0 and got a release note. Removing "Needs release note".

  55. delta1 referenced this in commit dc83a5d546 on May 14, 2023
  56. bitcoin locked this on Sep 15, 2023
Contributors
achow101fanquakeMarcoFalkelaanwjprusnaksipajonatackDrahtBotinstagibbsdarosiorlsilva01ghostmeshcollidertheStackSjorsryanofsky
Labels
Wallet

Milestone
23.0

Linked (view graph)
#18554 wallet: ensure wallet files are not reused across chains#20160 Proposed Timeline for Legacy Wallet and BDB removal#22364 wallet: Make a tr() descriptor by default#24281 docs: Update to match new default wallet type#24519 doc: update multisig-tutorial.md to descriptor wallet by default#24550 remove explicit `descriptors=true` parameter for createwallet RPCs in functional tests#24551 test: remove explicit descriptors=True for createwallet RPC calls
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:53 UTC