docs: Update to match new default wallet type #24281
Conversation
shaavan
left a comment
shaavan
left a comment
There was a problem hiding this comment.
ACK 2a8cb26
I have checked that the default wallet_type is set to descriptor wallet after #23002.
The managing-wallets documentation is the first thing one would see if they intend to create a wallet, and I think it makes sense to update it along with the latest changes regarding the creation of wallets.
The wording used in this PR is concise and to the point while clearly intending the working of the RPC argument.
p.s.: I shall look through the codebase for other probable updates related to this PR.
doc/managing-wallets.md
Outdated
| The following command, for example, creates a descriptor wallet. More information about this command may be found by running `bitcoin-cli help createwallet`. | ||
|
|
||
| ``` | ||
| $ bitcoin-cli -named createwallet wallet_name="wallet-01" descriptors=true |
There was a problem hiding this comment.
Might as well drop descriptors=true from this example. That also removes the need for -named arguments: $ bitcoin-cli createwallet "wallet-01" (quotes can probably be dropped too)
There was a problem hiding this comment.
Took your advice except I left the quotes since "wallet-01" is quoted everywhere else in this doc.
doc/managing-wallets.md
Outdated
| ``` | ||
|
|
||
| The `descriptors` parameter can be omitted if the intention is to create a legacy wallet. For now, the default type is the legacy wallet, but that is expected to change in a future release. | ||
| The `descriptors` parameter can be set to false if the intention is to create a legacy wallet. |
There was a problem hiding this comment.
If anyone wants to to this, they can read the docs. So maybe just drop this paragraph.
There was a problem hiding this comment.
I tried to find information on creating a legacy wallet, but I couldn't easily find one other than this file.
If our goal is to discourage users from creating a new wallet as a legacy wallet, I think it makes to remove this line. However, if that's not our goal, I think this line shouldn't be removed.
This file is documentation on managing wallets. I don't think it's optimal not to mention how to create a legacy wallet in this file unless our goal is to make it challenging for the user to create a legacy wallet.
There was a problem hiding this comment.
The goal is not to make it challenging. But this "chapter" is called "Backing Up and Restoring The Wallet". That seems the wrong context to discuss legacy wallets. Any distraction in an explanation of backups can cause users to think "oh it's complicated" and end up not making a backup.
|
Concept ACK for updating this. |
|
Concept ACK |
|
Concept ACK Note: It may be useful to emphasize a few of these warnings.
Github mutes the bold text but it has good contrast in a desktop markdown viewer. |
Which changed in bitcoin#23002.
bitcoinhodler
force-pushed
the
doc-fix-default-wallet
branch
from
2a8cb26 to
8e9699c
Compare
I don't disagree but it's a little beyond the scope of what I was trying to do in this PR. |
FWIW there's further occurences of |
luke-jr
left a comment
luke-jr
left a comment
There was a problem hiding this comment.
Probably better to leave the explicit descriptors=true around as long as the legacy-default versions are supported.
| $ bitcoin-cli createwallet "wallet-01" | ||
| ``` | ||
|
|
||
| The `descriptors` parameter can be omitted if the intention is to create a legacy wallet. For now, the default type is the legacy wallet, but that is expected to change in a future release. |
There was a problem hiding this comment.
Concept ACK removing this one line
I looked through it, and I think removing the |
I don't think this matters. Documentation on the master branch refers only to the code on the master branch. |
|
ACK 8e9699c |
…riptor wallet by default 5347c97 doc: update multisig-tutorial.md to default wallet type (Jon Atack) Pull request description: Follow-up to #24281 and bitcoin/bitcoin#24281 (comment). The default wallet type was changed to descriptor wallets in #23002. ACKs for top commit: laanwj: ACK 5347c97 michaelfolkson: ACK 5347c97 achow101: ACK 5347c97 theStack: Concept and code-review ACK 5347c97 Tree-SHA512: 8074a33ad253ecb7d3f78645a00c808c7c224996cc1748067928aa59ef31a58f24fcfc75169494b26a19c7fbbf23bbd78516ab4102bc52fa92f08f1f49b18b63
11 participants
#23002 changed the default wallet type to descriptors, so this doc was out of date.