gui
gui copied to clipboard
bitcoin-core
Bugfix: Allow the user to start anyway when loading a wallet errors
Right now, if a GUI user leaves a wallet loaded and something happens to make it error at startup, they're stuck.
This turns the error dialog into a question about starting without the wallet instead.
~~Not sure why GitHub thinks this has conflicts - it doesn't.~~
~~Mainly this is a WIP because it uses a hacky global to avoid trying to load the wallets the user has chosen to skip... Suggestions for a good way to deal with that?~~
Rebased, which made it practical to drop the global hack. This seems ready for review now.
Still gives me error message without ability to continue, not question.
$ ./src/qt/bitcoin-qt -version
Bitcoin Core versija v22.99.0-fb3ea0ad3a87
Copyright (C) 2009-2021 The Bitcoin Core developers
Please contribute if you find Bitcoin Core useful. Visit
<https://bitcoincore.org/> for further information about the software.
The source code is available from <https://github.com/bitcoin/bitcoin>.
This is experimental software.
Distributed under the MIT software license, see the accompanying file COPYING
or <https://opensource.org/licenses/MIT>
$ git log | head -n 1
commit fb3ea0ad3a8788e023b9ba7237c26fc8ef0dba79
$ ./src/qt/bitcoin-qt -signet
Error: Error loading /fast_home/neonz/.bitcoin/signet/wallets/jmw/wallet.dat: Wallet requires newer version of Bitcoin Core
Ah, it only caught some errors. Pushed a revision that should catch the rest.
Apart from review comment above, ACK, working for me (tested scenario with trying to load external signer wallet without external signer support compiled into Bitcoin Core).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #bitcoin/bitcoin/17783 (util: Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky)
- #bitcoin/bitcoin/17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
- #bitcoin/bitcoin/17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)
- #bitcoin/bitcoin/16545 (refactor: Implement missing error checking for 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.
@luke-jr do you want to respond to suggestions here? Or would you be open to a someone making a new PR?
Status of the PR seems to be that it is up to date, but has a bug that could cause command line and bitcoin.conf wallets to be ignored. I suggested a fix for the bug and some cleanups in https://github.com/bitcoin-core/gui/pull/236#discussion_r842066839.
I think there is also a usability problem with this PR in the case of temporary errors where a wallet can't be loaded due to a temporary issue like: background assumeutxo download https://github.com/bitcoin/bitcoin/pull/23997, pruning, a missing external signer https://github.com/bitcoin/bitcoin/pull/22173, an encrypted drive not being mounted, a removable drive not being plugged in, or a version incompatibility that will be resolved by upgrading or downgrading. In these cases it would be useful to have the option to continue starting the GUI and letting the node sync, while temporarily not loading individual wallets that are unavailable, and the current dialog doesn't provide an option to temporarily not load a wallet. The only options are quit or continue without loading the wallet next time. I think it would be a improvement to change the dialog to "Wallet
@luke-jr do you want to respond to suggestions here? Or would you be open to a someone making a new PR?
I'm going to close this, and mark "Up for grabs".
