wallet: Make -wallet setting not create wallets #20186
Conversation
|
Concept ACK. |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
Testing bc2b0d9 with the GUI. Steps:
Is such behavior expected? If "yes", is it worth to document it? |
There was a problem hiding this comment.
re: #20186 (comment)
Is such behavior expected? If "yes", is it worth to document it?
This comment describes the behavior (which is expected behavior) but doesn't describe which parts of the behavior are good or bad, or which parts of the behavior are undocumented. Maybe reading list of alternatives bitcoin-core/gui#95 (comment) would clarify approach in this PR. This PR is just doing the simplest possible thing it can do to avoid creating wallets unintentionally. Other improvements like displaying temporarily unavailable wallets or updating the settings file can build on top of this PR, but would be more complicated followups.
If the problem here is just documentation, it would help to have a specific suggestion how to improve the documentation.
|
Concept ACK |
This changes -wallet setting to only load existing wallets, not create new ones. - Fixes settings.json corner cases reported by sjors & promag: bitcoin-core/gui#95, bitcoin#19754 (comment), bitcoin#19754 (comment) - Prevents accidental creation of wallets reported most recently by jb55 http://www.erisian.com.au/bitcoin-core-dev/log-2020-09-14.html#l-355 - Simplifies behavior after bitcoin#15454. bitcoin#15454 took the big step of disabling creation of the default wallet. This PR extends it to avoid creating other wallets as well. With this change, new wallets just aren't created on startup, instead of sometimes being created, sometimes not. bitcoin#15454 release notes are updated here and are simpler. This change should be targeted for 0.21.0. It's a bug fix and simplifies behavior of the bitcoin#15937 / bitcoin#19754 / bitcoin#15454 features added in 0.21.0.
|
ACK 01476a8 |
|
Concept ACK. I agree creating wallets is better left as an explicit operation. |
|
ACK roundup meshcollider utACK #20186 (review) jonatack concept ACK #20186 (review) If anyone else is curious to review, the code change is very simple. PR is 90% documentation and test changes |
There was a problem hiding this comment.
review ACK 01476a8 🏂
Show signature and timestamp
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK 01476a88a6095fd3af71cb9bf1eadef920a1197b 🏂
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgBvwv/bY+eUShaDOuvAG0i9cebopzCwinm/okeSaUY8n7+aFqqXQCLxCyDUqjd
iy7tG01DaGjtqwGNfHRzSM6aOLEokQDKG3jwwMLn6FhofEsvfUhyBPrf282pjyeQ
81CdmAwM/jZv1ZEMIRZ+NTGzoRPsSmXbIF5T8SEz1D1lxtu4ZZkQu+sP9aCCPgDW
W30fi++JD0y7E3R3Qzkzgj+XjUAfE31ishJ8omXn6RYpZ905D/XQYsi8qnRcg0Zb
tE3LhCcUa2OJaN6suBWI/C9YKP2a5y7Kv2+4DVEJHDieAsjQFiGkkmqr3WLWChZF
zuP0diiTO4pKGl0LEBfGmP58yxqBWkaBtf6J9xNg/2lOJf5Hca1qYrRQ82YUBsOq
a90o5DSuuE+Su3GPa5npdJ4TKH1cHK3zPW2ntYEHFhCheV6I46Snij0oYlq/odS0
AZHwD7Wcf1Hs3adPOqSBeFT5k1gv7j7KR/d3nCh0LzfbR7/GQtA7brDVnSUVOhjg
guDVKx3B
=B5t/
-----END PGP SIGNATURE-----
Timestamp of file with hash 8c05cf2e3a6f8f2bf14f360a9f14a7a7d62deab81e5fc85dad09447ac883fef6 -
|
|
||
| New wallets can be created through the GUI (which has a more prominent create | ||
| wallet option), through the `bitcoin-cli createwallet` or `bitcoin-wallet | ||
| create` commands, or the `createwallet` RPC. (#15454) |
There was a problem hiding this comment.
| create` commands, or the `createwallet` RPC. (#15454) | |
| create` commands, or the `createwallet` RPC. (#15454, #20186) |
There was a problem hiding this comment.
re: #20186 (comment)
Thanks, added this to https://github.com/bitcoin-core/bitcoin-devwiki/wiki/0.21.0-Release-Notes-Draft
| Bitcoin Core will no longer automatically create new wallets on startup. It will | ||
| load existing wallets specified by `-wallet` options on the command line or in | ||
| `bitcoin.conf` or `settings.json` files. And by default it will also load a | ||
| top-level unnamed ("") wallet. However, if specified wallets don't exist, |
There was a problem hiding this comment.
| top-level unnamed ("") wallet. However, if specified wallets don't exist, | |
| top-level unnamed ("") wallet if it exists. However, if specified wallets don't exist, |
There was a problem hiding this comment.
re: #20186 (comment)
My brain might just be poisoned from looking at this paragraph too many times, but "if it exists" seems like a suspicious condition to add. Like going to a bank teller and saying "I am not robbing this bank today." The paragraph starts off by saying new wallets won't be created, and if a wallet doesn't exist it can't be loaded. Anyway this can be edited in the wiki if someone has a better sense
01476a8 wallet: Make -wallet setting not create wallets (Russell Yanofsky) Pull request description: This changes `-wallet` setting to only load existing wallets, not create new ones. - Fixes settings.json corner cases reported by sjors & promag: bitcoin-core/gui#95, bitcoin#19754 (comment), bitcoin#19754 (comment) - Prevents accidental creation of wallets reported most recently by jb55 http://www.erisian.com.au/bitcoin-core-dev/log-2020-09-14.html#l-355 - Simplifies behavior after bitcoin#15454. bitcoin#15454 took the big step of disabling creation of the default wallet. This PR extends it to avoid creating other wallets as well. With this change, new wallets just aren't created on startup, instead of sometimes being created, sometimes not. bitcoin#15454 release notes are updated here and are simpler. This change should be targeted for 0.21.0. It's a bug fix and simplifies behavior of the bitcoin#15937 / bitcoin#19754 / bitcoin#15454 features added in 0.21.0. --- This PR is implementing the simplest, most basic alternative listed in bitcoin-core/gui#95 (comment). Other improvements mentioned there can build on top of this. ACKs for top commit: achow101: ACK 01476a8 hebasto: re-ACK 01476a8 MarcoFalke: review ACK 01476a8 🏂 Tree-SHA512: 0d50f4e5dfbd04a2efd9fd66c02085a0ed705807bdec1cf5770d0ae8cb6af07080fb81306349937bf66acdb713d03fb35636f6442b650d0820e66cbae09c2f87
There was a problem hiding this comment.
Code review ACK 01476a8.
@ryanofsky not sure if the following should be fixed:
Running with an empty datadir:
bitcoind -regtest -wallet=""Gives
<datadir>/regtest/wallets'. Data is not in recognized format.
This changes
-walletsetting to only load existing wallets, not create new ones.Fixes settings.json corner cases reported by sjors & promag: Don't abort launch when failing to load a future wallet bitcoin-core/gui#95, wallet, gui: Reload previously loaded wallets on startup #19754 (comment), wallet, gui: Reload previously loaded wallets on startup #19754 (comment)
Prevents accidental creation of wallets reported most recently by jb55 http://www.erisian.com.au/bitcoin-core-dev/log-2020-09-14.html#l-355
Simplifies behavior after Remove the automatic creation and loading of the default wallet #15454. Remove the automatic creation and loading of the default wallet #15454 took the big step of disabling creation of the default wallet. This PR extends it to avoid creating other wallets as well. With this change, new wallets just aren't created on startup, instead of sometimes being created, sometimes not. Remove the automatic creation and loading of the default wallet #15454 release notes are updated here and are simpler.
This change should be targeted for 0.21.0. It's a bug fix and simplifies behavior of the #15937 / #19754 / #15454 features added in 0.21.0.
This PR is implementing the simplest, most basic alternative listed in bitcoin-core/gui#95 (comment). Other improvements mentioned there can build on top of this.