Catch invalid networks combination crash #690
Conversation
We shouldn't crash if a network is set inside bitcoin.conf and another one is provided as param.
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. |
jarolrod
left a comment
jarolrod
left a comment
There was a problem hiding this comment.
tACK f4a11d7
Reproduced the crash on master by passing in a chain param while my bitcoin.conf specified a different chain:
master
libc++abi: terminating with uncaught exception of type std::runtime_error: Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.
[1] 53813 abort ./src/qt/bitcoin-qt -signet
The PR branch fixes the issue for me:
pr
Error: Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.
Also tested the scenario where we show the intro to choose a datadir and a network parameter is passed; crash on master, fixed in pr branch.
There was a problem hiding this comment.
tACK f4a11d7
Tested the changes with the following method.
- Set bitcoin.conf to testnet
// /home/user/.bitcoin/bitcoin.conf
testnet=1
- ran bitcoin-qt with the following command
bitcoin-qt -signet
On masterr (e9262ea):
~/github/gui$ ./src/qt/bitcoin-qt -signet
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
terminate called after throwing an instance of 'std::runtime_error'
what(): Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.
Aborted (core dumped)
With PR (f4a11d7):
~/github/gui$ ./src/qt/bitcoin-qt -signet
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
Error: Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.
And the following error dialog also appears now:
The change expands the try block to handle the exception thrown from GetChainName and is a reasonable approach to handling the error and giving the user expected gui error dialog. Some follow up to this bug might include functional test coverage with the settings testcases to prevent regression. Another possible follow up is to handle the exception within ReadConfigFiles so bitcoind doesn't abort from the runtime_error.
hernanmarino
left a comment
hernanmarino
left a comment
There was a problem hiding this comment.
Tested ACK. Also fails on master and succeds on PR.
hebasto
changed the title
8 participants
The app currently crashes if a network is set inside bitcoin.conf and
another one is provided as param.
The reason is an uncaught runtime_error.