[Qt] add startup option to reset Qt settings #7006

pull jonasschnelli wants to merge 2 commits into bitcoin:master from jonasschnelli:2015/11/qt_resetsettings changing 4 files +13 −9
  1. jonasschnelli commented at 3:38 PM on November 13, 2015: contributor

    Fixes #6749. can cure issues like #6749.

    Setting a invalid Proxy (-proxy) or TorProxy (-onion) over the GUI settings panel will result in an InitError() (terminate the app) during the next app startup. Qt stores its settings in the windows registry and similar infrastructures and are therefore non-trivial to flush.

    This PR adds a startup argument -resetguisettings which resets all GUI settings and reenable a smooth startup. This does not affect the bitcoin.conf file (different settings layer).

    I'm not entirely happy with this solution because some (most?) non expert users have problems starting bitcoin-qt with a command line argument because it involves opening a shell (cmd.exe, etc.).

  2. [Qt] add startup option to reset Qt settings ae98388b22
  3. paveljanik commented at 3:59 PM on November 13, 2015: contributor

    What kind of invalid settings is it? Do we save invalid values or values become invalid by time or by other reasons? Do you have an example? If we can print "invalid proxy or something", we can also open the relevant dialog setting the proxy and allow the user to fix it. No?

  4. paveljanik commented at 4:01 PM on November 13, 2015: contributor

    Otherwise we have to document this for users so they can find it. But I think it is cleaner to prevent it to happen.

  5. jonasschnelli added the label GUI on Nov 13, 2015
  6. jonasschnelli commented at 4:15 PM on November 13, 2015: contributor

    I think having a way of resetting the qt settings is nice, although i agree with @paveljanik that it should not be possible to "save" a invalid proxy over the GUI settings panel.

    You can test it yourself by setting a proxy in the GUI settings panel with the IP "0.0.0.0" (port doesn't matter).

    Open the settings window and allow the user to change it, would be a invasive change to the startup process. I have though about a "reset setting" button in a error dialog during startup,... but that also feels wrong.

  7. laanwj commented at 4:15 PM on November 13, 2015: member

    What kind of invalid settings is it? Do we save invalid values or values become invalid by time or by other reasons? Do you have an example?

    See #6749. It's not supposed to happen, but it can happen that the settings in the QSettings are - somehow - unparseable or corrupted. Passing a command line option may be somewhat easier than directing the user to find the settings in regedit.

  8. paveljanik commented at 4:22 PM on November 13, 2015: contributor

    I do not want the user to be redirected to regedit ;-) I think he should end up in our dialog for setting the "corrupted" proxy setting in this case.

    But anyway, having this option is good anyway, lets document it :-)

  9. laanwj commented at 9:02 AM on November 14, 2015: member

    Well yes, a corrupted proxy setting should result in a warning, not a fatal error.

    But I suspect that's only one of the things that can go wrong. Having a 'reset to factory settings' can never hurt.

  10. laanwj commented at 10:28 AM on November 14, 2015: member

    Well yes, a corrupted proxy setting should result in a warning, not a fatal error.

    Thinking about this a bit I'm not sure. It makes sense to have this fatal. Or at least have it disable all networking. Say you set up a proxy to hide your identify. It happens to be detected as corrupted (for some reason). Then the last thing you want is to disable it, one such leak can ruin everything.

  11. paveljanik commented at 10:32 AM on November 14, 2015: contributor

    Yes, proxy setting is very sensible (this is why I asked for other examples, because I have heard - and read in #6749 - only about proxy setting so far).

  12. jonasschnelli commented at 11:31 AM on November 14, 2015: contributor

    I agree, the proxy setting is very sensible. A better approach would be to check the proxy when entering an IP in the QT settings panel. We shouldn't allow a invalid proxy so it would never lead to a startup error.

  13. paveljanik commented at 11:49 AM on November 14, 2015: contributor

    Jonas: the setting can be invalidated by time, network issues etc...

  14. paveljanik commented at 11:49 AM on November 14, 2015: contributor

    Or it can even be an attack.

  15. MarcoFalke commented at 5:10 PM on November 14, 2015: member

    It makes sense to have this fatal.

    There is more than just fatal and warning. Couldn't we do a prompt with something like

    Problem with $[invalid qt setting]
    Button("Stop Bitcoin Core")     Button("Reset $[invalid qt setting]")
    

    ?

  16. jonasschnelli commented at 6:57 PM on November 14, 2015: contributor

    Hmm.. the only check where a invalid proxy can lead to a app startup error & termination is CAddress.isValid() (https://github.com/bitcoin/bitcoin/blob/master/src/init.cpp#L1117). I think changes in the local network or somehow something that influents the local time can't raise a startup issue like #6749. A invalid proxy address can be detected and rejected shortly before storing to the QSettings persistent store.

    An attack would be possible, although not much different to an attack to bitcoin.conf. Out of scope for this PR.

    However, I think this PR is useful in case of invalid or corrupt Qt settings.

  17. MarcoFalke commented at 7:00 PM on November 14, 2015: member

    Yes, this PR is useful, but you shouldn't call it "Fixes #6749."

  18. jonasschnelli commented at 8:00 AM on November 16, 2015: contributor

    Added a HelpMessageOpt for -resetguisettings.

  19. jonasschnelli commented at 8:01 AM on November 16, 2015: contributor

    @MarcoFalke: right. It doesn't fix #6749, i'll just changes the PR description. Will work on a additional fix.

  20. paveljanik commented at 8:48 PM on November 18, 2015: contributor

    -help looks like this now:

      -min
           Start minimized
    ...
      -resetguisettings
           Reset all settings changes made over the GUI (default: 0)
    

    What about removing (default: 0) completely?

  21. add UI help for -resetguisettings f71bfefcb0
  22. jonasschnelli force-pushed on Nov 18, 2015
  23. jonasschnelli commented at 8:51 PM on November 18, 2015: contributor

    Yes. The default: 0 is somehow useless. Just removed it over a amend force push.

  24. paveljanik commented at 8:57 PM on November 18, 2015: contributor

    ACK

    Thanks!

  25. MarcoFalke commented at 11:22 AM on November 19, 2015: member

    Concept ACK

  26. jonasschnelli merged this on Nov 25, 2015
  27. jonasschnelli closed this on Nov 25, 2015

  28. jonasschnelli referenced this in commit 26af1ac7cb on Nov 25, 2015
  29. MarcoFalke locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 18:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me