Call init's parameter interaction before we create the UI options model #6780

pull jonasschnelli wants to merge 4 commits into bitcoin:master from jonasschnelli:2015/10/qt_fix_listen changing 4 files +102 −67
  1. jonasschnelli commented at 8:05 AM on October 8, 2015: contributor

    Currently optionsModel(QT) parameters interaction overwrites/dominates initial and depending parameter settings. This PR would factor out init's parameter interaction and call it before AppInit2().

    Fixes #6773

  2. jonasschnelli force-pushed on Oct 8, 2015
  3. jonasschnelli commented at 8:47 AM on October 8, 2015: contributor

    Force pushed a copy'n'paste fix: s/bool InitParameterInteraction()/void InitParameterInteraction(). Credits to @MarcoFalke on IRC.

  4. MarcoFalke commented at 11:58 AM on October 8, 2015: member

    Concept ACK.

  5. laanwj added the label GUI on Oct 8, 2015
  6. laanwj commented at 4:57 PM on October 8, 2015: member

    Concept ACK

  7. paveljanik commented at 6:34 PM on October 8, 2015: contributor

    ACK

  8. in src/qt/bitcoin.cpp:None in e62a033455 outdated 
     397 | @@ -381,7 +398,7 @@ void BitcoinApplication::startThread()
 398 |      if(coreThread)
 399 |          return;
 400 |      coreThread = new QThread(this);
 401 | -    BitcoinCore *executor = new BitcoinCore();
 402 | +    executor = new BitcoinCore();
 403 |      executor->moveToThread(coreThread);
    laanwj commented at 9:09 AM on October 30, 2015:

    It's not allowed to use this object [from this thread] after moving it to another thread. I'd suggest calling InitParameterInteraction() directly from BitcoinApplication::parameterSetup() and not bothering with the stuff in BitcoinCore.

  9. jonasschnelli force-pushed on Nov 13, 2015
  10. jonasschnelli force-pushed on Nov 13, 2015
  11. jonasschnelli force-pushed on Nov 13, 2015
  12. jonasschnelli commented at 2:27 PM on November 13, 2015: contributor

    Rebased and fixed the pointer access after moveToThread by following @laanwj advice.

  13. gmaxwell commented at 7:37 AM on November 17, 2015: contributor

    Sorry, blocksonly mode caused a need for a small change here, as it needs to get moved. @jonasschnelli mind rebasing?

    What about that prune/txindex "interaction" (fails to run), should it move too?

  14. jonasschnelli force-pushed on Nov 17, 2015
  15. jonasschnelli commented at 8:05 AM on November 17, 2015: contributor

    Sorry, blocksonly mode caused a need for a small change here, as it needs to get moved. @jonasschnelli mind rebasing?

    Just rebased and moved the -blocksonly interaction to the new function.

    What about that prune/txindex "interaction" (fails to run), should it move too?

    My idea was to only move "rules" to the new function. Things that evaluate parameters through GetBoolArg or mapArgs, etc. and depending on its state, optional change another startup variable through SoftSetBoolArg, etc.

    Pruning and TxIndex are checks that lead to a InitError() (startup termination) and it would require to move the InitError() context to the ParameterInteraction() function.

  16. MarcoFalke commented at 10:42 PM on November 17, 2015: member

    utACK 51627cc

  17. jonasschnelli added this to the milestone 0.12.0 on Nov 18, 2015
  18. jonasschnelli commented at 9:43 AM on November 21, 2015: contributor

    Would be nice if someone finds time to test this. Binaries: https://bitcoin.jonasschnelli.ch/pulls/6780/ This fixes a ugly bug (#6773) which would be nice to have fixed in 0.12.

  19. fanquake commented at 9:44 AM on November 21, 2015: member

    Will do.

    On Saturday, November 21, 2015, Jonas Schnelli notifications@github.com wrote:

    Would be nice if someone finds time to test this. Binaries: https://bitcoin.jonasschnelli.ch/pulls/6780/ This fixes a ugly bug (#6773 #6773) which would be nice to have fixed in 0.12.

    — Reply to this email directly or view it on GitHub #6780 (comment).

  20. MarcoFalke commented at 6:03 PM on November 21, 2015: member

    @jonasschnelli

    • master:
    $ src/bitcoind -regtest -connect=127.0.0.1 -server -printtoconsole |grep parameter 
2015-11-21 17:52:56 AppInit2: parameter interaction: -connect set -> setting -dnsseed=0
2015-11-21 17:52:56 AppInit2: parameter interaction: -connect set -> setting -listen=0
2015-11-21 17:52:56 AppInit2: parameter interaction: -listen=0 -> setting -upnp=0
2015-11-21 17:52:56 AppInit2: parameter interaction: -listen=0 -> setting -discover=0
2015-11-21 17:52:56 AppInit2: parameter interaction: -listen=0 -> setting -listenonion=0

$ src/qt/bitcoin-qt -regtest -connect=127.0.0.1 -server -printtoconsole |grep parameter 
2015-11-21 17:53:34 AppInit2: parameter interaction: -connect set -> setting -dnsseed=0
    • 51627cc
    $ ~/bitcoin* -regtest -connect=127.0.0.1 -debug=1 -printtoconsole |grep parameter
  21. dcousens commented at 1:42 AM on November 24, 2015: contributor

    concept ACK / utACK

  22. jonasschnelli commented at 10:37 AM on November 26, 2015: contributor

    I'd like to take this into 0.12 because it fixes a slightly security-critical issue (#6773). Some tested ACKs would be nice (another 5 days left until feature freeze).

  23. MarcoFalke commented at 10:50 AM on November 26, 2015: member

    @jonasschnelli I am happy to test but the debug.log issue seems not yet resolved? Am I missing something?

  24. laanwj commented at 11:11 AM on November 26, 2015: member

    utACK. Agree it needs to be in 0.12

    Haven't tested any specific cases - I've been holding off on this because the parameter processing is kind of fragile and I haven't properly reasoned it through yet that this doesn't have effect on the precedence of options specified in e.g. configuration files, command line, QSettings.

  25. jonasschnelli commented at 1:07 PM on November 26, 2015: contributor

    Thanks @MarcoFalke for pointing out the logging issue. Added another commit that moves the logging initialization.

    For reviewers: The critical part are the first 100 lines in AppInit2() because the logging init and the parameter interactions are now before AppInit2().

  26. MarcoFalke commented at 7:54 AM on November 27, 2015: member

    Sanity tests pass now:

    $ src/bitcoind -regtest -connect=127.0.0.1 -server -printtoconsole |egrep "(parameter|Bitcoin)"
2015-11-27 07:39:08 Bitcoin version v0.11.99.0-296c42e (2015-11-26 14:03:27 +0100)
2015-11-27 07:39:08 InitParameterInteraction: parameter interaction: -connect set -> setting -dnsseed=0
2015-11-27 07:39:08 InitParameterInteraction: parameter interaction: -connect set -> setting -listen=0
2015-11-27 07:39:08 InitParameterInteraction: parameter interaction: -listen=0 -> setting -upnp=0
2015-11-27 07:39:08 InitParameterInteraction: parameter interaction: -listen=0 -> setting -discover=0
2015-11-27 07:39:08 InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0

$ src/qt/bitcoin-qt -regtest -connect=127.0.0.1 -server -printtoconsole |egrep "(parameter|Bitcoin)"
2015-11-27 07:53:02 Bitcoin version v0.11.99.0-296c42e (2015-11-26 14:03:27 +0100)
2015-11-27 07:53:02 InitParameterInteraction: parameter interaction: -connect set -> setting -dnsseed=0
2015-11-27 07:53:02 InitParameterInteraction: parameter interaction: -connect set -> setting -listen=0
2015-11-27 07:53:02 InitParameterInteraction: parameter interaction: -listen=0 -> setting -upnp=0
2015-11-27 07:53:02 InitParameterInteraction: parameter interaction: -listen=0 -> setting -discover=0
2015-11-27 07:53:02 InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0
  27. MarcoFalke commented at 7:54 AM on November 27, 2015: member

    utACK 296c42e

  28. Refactor parameter interaction, call it before AppInit2() 411b05ac95
  29. [QT] Call inits parameter interaction before we create the options model 68354e75e9
  30. Move -blocksonly parameter interaction to the new ParameterInteraction() function df66147613
  31. Initialize logging before we do parameter interaction a46f87f0c1
  32. in src/init.cpp:None in 296c42e999 outdated 
     846 | -#ifdef ENABLE_WALLET
 847 | -        if (SoftSetBoolArg("-walletbroadcast", false))
 848 | -            LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__);
 849 | -#endif
 850 | -    }
 851 | -
    MarcoFalke commented at 12:17 PM on November 27, 2015:

    @jonasschnelli Trivial white space rebase conflict due to 9a3e1a59dfed7cb0cf07a34f75d692280aecf2a6. (Looks like you can ignore changes made in master)

  33. jonasschnelli force-pushed on Nov 27, 2015
  34. jonasschnelli commented at 12:26 PM on November 27, 2015: contributor

    Thanks @MarcoFalke: rebased!

  35. laanwj merged this on Nov 27, 2015
  36. laanwj closed this on Nov 27, 2015
  37. laanwj referenced this in commit 2a94cd67e8 on Nov 27, 2015
  38. zkbot referenced this in commit f1aeaec471 on Mar 21, 2018
  39. zkbot referenced this in commit 4fc490c430 on Dec 4, 2019
  40. zkbot referenced this in commit 868c63f92d on Dec 4, 2019
  41. random-zebra referenced this in commit 41aae2d589 on Mar 12, 2020
  42. akshaynexus referenced this in commit e518ef43af on Mar 13, 2020
  43. akshaynexus referenced this in commit 8567895219 on Mar 15, 2020
  44. akshaynexus referenced this in commit 466719cd45 on Mar 15, 2020
  45. akshaynexus referenced this in commit 7e5371edcc on Mar 15, 2020
  46. akshaynexus referenced this in commit a471443c63 on Mar 15, 2020
  47. wqking referenced this in commit df462d054d on Aug 13, 2020
  48. zkbot referenced this in commit ba4eb241e7 on Mar 31, 2021
  49. MarcoFalke locked this on Sep 8, 2021
Contributors
jonasschnelliMarcoFalkelaanwjpaveljanikgmaxwellfanquakedcousens
Labels
GUI

Milestone
0.12.0

Linked (view graph)
#6773 -connect doesn't turn off -listen in bitcoin-qt#7126 [trivial] Move blocksonly parameter interaction to InitParameterInteraction()
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-16 03:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me