Catch invalid networks combination crash #690

1. 
   furszy commented at 2:12 pm on December 26, 2022: member

   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.
2. gui: bugfix, catch invalid networks combination crash

   We shouldn't crash if a network is set inside
   bitcoin.conf and another one is provided as param.

   f4a11d7baf
3. 
   DrahtBot commented at 2:12 pm on December 26, 2022: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	jarolrod, johnny9, john-moffett, pablomartin4btc, hebasto
   Concept ACK	hernanmarino

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. jarolrod approved
5. 
   jarolrod commented at 0:21 am on December 29, 2022: member

   tACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e

   Reproduced the crash on master by passing in a chain param while my bitcoin.conf specified a different chain:

   master

   0libc++abi: terminating with uncaught exception of type std::runtime_error: Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.
   1[1]    53813 abort      ./src/qt/bitcoin-qt -signet
   

   The PR branch fixes the issue for me:

   pr

   0Error: 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.

6. johnny9 approved
7. 
   johnny9 commented at 0:54 am on December 29, 2022: contributor

   tACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e

   Tested the changes with the following method.

   1. Set bitcoin.conf to testnet

   0// /home/user/.bitcoin/bitcoin.conf
   1testnet=1
   

   2. ran bitcoin-qt with the following command bitcoin-qt -signet

   On masterr (e9262ea32a6e1d364fb7974844fadc36f931f8c6):

   0~/github/gui$ ./src/qt/bitcoin-qt -signet                                                                                                                                                                                            
   1Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
   2terminate called after throwing an instance of 'std::runtime_error'
   3  what():  Invalid combination of -regtest, -signet, -testnet and -chain. Can use at most one.
   4Aborted (core dumped)
   

   With PR (f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e):

   0~/github/gui$ ./src/qt/bitcoin-qt -signet
   1Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
   2Error: 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.

8. 
   hernanmarino commented at 3:35 am on December 29, 2022: contributor

   Tested ACK. Also fails on master and succeds on PR.
9. jarolrod added the label Bug on Jan 3, 2023
10. john-moffett approved
11. 
    john-moffett commented at 2:35 pm on January 5, 2023: contributor

    ACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e
12. pablomartin4btc approved
13. 
    pablomartin4btc commented at 6:59 pm on January 5, 2023: contributor

    Tested ACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e. It behaves as expected, tested using different networks before and after the fix. I agree with @johnny9 on functional tests if possible as a follow up ofc.
14. 
    furszy commented at 12:48 pm on January 9, 2023: member

    @hebasto
15. hebasto approved
16. 
    hebasto commented at 6:39 pm on January 15, 2023: member

    ACK f4a11d7baf79ff6929d9ba8a934f53c4c7eb7c8e, tested on Ubuntu 22.04 (Qt 5.15.3).
17. hebasto renamed this:
    bugfix, catch invalid networks combination crash
    Catch invalid networks combination crash
    on Jan 15, 2023
18. hebasto merged this on Jan 15, 2023
19. hebasto closed this on Jan 15, 2023

    ---

20. sidhujag referenced this in commit 64584341bc on Jan 16, 2023
21. bitcoin-core locked this on Jan 15, 2024

Contributors
furszy DrahtBot jarolrod johnny9 hernanmarino john-moffett pablomartin4btc hebasto

Labels
Bug