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

  16. 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.

  17. lsilva01 approved
  18. 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

  19. 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:
  20. 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.

  21. 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
  22. unknown approved
  24. meshcollider commented at 2:02 PM on September 22, 2021: contributor

    Concept ACK

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

    Code review ACK 9c1052a5218e191fd23c0d9fc06f2fca34b03411

  26. theStack commented at 10:43 AM on October 11, 2021: contributor

    Concept ACK

  27. 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.

  28. 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?

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

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

  31. MarcoFalke merged this on Oct 22, 2021
  32. MarcoFalke closed this on Oct 22, 2021
  33. sidhujag referenced this in commit 47c21fec08 on Oct 22, 2021
  34. 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.

  35. bitcoinhodler referenced this in commit 2a8cb264a6 on Feb 7, 2022
  36. bitcoinhodler referenced this in commit 8e9699cb10 on Feb 9, 2022
  37. achow101 referenced this in commit 3a618c1e3b on Feb 17, 2022
  38. hmel referenced this in commit 552a03f90e on Feb 20, 2022
  39. prusnak referenced this in commit ade16d75d8 on Mar 13, 2022
  40. prusnak referenced this in commit e334426816 on Mar 14, 2022
  41. achow101 referenced this in commit 114754adf4 on Mar 16, 2022
  42. fanquake removed the label Needs release note on Sep 15, 2022
  43. 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".

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

Milestone
23.0

Linked (view graph)
#20160 Proposed Timeline for Legacy Wallet and BDB removal#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-04-22 09:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me