-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: make Descriptor wallets to be created by default #7204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
e182820
e7e14e3
1cb63a8
5b3fc66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| Notable changes | ||
| =============== | ||
|
|
||
| Updated RPCs | ||
| ------------ | ||
|
|
||
|
|
||
| Changes to wallet related RPCs can be found in the Wallet section below. | ||
|
|
||
| Wallet | ||
| ------ | ||
|
|
||
| - Descriptor wallets are now the default wallet type. Newly created wallets | ||
| will use descriptors unless `descriptors=false` is set during `createwallet`, or | ||
| the `Descriptor wallet` checkbox is unchecked in the GUI. (dash#7204) | ||
knst marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Note that wallet RPC commands like `importmulti` and `dumpprivkey` cannot be | ||
| used with descriptor wallets, so if your client code relies on these commands | ||
| without specifying `descriptors=false` during wallet creation, you will need | ||
| to update your code. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -134,6 +134,10 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) | |
| tfm::format(std::cerr, "The -descriptors option can only be used with the 'create' command.\n"); | ||
| return false; | ||
| } | ||
| if (args.IsArgSet("-legacy") && command != "create") { | ||
| tfm::format(std::cerr, "The -legacy option can only be used with the 'create' command.\n"); | ||
| return false; | ||
| } | ||
| if (command == "create" && !args.IsArgSet("-wallet")) { | ||
| tfm::format(std::cerr, "Wallet name must be provided when creating a new wallet.\n"); | ||
| return false; | ||
|
|
@@ -145,7 +149,19 @@ bool ExecuteWalletToolFunc(const ArgsManager& args, const std::string& command) | |
| DatabaseOptions options; | ||
| ReadDatabaseArgs(args, options); | ||
| options.require_create = true; | ||
| if (args.GetBoolArg("-descriptors", false)) { | ||
| // If -legacy is set, use it. Otherwise default to false. | ||
| bool make_legacy = args.GetBoolArg("-legacy", false); | ||
| // If neither -legacy nor -descriptors is set, default to true. If -descriptors is set, use its value. | ||
| bool make_descriptors = (!args.IsArgSet("-descriptors") && !args.IsArgSet("-legacy")) || (args.IsArgSet("-descriptors") && args.GetBoolArg("-descriptors", true)); | ||
| if (make_legacy && make_descriptors) { | ||
| tfm::format(std::cerr, "Only one of -legacy or -descriptors can be set to true, not both\n"); | ||
| return false; | ||
| } | ||
| if (!make_legacy && !make_descriptors) { | ||
| tfm::format(std::cerr, "One of -legacy or -descriptors must be set to true (or omitted)\n"); | ||
| return false; | ||
|
Comment on lines
+152
to
+162
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep This now turns an explicit descriptor opt-out into a hard error because both Compatibility-preserving approach- // If -legacy is set, use it. Otherwise default to false.
- bool make_legacy = args.GetBoolArg("-legacy", false);
- // If neither -legacy nor -descriptors is set, default to true. If -descriptors is set, use its value.
- bool make_descriptors = (!args.IsArgSet("-descriptors") && !args.IsArgSet("-legacy")) || (args.IsArgSet("-descriptors") && args.GetBoolArg("-descriptors", true));
- if (make_legacy && make_descriptors) {
+ const bool legacy_requested = args.GetBoolArg("-legacy", false);
+ const bool descriptors_requested = args.GetBoolArg("-descriptors", true);
+
+ if (legacy_requested && args.IsArgSet("-descriptors") && descriptors_requested) {
tfm::format(std::cerr, "Only one of -legacy or -descriptors can be set to true, not both\n");
return false;
}
- if (!make_legacy && !make_descriptors) {
- tfm::format(std::cerr, "One of -legacy or -descriptors must be set to true (or omitted)\n");
- return false;
- }
+
+ // Preserve explicit descriptor opt-out for older automation.
+ const bool make_legacy = legacy_requested || (args.IsArgSet("-descriptors") && !descriptors_requested);
+ const bool make_descriptors = !make_legacy;🤖 Prompt for AI Agents |
||
| } | ||
| if (make_descriptors) { | ||
| options.create_flags |= WALLET_FLAG_DESCRIPTORS; | ||
| options.require_format = DatabaseFormat::SQLITE; | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hyphenate “wallet-related”.
Line 8 should use the compound adjective form for consistency and readability.
Suggested wording
📝 Committable suggestion
🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: Use a hyphen to join words.
Context: ...ed RPCs ------------ Changes to wallet related RPCs can be found in the Wallet ...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's copy-paste of bitcoin core's release note. I'd like to see human's feedback here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@knst, confirmed — the phrase is a direct copy-paste from Bitcoin Core. Bitcoin Core's v23.0 release notes use the exact unhyphenated form "Changes to wallet related RPCs can be found in the Wallet section below." The same wording appears verbatim across v24.0.1, v26.0, and v25.0 release notes as well.So while the LanguageTool grammar flag is technically valid, Bitcoin Core has consistently used this unhyphenated style across multiple releases. For a downstream project like Dash Core that tracks Bitcoin Core's release notes, keeping the text identical to upstream is a reasonable choice — it eases future diffs and attribution. I'll defer to your judgment on whether to apply the hyphen or preserve the upstream wording as-is.
✏️ Learnings added