util: Make default arg values more specific

hebasto commented at 3:28 pm on July 8, 2020: member

This PR:

silents printing of the -daemon option for bitcoin-qt
prints the correct default value of the -printtoconsole option for bitcoin-qt
prints the default value of the -server option

On master (abdfd2d0e3ebec7dbead89317ee9192189a35809):

 0$ ./src/bitcoind -help  # `bitcoin-qt -help' prints the same
 1...
 2  -daemon
 3       Run in the background as a daemon and accept commands
 4...
 5  -printtoconsole
 6       Send trace/debug info to console (default: 1 when no -daemon. To disable
 7       logging to file, set -nodebuglogfile)
 8...
 9  -server
10       Accept command line and JSON-RPC commands

With this PR:

 0$ ./src/bitcoind -help
 1...
 2  -daemon
 3       Run in the background as a daemon and accept commands
 4...
 5  -printtoconsole
 6       Send trace/debug info to console (default: 1). Disabled when -daemon=1.
 7       To disable logging to file, set -nodebuglogfile
 8...
 9  -server
10       Accept command line and JSON-RPC commands (default: 1)
11
12$ ./src/qt/bitcoin-qt -help
13...
14  -printtoconsole
15       Send trace/debug info to console (default: 0). To disable logging to
16       file, set -nodebuglogfile
17...
18  -server
19       Accept command line and JSON-RPC commands (default: 0)

in src/util/system.h:119 in f0add14537 outdated

114@@ -115,6 +115,18 @@ inline bool IsSwitchChar(char c)
115 #endif
116 }
117 
118+struct DefaultArgHints {
119+    constexpr DefaultArgHints() = default;

MarcoFalke commented at 3:38 pm on July 8, 2020:

Any reason to have the default constructor and specify default values for the default args? Note that the compiler forces initialization when a member is const, so moving the initialization of the defaults to bitcoind.cpp seems cleaner.

hebasto commented at 3:51 pm on July 8, 2020:

Updated.

hebasto force-pushed on Jul 8, 2020

hebasto commented at 3:51 pm on July 8, 2020: member

Updated f0add1453739be415962c1a4223c7244db4171e5 -> 6a5f5d8b3fc2dcad1b64a59fe9beacde739b578b (pr19471.01 -> pr19471.02, diff):

addressed @MarcoFalke’s comment:

Any reason to have the default constructor and specify default values for the default args? Note that the compiler forces initialization when a member is const, so moving the initialization of the defaults to bitcoind.cpp seems cleaner.

DrahtBot added the label GUI on Jul 8, 2020

DrahtBot added the label Tests on Jul 8, 2020

DrahtBot added the label Utils/log/libs on Jul 8, 2020

DrahtBot commented at 6:46 pm on July 8, 2020: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#21732 (MOVEONLY: Move common init code to init/common by ryanofsky)
#19461 (multiprocess: Add bitcoin-gui -ipcconnect option by ryanofsky)
#19460 (multiprocess: Add bitcoin-wallet -ipcconnect option by ryanofsky)
#10102 ([experimental] Multiprocess bitcoin by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake requested review from ryanofsky on Jul 9, 2020

MarcoFalke removed the label Tests on Jul 22, 2020

DrahtBot added the label Needs rebase on Jul 23, 2020

hebasto force-pushed on Jul 24, 2020

DrahtBot removed the label Needs rebase on Jul 24, 2020

hebasto force-pushed on Jul 24, 2020

hebasto commented at 9:01 am on July 24, 2020: member

Rebased 6a5f5d8b3fc2dcad1b64a59fe9beacde739b578b -> c1030526ebbe2f36c0943a6ba9a4dac12f8fd68f (pr19471.02 -> pr19471.04) due to the conflicts with #15935 and #18923.

MarcoFalke commented at 6:46 am on July 25, 2020: member

re-run ci

MarcoFalke closed this on Jul 25, 2020

MarcoFalke reopened this on Jul 25, 2020

hebasto force-pushed on Jul 29, 2020

hebasto commented at 12:46 pm on July 29, 2020: member

Rebased c1030526ebbe2f36c0943a6ba9a4dac12f8fd68f -> 03bf37b51475e82f9a4df4646138a52c2d8c63f3 (pr19471.04 -> pr19471.05) to include the recent CI changes.

DrahtBot added the label Needs rebase on Jul 30, 2020

hebasto force-pushed on Jul 31, 2020

hebasto commented at 1:29 pm on July 31, 2020: member

Rebased 03bf37b51475e82f9a4df4646138a52c2d8c63f3 -> 96192efd58df41ae871bed2c9d3032bb221f8630 (pr19471.05 -> pr19471.06) due to the conflict with #19561.

DrahtBot removed the label Needs rebase on Jul 31, 2020

DrahtBot added the label Needs rebase on Aug 7, 2020

hebasto force-pushed on Aug 8, 2020

hebasto commented at 7:04 am on August 8, 2020: member

Rebased 96192efd58df41ae871bed2c9d3032bb221f8630 -> c501b996a71db82fce778375fec5482e80e506bf (pr19471.06 -> pr19471.07) due to the conflict with #19098.

hebasto commented at 7:05 am on August 8, 2020: member

@ryanofsky Mind reviewing this PR?

DrahtBot removed the label Needs rebase on Aug 8, 2020

in src/util/system.h:131 in c501b996a7 outdated

127@@ -128,6 +128,17 @@ inline bool IsSwitchChar(char c)
128 #endif
129 }
130 
131+struct DefaultArgHints {

ryanofsky commented at 1:02 pm on August 12, 2020:

In commit “util: Add DefaultArgHints struct” (f5e3b93076403997ce9a3e23253fdaa5514753c9)

I think this struct should be moved to init.h and not be in system.h, since it’s defining options for SetupServerArgs and AppInit function, not interacting with the ArgsManager class or other system functions. It’s fine to include init.h in gui code and https://github.com/bitcoin-core/gui/pull/35 makes the call to SetupServerArgs there more direct
To be a little more consistent with naming and explicit, would rename DefaultArgHints to ServerArgsOptions, and rename printtoconsole to printtoconsole_default, and rename server to server_default. I think ServerArgsOptions::gui is a better name than DefaultArgHints::gui because the gui value is used to show or hide help text, not to determine default argument values.

hebasto commented at 4:06 pm on August 13, 2020:

@ryanofsky Thank you for your review! Updated.

ryanofsky approved

ryanofsky commented at 1:11 pm on August 12, 2020: member

Code review ACK c501b996a71db82fce778375fec5482e80e506bf. Suggested some naming changes below but behavior here is a clear improvement and implementation is good.

hebasto force-pushed on Aug 13, 2020

hebasto commented at 3:44 pm on August 13, 2020: member

Updated c501b996a71db82fce778375fec5482e80e506bf -> 0ae8c14d8d85eda41ecb1731b9788a58e332df09 (pr19471.07 -> pr19471.08, diff):

addressed @ryanofsky’s comments

hebasto force-pushed on Aug 13, 2020

hebasto commented at 4:03 pm on August 13, 2020: member

Rebased 0ae8c14d8d85eda41ecb1731b9788a58e332df09 -> cdd6f157c1780189d2340e06122931a5b8a428c5 (pr19471.08 -> pr19471.09) due to the conflict with #19011.

ryanofsky approved

ryanofsky commented at 11:05 pm on August 24, 2020: member

Code review ACK cdd6f157c1780189d2340e06122931a5b8a428c5. Just rebase and naming updates (thanks!) since last review

DrahtBot added the label Needs rebase on Aug 26, 2020

hebasto force-pushed on Aug 26, 2020

hebasto commented at 9:26 am on August 26, 2020: member

Rebased cdd6f157c1780189d2340e06122931a5b8a428c5 -> 88a5061217332e590948b942815083ad5625d8fe (pr19471.09 -> pr19471.10) due to the conflict with #19779.

DrahtBot removed the label Needs rebase on Aug 26, 2020

DrahtBot added the label Needs rebase on Aug 26, 2020

hebasto force-pushed on Aug 26, 2020

hebasto commented at 6:34 pm on August 26, 2020: member

Rebased 88a5061217332e590948b942815083ad5625d8fe -> 1a02a037a53a10995892c4bd86faa663cad8f656 (pr19471.10 -> pr19471.11) due to the conflict with https://github.com/bitcoin-core/gui/pull/35.

DrahtBot removed the label Needs rebase on Aug 26, 2020

hebasto closed this on Aug 26, 2020

hebasto reopened this on Aug 26, 2020

MarcoFalke removed the label GUI on Sep 7, 2020

ryanofsky approved

ryanofsky commented at 12:03 pm on October 14, 2020: member

Code review ACK 1a02a037a53a10995892c4bd86faa663cad8f656. No changes, just rebase and a few conflict resolutions

DrahtBot added the label Needs rebase on Mar 11, 2021

hebasto force-pushed on Mar 16, 2021

hebasto commented at 8:37 am on March 16, 2021: member

Rebased 1a02a037a53a10995892c4bd86faa663cad8f656 -> d796dd09dd0866f391cf5663bc8e1cc8ca1b3402 (pr19471.11 -> pr19471.12) due to the conflict with #21007.

DrahtBot removed the label Needs rebase on Mar 16, 2021

hebasto force-pushed on Mar 16, 2021

hebasto commented at 4:02 pm on March 16, 2021: member

Rebased d796dd09dd0866f391cf5663bc8e1cc8ca1b3402 -> 2ce271c84d7ac5cee0cb79f5f04662b8b7c42f5b (pr19471.12 -> pr19471.13) on top of the #21447.

ryanofsky approved

ryanofsky commented at 7:20 pm on March 31, 2021: member

Code review ACK 2ce271c84d7ac5cee0cb79f5f04662b8b7c42f5b. Just rebased after conflict

Add ServerArgsOptions struct

A struct is used instead of bit flags as args could have non-boolean
types.

d59d2806c2

Use ServerArgsOptions struct

This change makes -help command print correct default values for some
options.

e7198353c7

in src/qt/bitcoin.cpp:314 in 2ce271c84d outdated

310@@ -309,9 +311,8 @@ void BitcoinApplication::startThread()
311 
312 void BitcoinApplication::parameterSetup()
313 {
314-    // Default printtoconsole to false for the GUI. GUI programs should not
315-    // print to the console unnecessarily.
316-    gArgs.SoftSetBoolArg("-printtoconsole", false);
317+    gArgs.SoftSetBoolArg("-printtoconsole", SERVER_ARGS_OPTIONS.printtoconsole_default);

MarcoFalke commented at 11:24 am on April 2, 2021:

what is the point of overwriting the arg here, but keeping the default value as !args.GetBoolArg("-daemon", false) where it is parsed? It would be cleaner to not do any overwriting here and simply use printtoconsole_default as default value.

hebasto commented at 5:11 pm on April 14, 2021:

Besides https://github.com/bitcoin/bitcoin/blob/2ce271c84d7ac5cee0cb79f5f04662b8b7c42f5b/src/init.cpp#L894 we also have https://github.com/bitcoin/bitcoin/blob/2ce271c84d7ac5cee0cb79f5f04662b8b7c42f5b/src/bitcoin-wallet.cpp#L67 with a different default value.

MarcoFalke commented at 5:41 pm on April 14, 2021:

bitcoin-wallet is a completely separate binary that isn’t even modified in this PR. I fail to see how this is relevant to the discussion?

hebasto commented at 2:54 pm on April 18, 2021:

@MarcoFalke

It would be cleaner to not do any overwriting here and simply use printtoconsole_default as default value.

Yes, you right. But at the parsing place the SERVER_ARGS_OPTIONS is out of the scope. It is possible to bring it into the scope by adding a data member to the ArgsManager but it will make a diff bigger. Is it worth it?

MarcoFalke commented at 3:17 pm on April 18, 2021:

Why add a data member to ArgsManager? Just pass it into InitLogging by reference?

hebasto commented at 4:18 pm on April 18, 2021:

Just pass it into InitLogging by reference?

What about interfaces::Node::initLogging()?

hebasto commented at 4:42 pm on April 18, 2021:

what is the point of overwriting the arg here, but keeping the default value as !args.GetBoolArg("-daemon", false) where it is parsed? It would be cleaner to not do any overwriting here and simply use printtoconsole_default as default value.

It will change the current behavior, as for bitcoind the printtoconsole_default must be disabled when the -daemon option is set.

hebasto commented at 5:23 pm on April 18, 2021:

@MarcoFalke

Thanks! Updated.

hebasto force-pushed on Apr 18, 2021

hebasto commented at 2:50 pm on April 18, 2021: member

Updated 2ce271c84d7ac5cee0cb79f5f04662b8b7c42f5b -> e7198353c72e7f6dcfacb4bf1cc4a284afcee0a8 (pr19471.13 -> pr19471.14, diff):

drop unneeded forward declaration

hebasto commented at 5:21 pm on April 18, 2021: member

Updated e7198353c72e7f6dcfacb4bf1cc4a284afcee0a8 -> ccc0388e18e1340387baa0bcdb8a812e48793721 (pr19471.14 -> pr19471.15, diff):

addressed @MarcoFalke’s comment

Use ServerArgsOptions::printtoconsole_default directly ccc0388e18

hebasto force-pushed on Apr 18, 2021

DrahtBot added the label Needs rebase on Apr 23, 2021

DrahtBot commented at 9:37 am on April 23, 2021: member

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

ryanofsky commented at 1:51 pm on April 27, 2021: member

Sorry, I think I created a lot of conflicts with here with #19160 and #21732. But I think this can be rebased as a change like the diff below. In the diff I consolidated the gui printtoconsole_default server_default options into a single is_gui option in src/bitcoind.cpp and src/qt/bitcoin.cpp because I thought it might be clearer if selection of default option values didn’t need to involve these files. But it didn’t really clean things up very much and it would be reasonable to split the options up again if this is not a good change.

I think this all could be cleaner after the change in #10102 adding separate BitcoinNodeInit / BitcoinWalletInit / BitcoinGuiInit / BitcoinQtInit / BitcoindInit classes, since it will be easier to add custom hooks for each executable. I will try to split that change into a smaller PR, but there is no need to delay better behavior implemented in this PR waiting for IPC stuff.

  0diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp
  1index cf9e4fad44d..7e44ca7517d 100644
  2--- a/src/bitcoind.cpp
  3+++ b/src/bitcoind.cpp
  4@@ -170,10 +170,8 @@ static bool AppInit(NodeContext& node, int argc, char* argv[])
  5             return false;
  6         }
  7 
  8-        // -server defaults to true for bitcoind but not for the GUI so do this here
  9-        args.SoftSetBoolArg("-server", true);
 10         // Set this early so that parameter interactions go to console
 11-        InitLogging(args);
 12+        InitLogging(args, node.is_gui);
 13         InitParameterInteraction(args);
 14         if (!AppInitBasicSetup(args)) {
 15             // InitError will have been called with detailed error, which ends up on console
 16diff --git a/src/init.cpp b/src/init.cpp
 17index 24d67f48dc1..09faa05f28c 100644
 18--- a/src/init.cpp
 19+++ b/src/init.cpp
 20@@ -347,7 +347,7 @@ void SetupServerArgs(NodeContext& node)
 21     SetupHelpOptions(argsman);
 22     argsman.AddArg("-help-debug", "Print help message with debugging options and exit", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); // server-only for now
 23 
 24-    init::AddLoggingArgs(argsman);
 25+    init::AddLoggingArgs(argsman, /* can_daemonize= */ !node.is_gui, /* default_printtoconsole= */ !node.is_gui);
 26 
 27     const auto defaultBaseParams = CreateBaseChainParams(CBaseChainParams::MAIN);
 28     const auto testnetBaseParams = CreateBaseChainParams(CBaseChainParams::TESTNET);
 29@@ -545,11 +545,16 @@ void SetupServerArgs(NodeContext& node)
 30     argsman.AddArg("-rpcwhitelist=<whitelist>", "Set a whitelist to filter incoming RPC calls for a specific user. The field <whitelist> comes in the format: <USERNAME>:<rpc 1>,<rpc 2>,...,<rpc n>. If multiple whitelists are set for a given user, they are set-intersected. See -rpcwhitelistdefault documentation for information on default whitelist behavior.", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 31     argsman.AddArg("-rpcwhitelistdefault", "Sets default behavior for rpc whitelisting. Unless rpcwhitelistdefault is set to 0, if any -rpcwhitelist is set, the rpc server acts as if all rpc users are subject to empty-unless-otherwise-specified whitelists. If rpcwhitelistdefault is set to 1 and no -rpcwhitelist is set, rpc server acts as if all rpc users are subject to empty whitelists.", ArgsManager::ALLOW_BOOL, OptionsCategory::RPC);
 32     argsman.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::RPC);
 33-    argsman.AddArg("-server", "Accept command line and JSON-RPC commands", ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 34+    argsman.AddArg("-server", strprintf("Accept command line and JSON-RPC commands (default: %u)", !node.is_gui), ArgsManager::ALLOW_ANY, OptionsCategory::RPC);
 35 
 36 #if HAVE_DECL_FORK
 37+    if (!node.is_gui) {
 38     argsman.AddArg("-daemon", strprintf("Run in the background as a daemon and accept commands (default: %d)", DEFAULT_DAEMON), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
 39     argsman.AddArg("-daemonwait", strprintf("Wait for initialization to be finished before exiting. This implies -daemon (default: %d)", DEFAULT_DAEMONWAIT), ArgsManager::ALLOW_BOOL, OptionsCategory::OPTIONS);
 40+    } else {
 41+        hidden_args.emplace_back("-daemon");
 42+        hidden_args.emplace_back("-daemonwait");
 43+    }
 44 #else
 45     hidden_args.emplace_back("-daemon");
 46     hidden_args.emplace_back("-daemonwait");
 47@@ -740,9 +745,10 @@ void InitParameterInteraction(ArgsManager& args)
 48  * Note that this is called very early in the process lifetime, so you should be
 49  * careful about what global state you rely on here.
 50  */
 51-void InitLogging(const ArgsManager& args)
 52+void InitLogging(const ArgsManager& args, bool is_gui)
 53 {
 54-    init::SetLoggingOptions(args);
 55+    init::SetLoggingOptions(args, /* is_daemon= */ args.GetBoolArg("-daemon", DEFAULT_DAEMON) || args.GetBoolArg("-daemonwait", DEFAULT_DAEMONWAIT), /* default_printtoconsole= */ !is_gui);
 56+
 57     init::LogPackageVersion();
 58 }
 59 
 60@@ -1169,7 +1175,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 61      * that the server is there and will be ready later).  Warmup mode will
 62      * be disabled when initialisation is finished.
 63      */
 64-    if (args.GetBoolArg("-server", false)) {
 65+    if (args.GetBoolArg("-server", !node.is_gui)) {
 66         uiInterface.InitMessage_connect(SetRPCWarmupStatus);
 67         if (!AppInitServers(node))
 68             return InitError(_("Unable to start HTTP server. See debug log for details."));
 69diff --git a/src/init.h b/src/init.h
 70index 328eda9c7e8..b0c953ab8a3 100644
 71--- a/src/init.h
 72+++ b/src/init.h
 73@@ -28,7 +28,7 @@ class thread_group;
 74 void Interrupt(NodeContext& node);
 75 void Shutdown(NodeContext& node);
 76 //!Initialize the logging infrastructure
 77-void InitLogging(const ArgsManager& args);
 78+void InitLogging(const ArgsManager& args, bool is_gui);
 79 //!Parameter interaction: change current parameters depending on various rules
 80 void InitParameterInteraction(ArgsManager& args);
 81 
 82diff --git a/src/init/common.cpp b/src/init/common.cpp
 83index 79e0c9da782..8ba2262b4f3 100644
 84--- a/src/init/common.cpp
 85+++ b/src/init/common.cpp
 86@@ -58,7 +58,7 @@ bool SanityChecks()
 87     return true;
 88 }
 89 
 90-void AddLoggingArgs(ArgsManager& argsman)
 91+void AddLoggingArgs(ArgsManager& argsman, bool can_daemonize, bool default_printtoconsole)
 92 {
 93     argsman.AddArg("-debuglogfile=<file>", strprintf("Specify location of debug log file. Relative paths will be prefixed by a net-specific datadir location. (-nodebuglogfile to disable; default: %s)", DEFAULT_DEBUGLOGFILE), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
 94     argsman.AddArg("-debug=<category>", "Output debugging information (default: -nodebug, supplying <category> is optional). "
 95@@ -74,15 +74,19 @@ void AddLoggingArgs(ArgsManager& argsman)
 96 #endif
 97     argsman.AddArg("-logsourcelocations", strprintf("Prepend debug output with name of the originating source location (source file, line number and function name) (default: %u)", DEFAULT_LOGSOURCELOCATIONS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
 98     argsman.AddArg("-logtimemicros", strprintf("Add microsecond precision to debug timestamps (default: %u)", DEFAULT_LOGTIMEMICROS), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
 99-    argsman.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set -nodebuglogfile)", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
100+    argsman.AddArg("-printtoconsole", strprintf("Send trace/debug info to console (default: %u).%s To disable logging to file, set -nodebuglogfile", default_printtoconsole, can_daemonize ? " Disabled when -daemon=1." : ""), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
101     argsman.AddArg("-shrinkdebugfile", "Shrink debug.log file on client startup (default: 1 when no -debug)", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
102 }
103 
104-void SetLoggingOptions(const ArgsManager& args)
105+void SetLoggingOptions(const ArgsManager& args, bool is_daemon, bool default_printtoconsole)
106 {
107     LogInstance().m_print_to_file = !args.IsArgNegated("-debuglogfile");
108     LogInstance().m_file_path = AbsPathForConfigVal(args.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE));
109-    LogInstance().m_print_to_console = args.GetBoolArg("-printtoconsole", !args.GetBoolArg("-daemon", false));
110+    if (is_daemon) {
111+        LogInstance().m_print_to_console = false;
112+    } else {
113+        LogInstance().m_print_to_console = args.GetBoolArg("-printtoconsole", default_printtoconsole);
114+    }
115     LogInstance().m_log_timestamps = args.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
116     LogInstance().m_log_time_micros = args.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
117 #ifdef HAVE_THREAD_LOCAL
118diff --git a/src/init/common.h b/src/init/common.h
119index fc4bc1b2800..6efa7917093 100644
120--- a/src/init/common.h
121+++ b/src/init/common.h
122@@ -18,8 +18,8 @@ void UnsetGlobals();
123  *  necessary library support.
124  */
125 bool SanityChecks();
126-void AddLoggingArgs(ArgsManager& args);
127-void SetLoggingOptions(const ArgsManager& args);
128+void AddLoggingArgs(ArgsManager& argsman, bool can_daemonize, bool default_printtoconsole);
129+void SetLoggingOptions(const ArgsManager& args, bool is_daemon, bool default_printtoconsole);
130 void SetLoggingCategories(const ArgsManager& args);
131 bool StartLogging(const ArgsManager& args);
132 void LogPackageVersion();
133diff --git a/src/node/context.h b/src/node/context.h
134index 06adb33a80e..1bdaaddb6b7 100644
135--- a/src/node/context.h
136+++ b/src/node/context.h
137@@ -39,6 +39,9 @@ class WalletClient;
138 struct NodeContext {
139     //! Init interface for initializing current process and connecting to other processes.
140     interfaces::Init* init{nullptr};
141+    //! Bool indicating if this is a GUI process that does not support
142+    //! daemonization, and should also disable the RPC server by default.
143+    bool is_gui{false};
144     std::unique_ptr<CAddrMan> addrman;
145     std::unique_ptr<CConnman> connman;
146     std::unique_ptr<CTxMemPool> mempool;
147diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
148index 8befbf5e307..8352936c182 100644
149--- a/src/node/interfaces.cpp
150+++ b/src/node/interfaces.cpp
151@@ -71,7 +71,7 @@ private:
152     ChainstateManager& chainman() { return *Assert(m_context->chainman); }
153 public:
154     explicit NodeImpl(NodeContext* context) { setContext(context); }
155-    void initLogging() override { InitLogging(*Assert(m_context->args)); }
156+    void initLogging() override { InitLogging(*Assert(m_context->args), m_context->is_gui); }
157     void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); }
158     bilingual_str getWarnings() override { return GetWarnings(true); }
159     uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
160@@ -93,7 +93,7 @@ public:
161     {
162         StartShutdown();
163         // Stop RPC for clean shutdown if any of waitfor* commands is executed.
164-        if (gArgs.GetBoolArg("-server", false)) {
165+        if (gArgs.GetBoolArg("-server", !m_context->is_gui)) {
166             InterruptRPC();
167             StopRPC();
168         }
169diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
170index de71b7dea7d..33a9c24d281 100644
171--- a/src/qt/bitcoin.cpp
172+++ b/src/qt/bitcoin.cpp
173@@ -311,11 +311,7 @@ void BitcoinApplication::startThread()
174 
175 void BitcoinApplication::parameterSetup()
176 {
177-    // Default printtoconsole to false for the GUI. GUI programs should not
178-    // print to the console unnecessarily.
179-    gArgs.SoftSetBoolArg("-printtoconsole", false);
180-
181-    InitLogging(gArgs);
182+    InitLogging(gArgs, /* is_gui= */ true);
183     InitParameterInteraction(gArgs);
184 }
185 
186@@ -464,6 +460,7 @@ int GuiMain(int argc, char* argv[])
187     util::ThreadSetInternalName("main");
188 
189     NodeContext node_context;
190+    node_context.is_gui = true;
191     std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context);
192 
193     // Subscribe to global signals from core
194diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
195index ffc51151458..42793c1abf6 100644
196--- a/src/test/util/setup_common.cpp
197+++ b/src/test/util/setup_common.cpp
198@@ -101,7 +101,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
199     SelectParams(chainName);
200     SeedInsecureRand();
201     if (G_TEST_LOG_FUN) LogInstance().PushBackCallback(G_TEST_LOG_FUN);
202-    InitLogging(*m_node.args);
203+    InitLogging(*m_node.args, m_node.is_gui);
204     AppInitParameterInteraction(*m_node.args);
205     LogInstance().StartLogging();
206     SHA256AutoDetect();

DrahtBot commented at 11:21 am on December 15, 2021: member

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

DrahtBot commented at 1:07 pm on March 21, 2022: member

Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
Is it no longer relevant? ➡️ Please close.
Did the author lose interest or time to work on this? ➡️ Please close it and mark it ‘Up for grabs’ with the label, so that it can be picked up in the future.

hebasto commented at 2:32 pm on April 22, 2022: member

I won’t be able to focus on this stuff in the near future.

So closing up for grabs.

hebasto closed this on Apr 22, 2022

hebasto added the label Up for grabs on Apr 22, 2022

DrahtBot locked this on Apr 22, 2023

util: Make default arg values more specific #19471

Conflicts