Bitcoin-Qt: massive options/settings handling rework #2612

pull Diapolo wants to merge 2 commits into bitcoin:master from Diapolo:options_prep changing 13 files +1010 −312
  1. Diapolo commented at 10:56 AM on May 3, 2013: none
    • add new options for database cache and script verification threads
    • add new options for setting separate IPv6 and Tor proxy and show for which networks the base proxy is used currently
    • add label which displays options that are overridden by command-line parameters
    • proxy settings are not applied on-the-fly anymore and require a client restart (ApplyProxySettings() was removed and was not working very well anyway)
    • re-work options reset and require a client shutdown (as it is much easier to do it this way without having to mess with what can be changed on-the-fly and what needs a restart anyway)
    • options reset now writes default values for every single option
    • when changing an option which requires a client restart display a 10 second warning message in statusLabel (via a QTimer)
    • when applying the changes via ok change that to a persistent message, which is displayed even after closing optionsdialog and re-open it, when no client restart was made
    • remove dialog boxes used when changing language or proxy settings
    • add setRestartRequired() and isRestartRequired() to OptionsModel and use the set function when updating options to signal OptionsDialog when a restart is needed
    • resize optionsdialog a little and add some min sizes for certain GUI elements
    • remove apply button from optionsdialog
    • save and restore optionsdialog window position
    • update nTransactionFee in QSettings with a set -paytxfee value when opening optionsdialog (I'm not sure about this yet, perhaps revert to not updating QSettings and just display current -paytxfee value in optionsdialog.)
  2. Diapolo commented at 10:40 AM on May 4, 2013: none

    @laanwj I tested the pulltester executable and it does what it should :).

  3. in src/qt/bitcoin.cpp:None in f845df025f outdated 
     155 | @@ -156,8 +156,8 @@ int main(int argc, char *argv[])
 156 |      // ... then GUI settings:
 157 |      OptionsModel optionsModel;
 158 |  
 159 | -    // Get desired locale (e.g. "de_DE") from command line or use system locale
 160 | -    QString lang_territory = QString::fromStdString(GetArg("-lang", QLocale::system().name().toStdString()));
 161 | +    // Get desired locale (e.g. "de_DE") from options model or use system locale
 162 | +    QString lang_territory = optionsModel.hasLanguage() ? optionsModel.getLanguage() : QLocale::system().name();
    laanwj commented at 1:44 PM on May 5, 2013:

    A suggestion: Instead of adding hasLanguage, we could move the default to QLocale::systemLocale() into OptionsModel

    Diapolo commented at 4:58 PM on May 5, 2013:

    Sounds good to me, I'll change that tomorrow... today :D.

  4. Diapolo commented at 5:12 PM on May 5, 2013: none

    Updated:

    • re-work handling of language settings and move default language code into OptionsModel::getLanguage()
  5. Diapolo commented at 7:00 AM on May 7, 2013: none

    @laanwj Did you yet test this pull?

  6. laanwj commented at 8:13 AM on May 7, 2013: member

    No, not yet

  7. Diapolo commented at 5:42 PM on May 8, 2013: none

    I found a bug, which always displays the "need to restart message" even when chaning stuff that doesn't require a client restart... will look into this later.

  8. Diapolo commented at 7:42 PM on May 19, 2013: none

    I'm still working on the options dialog, currently it's looking like this: options @laanwj I intend to always show the real QSettings values without trying to mess around with showing the real state values. Reason is, it really sucks to try to allow changing a QSetting, which is overriden and whose saved state isn't even shown in the GUI.

  9. luke-jr commented at 4:14 AM on July 17, 2013: member

    @Diapolo Needs rebase.

  10. Diapolo commented at 11:46 AM on July 23, 2013: none

    @luke-jr And I need some feedback :)! Anyway, I currently don't intend to rebase but will rather update the whole pull. I just need some time and motivation to restart working on this.

  11. luke-jr commented at 5:10 PM on July 23, 2013: member

    I don't like that it requires a restart unnecessicarily.

    I didn't notice the rest of the changes.

  12. jonasschnelli commented at 6:40 PM on September 11, 2013: contributor

    @Diapolo could you rebase (if motivated again...). I could also check the changes and give you feedback.

  13. Diapolo commented at 9:50 AM on September 14, 2013: none

    @jonasschnelli Yeah I can rebase and I really would love to get some help with this also :). I'm just taking my current code without doing the commit-msg now and will base it on top of my core proxy pull, as this is also used in the options rework.

  14. Diapolo commented at 11:00 AM on September 14, 2013: none

    Rebased and updated to include all changes I had in my local build for some months now. Needs feedback and a careful review, so please help me dear core-devs and others :).

  15. jonasschnelli commented at 7:44 AM on September 25, 2013: contributor

    recommended graphical/UI changes:

    bildschirmfoto-2013-09-25-um-09 39 16

  16. jonasschnelli commented at 7:46 AM on September 25, 2013: contributor

    because i'm not a proxy/tor guy as well as i have not enough understanding of the cache / verification thread mechanism, i can't give you a ACK on that.

  17. jonasschnelli commented at 7:49 AM on September 25, 2013: contributor

    after building and starting bitcoin-qt i've got a -paytxfee warning (see picture). But my txfee set in QT perfs pannel is 0.00000005.

    Check: bildschirmfoto 2013-09-25 um 09 48 26 bildschirmfoto 2013-09-25 um 09 49 29

  18. Diapolo commented at 9:14 AM on September 25, 2013: none

    Thanks for your comments, I will take a look into your UI suggestions. Have you a bitcoin.conf in use? I never got that -paytxfee warning and will take a look there too. @jonasschnelli Can you do a configuration reset and see if that removes the txfee warning?

  19. jonasschnelli commented at 9:27 AM on September 25, 2013: contributor

    @Diapolo yes bitcoin.conf in use. But only rpc/testet settings there. The warning popped up after building with your pull. But did some other related testing some minutes before. So it might be another thing.

  20. Diapolo commented at 11:04 AM on September 29, 2013: none

    Updated: Removes Apply button from optionsdialog as requested in #3030.

  21. Diapolo commented at 11:15 AM on September 29, 2013: none

    @jonasschnelli options

    I changed the lower part of the UI to be not special anymore (it's a seperate frame but has no special UI style set). It's not part of the tab element, because I wanted it to be visible everytime, no matter, which tab is activated. Can you perhaps post a screen of the current pull?

    Edit: I'm not sure about the expert tab, as this 2 options were requested by @sipa for general availability ;).

  22. Diapolo commented at 6:46 AM on October 2, 2013: none

    @jonasschnelli ping

  23. jonasschnelli commented at 10:48 AM on October 3, 2013: contributor

    some screenshots on mac osx 10.7 qt 4.8

    bildschirmfoto 2013-10-03 um 11 21 06

    bildschirmfoto 2013-10-03 um 11 21 14

    bildschirmfoto 2013-10-03 um 11 21 10

  24. jonasschnelli commented at 10:50 AM on October 3, 2013: contributor

    There is still a problem with the unit... see:

    bildschirmfoto 2013-10-03 um 12 19 45

    bildschirmfoto 2013-10-03 um 12 20 16

  25. Diapolo commented at 1:15 PM on October 3, 2013: none

    Thanks for the screens, UI looks good IMHO, but I have NO idea, about the -paytxfee problem currently. As requested, what happens without bitcoin.conf and after an options reset? I'll investigate further...

  26. jonasschnelli commented at 1:22 PM on October 3, 2013: contributor

    used a fresh datadir,... same problem (txfee in preference window is 0.0001, but seams to be 5BTC internally). Looks like that the default tx fee is somehow wrong and are not corresponding to the UI settings value.

    If i set the -paytxfee=0.0001 by cmd line, ... all good.

  27. Diapolo commented at 1:30 PM on October 3, 2013: none

    The default tx fee is 0 in the core code AFAIK.

    Setting paytxfee=0.0001 via bitcoin.conf is working here, same for -paytxfee=0.0001 via cmd line. The UI value is only used, if there is no paytxfee in .conf or cmd line anyway and is NOT synced with the current state! When you change the tx fee in the UI you will need to restart currently, because of ParseMoney() in init.cpp. Perhaps I have some brain-fuckup in there currently.

    Got it, there IS currently a bug with conversion from UI setting 0.001 to used tx fee I need to investigate! Thanks for bringing me on the right track!

    I'm going to guess, before my changes it was possible to change tx fee on the fly because UI settings don't need PareseMoney() to work and I'm going to restore the previous behaviour to fix that...

  28. Diapolo commented at 3:45 PM on October 3, 2013: none

    Updated:

    • save and restore optionsdialog window position
    • update nTransactionFee in QSettings with a set -paytxfee value when opening optionsdialog (I'm not sure about this yet, perhaps revert to not updating QSettings and just display current -paytxfee value in optionsdialog.) @jonasschnelli Can you retest current code and see if this fixes the tx-fee problems :).
  29. jonasschnelli commented at 6:48 AM on October 4, 2013: contributor

    @Diapolo paytxfee issue is fixed. Works as expected now. But can't give you an ACK because i don't have the time and experience to test the proxy, etc. settings.

    The visual part (UI) looks good for me.

  30. Diapolo commented at 8:18 AM on October 4, 2013: none

    @jonasschnelli Thanks for testing! There were reports of misbehaving options in our Github issues, can you report if these are fixed or if something has changed on that part?

  31. laanwj commented at 9:36 AM on October 4, 2013: member

    @diapolo, @jonasschnelli great work here

  32. laanwj commented at 12:24 PM on October 21, 2013: member

    Needs rebase for -tor -> -onion change in init.cpp.

  33. laanwj commented at 1:53 PM on November 19, 2013: member

    Needs rebase after coin control features introduction. @Diapolo Can you make a list of what needs to be tested here? It's too bad that this contains so many core changes and isn't just a GUI refactor, this makes it harder to merge and test.

  34. Diapolo commented at 3:05 PM on November 20, 2013: none

    Indeed it sucks that this is dependend on core changes... I thought about reworking it or splitting it up into smaller GUI changes, but had not time for this recently :-/. Main stuff that needs to be tested is IMHO the proxy code and restart message handling. The 2 other new core options should just work :).

  35. extend core proxy options and handling
    - rework the proxy handling in init to cover more cases and work more
  thoroughly
- add -proxy6 to allow setting a separate SOCKS5 proxy to reach peers via
  IPv6
- add a hard-block for -tor to be able to remove compatibility code
- rework proxy data-structures to allow recognition of the default proxy
  (-proxy) to give users the ability to see, which proxy (IPv6 / Tor) is
  derived from the default proxy and which was explicitly set
- remove proxy info from RPC getinfo call
- add new getproxyinfo RPC call
    34846825a6
  36. Bitcoin-Qt: massive options/settings handling rework
    - add new options for database cache and script verification threads
- add new options for setting separate IPv6 and Tor proxy and show for
  which networks the base proxy is used currently
- add label which displays options that are overridden by command-line
  parameters
- proxy settings are not applied on-the-fly anymore and require a
  client restart (ApplyProxySettings() was removed and was not working
  very well anyway)
- re-work options reset and require a client shutdown (as it is much
  easier to do it this way without having to mess with what can be
  changed on-the-fly and what needs a restart anyway)
- options reset now writes default values for every single option
- when changing an option which requires a client restart display a 10
  second warning message in statusLabel (via a QTimer)
- when applying the changes via ok change that to a persistent message,
  which is displayed even after closing optionsdialog and re-open it,
  when no client restart was made
- remove dialog boxes used when changing language or proxy settings
- add setRestartRequired() and isRestartRequired() to OptionsModel and
  use the set function when updating options to signal OptionsDialog when
  a restart is needed
- resize optionsdialog a little and add some min sizes for certain GUI
  elements
- remove apply button from optionsdialog
- save and restore optionsdialog window position
- update nTransactionFee in QSettings with a set -paytxfee value when
  opening optionsdialog (I'm not sure about this yet, perhaps revert
  to not updating QSettings and just display current -paytxfee value in
  optionsdialog.)
    f873f7abb8
  37. BitcoinPullTester commented at 5:43 PM on November 20, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f873f7abb8ebf4774b53275aa9c0b5f907130730 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  38. laanwj commented at 11:36 AM on November 21, 2013: member

    Yes it would be good to split it up. As the release of 0.9 is planned in January, let's not have any functional changes to proxy handling for 0.9. There are just no good tests for that.

    On the other hand, a lot of fixes to the dialog itself are very welcome and I'd love to have them in 0.9.

  39. Diapolo commented at 7:59 AM on December 3, 2013: none

    @laanwj I'm going to split of core-changes today, a pull is in the works :).

  40. Diapolo commented at 1:48 PM on December 11, 2013: none

    Closing in favor of #3347, a new version will be made, when and if the core changes get in.

  41. Diapolo closed this on Dec 11, 2013
  42. Diapolo deleted the branch on Dec 11, 2013
  43. DrahtBot locked this on Sep 8, 2021
Contributors
Diapololaanwjluke-jrjonasschnelliBitcoinPullTester
Linked (view graph)
#3347 [Qt] massive options/settings rework (no core changes)
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-18 18:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me