Enable the GUI to also use the load_on_startup feature. Wallets loaded in the GUI always have load_on_startup=true. When they are unloaded from the GUI, load_on_startup=false.
To facilitate this change, UpdateWalletSetting is moved into the wallet module and called from within LoadWallet, RemoveWallet, and Createwallet. This change does not actually touch the GUI code but rather the wallet functions that are shared between the GUI and RPC.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, 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.
Code review ACK e8bfa09b1c1888c33acfd5dcebbe136bf0d6fe82
I think the PR title and description here are hard to follow and it would be better to just describe the behavior change that this is implementing: reloading wallets previously loaded in the GUI on startup. A good title might be “gui: Load previously loaded wallets on startup.” Release notes can be updated too with this. Suggest maybe:
0--- a/doc/release-notes-15937.md
1+++ b/doc/release-notes-15937.md
2@@ -1,12 +1,15 @@
3 Configuration
4 -------------
5
6-The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept
7-`load_on_startup` options that modify bitcoin's dynamic configuration in
8-`\<datadir\>/settings.json`, and can add or remove a wallet from the list of
9-wallets automatically loaded at startup. Unless these options are explicitly
10-set to true or false, the load on startup wallet list is not modified, so this
11-change is backwards compatible.
12+Wallets created or loaded in the GUI will now be automatically loaded on
13+startup, so they don't need to be manually reloaded next time bitcoin is
14+started. The list of wallets to load on startup is stored in
15+`\<datadir\>/settings.json` and augments any command line or `bitcoin.conf`
16+`-wallet=` settings that specify more wallets to load. Wallets that are
17+unloaded in the GUI get removed from the settings list so they won't load again
18+automatically next startup.
19
20-In the future, the GUI will start updating the same startup wallet list as the
21-RPCs to automatically reopen wallets previously opened in the GUI.
22+The `createwallet`, `loadwallet`, and `unloadwallet` RPCs accept new
23+`load_on_startup` options to modify the settings list. Unless these options are
24+explicitly set to true or false, the list is not modified, so the RPC methods
25+remain backward compatible.
Historical note: Commit e8bfa09b1c1888c33acfd5dcebbe136bf0d6fe82 is an expanded version of 546bdce4ece60635a1eb2fa9d46d82ae41aeee24 from #15454. New commit and old commit do the same thing but the new commit does extra refactoring in wallet code.
Code review ACK 4cff046507df20717084260c9902fcd0733a95fa. Only doc changes since last review.
I might still try to make the PR description easier to understand. Not referencing #15937 in the first sentence for example, because you don’t need to know about #15937 to understand this PR. The main thing I think this PR is doing is automatically loading previously loaded wallets automatically on startup so you don’t need to load them manually. Rest is refactoring.
Also could tweak title
wallet: Reload previously loaded wallets on GUI startup
to
gui, wallet: Reload previously loaded wallets on startup
The “on GUI startup” part seems a little inaccurate because the wallets are loaded on bitcoind startup, too, not just bitcoin-qt startup.
11+startup, so they don't need to be manually reloaded next time Bitcoin is
12+started. The list of wallets to load on startup is stored in
13+`\<datadir\>/settings.json` and augments any command line or `bitcoin.conf`
14+`-wallet=` settings that specify more wallets to load. Wallets that are
15+unloaded in the GUI get removed from the settings list so they won't load again
16+automatically next startup.
0automatically next startup. (#19754)
18-In the future, the GUI will start updating the same startup wallet list as the
19-RPCs to automatically reopen wallets previously opened in the GUI.
20+The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept
21+`load_on_startup` options to modify the settings list. Unless these options are
22+explicitly set to true or false, the list is not modified, so the RPC methods
23+remain backwards compatible.
0remain backwards compatible. (#15937)
Enable the GUI to also use the load_on_startup feature.
Wallets loaded in the GUI always have load_on_startup=true.
When they are unloaded from the GUI, load_on_startup=false.
To facilitate this change, UpdateWalletSetting is moved into the wallet
module and called from within LoadWallet, RemoveWallet, and
Createwallet. This change does not actually touch the GUI code but
rather the wallet functions that are shared between the GUI and RPC.
settings.json should never result in new wallets - an entry that doesn’t exists is just skipped maybe?
Tested but found a corner case that I think it should be fixed. If I manually move away a wallet then it will be created again. I think that
settings.jsonshould never result in new wallets - an entry that doesn’t exists is just skipped maybe?
I think that’s a bit orthogonal to this PR. It’s part of the settings reading stuff I think. I would prefer that to be fixed in a followup as I believe it is a current bug in the loading implementation.
Tested but found a corner case that I think it should be fixed. If I manually move away a wallet then it will be created again
This is similar to what happens with the default wallet currently. If you move it away and restart, it will be created again. I don’t think this PR really changes the situation: all it is doing is expanding the list of default wallets.
I do agree it would be better not to create settings wallets on startup if they don’t exist. It would be natural to do this in #15454 as part of the change that avoids creating the default wallet.
I also think we could go further and not create wallets at all on startup. There are 4 sources of wallet names that can be created on startup:
"" wallet-wallet command line valuesbitcoin.conf wallet valuessettings.json wallet valuesSimplest and safest thing would be to just load wallets with these names and not create them
-server mode) or through the RPC console won’t be loaded on startup but wallets loaded via the GUI menu will.
