Skip to content

Commit

Permalink
gui: Partially revert bitcoin#10244 gArgs and Params changes
Browse files Browse the repository at this point in the history 
Change gui code to use gArgs, Params() functions directly instead of going
through interfaces::Node.

Remotely accessing bitcoin-node ArgsManager from bitcoin-gui works fine in
bitcoin#10102, when bitcoin-gui spawns a new
bitcoin-node process and controls its startup, but for bitcoin-gui to support
-ipcconnect option in bitcoin#19461 and connect
to an existing bitcoin-node process, it needs ability to parse arguments itself
before connecting out.

This change also simplifies bitcoin#10102 a
bit, by making the bitcoin-gui -> bitcoin-node startup sequence more similar to
the bitcoin-node -> bitcoin-wallet startup sequence where the parent process
parses arguments and passes them to the child process instead of the parent
process using the child process to parse arguments.
  • Loading branch information
@ryanofsky
ryanofsky committed Aug 26, 2020
1 parent a12d9e5 commit e133631
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 87 deletions.
19 changes: 2 additions & 17 deletions src/interfaces/node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,8 @@ class NodeImpl : public Node
{
public:
NodeImpl(NodeContext* context) { setContext(context); }
void initError(const bilingual_str& message) override { InitError(message); }
bool parseParameters(int argc, const char* const argv[], std::string& error) override
{
return gArgs.ParseParameters(argc, argv, error);
}
bool readConfigFiles(std::string& error) override { return gArgs.ReadConfigFiles(error, true); }
void forceSetArg(const std::string& arg, const std::string& value) override { gArgs.ForceSetArg(arg, value); }
bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
void selectParams(const std::string& network) override { SelectParams(network); }
bool initSettings(std::string& error) override { return gArgs.InitSettings(error); }
uint64_t getAssumedBlockchainSize() override { return Params().AssumedBlockchainSize(); }
uint64_t getAssumedChainStateSize() override { return Params().AssumedChainStateSize(); }
std::string getNetwork() override { return Params().NetworkIDString(); }
void initLogging() override { InitLogging(gArgs); }
void initParameterInteraction() override { InitParameterInteraction(gArgs); }
void initLogging() override { InitLogging(*Assert(m_context->args)); }
void initParameterInteraction() override { InitParameterInteraction(*Assert(m_context->args)); }
bilingual_str getWarnings() override { return GetWarnings(true); }
uint32_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
bool baseInitialize() override
Expand Down Expand Up @@ -109,7 +95,6 @@ class NodeImpl : public Node
StopMapPort();
}
}
void setupServerArgs() override { return SetupServerArgs(*m_context); }
bool getProxy(Network net, proxyType& proxy_info) override { return GetProxy(net, proxy_info); }
size_t getNodeCount(CConnman::NumConnections flags) override
{
Expand Down
38 changes: 0 additions & 38 deletions src/interfaces/node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,41 +55,6 @@ class Node
public:
virtual ~Node() {}

//! Send init error.
virtual void initError(const bilingual_str& message) = 0;

//! Set command line arguments.
virtual bool parseParameters(int argc, const char* const argv[], std::string& error) = 0;

//! Set a command line argument
virtual void forceSetArg(const std::string& arg, const std::string& value) = 0;

//! Set a command line argument if it doesn't already have a value
virtual bool softSetArg(const std::string& arg, const std::string& value) = 0;

//! Set a command line boolean argument if it doesn't already have a value
virtual bool softSetBoolArg(const std::string& arg, bool value) = 0;

//! Load settings from configuration file.
virtual bool readConfigFiles(std::string& error) = 0;

//! Choose network parameters.
virtual void selectParams(const std::string& network) = 0;

//! Read and update <datadir>/settings.json file with saved settings. This
//! needs to be called after selectParams() because the settings file
//! location is network-specific.
virtual bool initSettings(std::string& error) = 0;

//! Get the (assumed) blockchain size.
virtual uint64_t getAssumedBlockchainSize() = 0;

//! Get the (assumed) chain state size.
virtual uint64_t getAssumedChainStateSize() = 0;

//! Get network name.
virtual std::string getNetwork() = 0;

//! Init logging.
virtual void initLogging() = 0;

Expand Down Expand Up @@ -117,9 +82,6 @@ class Node
//! Return whether shutdown was requested.
virtual bool shutdownRequested() = 0;

//! Setup arguments
virtual void setupServerArgs() = 0;

//! Map port.
virtual void mapPort(bool use_upnp) = 0;

Expand Down
33 changes: 18 additions & 15 deletions src/qt/bitcoin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@
#include <qt/walletmodel.h>
#endif // ENABLE_WALLET

#include <init.h>
#include <interfaces/handler.h>
#include <interfaces/node.h>
#include <node/context.h>
#include <node/ui_interface.h>
#include <noui.h>
#include <uint256.h>
#include <util/system.h>
#include <util/threadnames.h>
#include <util/translation.h>
#include <validation.h>

#include <boost/signals2/connection.hpp>
#include <memory>

#include <QApplication>
Expand Down Expand Up @@ -298,8 +301,8 @@ void BitcoinApplication::parameterSetup()
// print to the console unnecessarily.
gArgs.SoftSetBoolArg("-printtoconsole", false);

m_node.initLogging();
m_node.initParameterInteraction();
InitLogging(gArgs);
InitParameterInteraction(gArgs);
}

void BitcoinApplication::InitializePruneSetting(bool prune)
Expand Down Expand Up @@ -437,9 +440,9 @@ int GuiMain(int argc, char* argv[])
std::unique_ptr<interfaces::Node> node = interfaces::MakeNode(&node_context);

// Subscribe to global signals from core
std::unique_ptr<interfaces::Handler> handler_message_box = node->handleMessageBox(noui_ThreadSafeMessageBox);
std::unique_ptr<interfaces::Handler> handler_question = node->handleQuestion(noui_ThreadSafeQuestion);
std::unique_ptr<interfaces::Handler> handler_init_message = node->handleInitMessage(noui_InitMessage);
boost::signals2::scoped_connection handler_message_box = ::uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox);
boost::signals2::scoped_connection handler_question = ::uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion);
boost::signals2::scoped_connection handler_init_message = ::uiInterface.InitMessage_connect(noui_InitMessage);

// Do not refer to data directory yet, this can be overridden by Intro::pickDataDirectory

Expand All @@ -457,11 +460,11 @@ int GuiMain(int argc, char* argv[])

/// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
// Command-line options take precedence:
node->setupServerArgs();
SetupServerArgs(node_context);
SetupUIArgs(gArgs);
std::string error;
if (!node->parseParameters(argc, argv, error)) {
node->initError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
if (!gArgs.ParseParameters(argc, argv, error)) {
InitError(strprintf(Untranslated("Error parsing command line arguments: %s\n"), error));
// Create a message box, because the gui has neither been created nor has subscribed to core signals
QMessageBox::critical(nullptr, PACKAGE_NAME,
// message can not be translated because translations have not been initialized
Expand Down Expand Up @@ -502,13 +505,13 @@ int GuiMain(int argc, char* argv[])
/// 6. Determine availability of data directory and parse bitcoin.conf
/// - Do not call GetDataDir(true) before this step finishes
if (!CheckDataDirOption()) {
node->initError(strprintf(Untranslated("Specified data directory \"%s\" does not exist.\n"), gArgs.GetArg("-datadir", "")));
InitError(strprintf(Untranslated("Specified data directory \"%s\" does not exist.\n"), gArgs.GetArg("-datadir", "")));
QMessageBox::critical(nullptr, PACKAGE_NAME,
QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", ""))));
return EXIT_FAILURE;
}
if (!node->readConfigFiles(error)) {
node->initError(strprintf(Untranslated("Error reading configuration file: %s\n"), error));
if (!gArgs.ReadConfigFiles(error, true)) {
InitError(strprintf(Untranslated("Error reading configuration file: %s\n"), error));
QMessageBox::critical(nullptr, PACKAGE_NAME,
QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error)));
return EXIT_FAILURE;
Expand All @@ -522,18 +525,18 @@ int GuiMain(int argc, char* argv[])

// Check for -chain, -testnet or -regtest parameter (Params() calls are only valid after this clause)
try {
node->selectParams(gArgs.GetChainName());
SelectParams(gArgs.GetChainName());
} catch(std::exception &e) {
node->initError(Untranslated(strprintf("%s\n", e.what())));
InitError(Untranslated(strprintf("%s\n", e.what())));
QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error: %1").arg(e.what()));
return EXIT_FAILURE;
}
#ifdef ENABLE_WALLET
// Parse URIs on command line -- this can affect Params()
PaymentServer::ipcParseCommandLine(*node, argc, argv);
#endif
if (!node->initSettings(error)) {
node->initError(Untranslated(error));
if (!gArgs.InitSettings(error)) {
InitError(Untranslated(error));
QMessageBox::critical(nullptr, PACKAGE_NAME, QObject::tr("Error initializing settings: %1").arg(QString::fromStdString(error)));
return EXIT_FAILURE;
}
Expand Down
7 changes: 4 additions & 3 deletions src/qt/intro.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <config/bitcoin-config.h>
#endif

#include <chainparams.h>
#include <fs.h>
#include <qt/intro.h>
#include <qt/forms/ui_intro.h>
Expand Down Expand Up @@ -199,13 +200,13 @@ bool Intro::showIfNeeded(interfaces::Node& node, bool& did_show_intro, bool& pru
{
/* Use selectParams here to guarantee Params() can be used by node interface */
try {
node.selectParams(gArgs.GetChainName());
SelectParams(gArgs.GetChainName());
} catch (const std::exception&) {
return false;
}

/* If current default data directory does not exist, let the user choose one */
Intro intro(0, node.getAssumedBlockchainSize(), node.getAssumedChainStateSize());
Intro intro(0, Params().AssumedBlockchainSize(), Params().AssumedChainStateSize());
intro.setDataDirectory(dataDir);
intro.setWindowIcon(QIcon(":icons/bitcoin"));
did_show_intro = true;
Expand Down Expand Up @@ -242,7 +243,7 @@ bool Intro::showIfNeeded(interfaces::Node& node, bool& did_show_intro, bool& pru
* (to be consistent with bitcoind behavior)
*/
if(dataDir != GUIUtil::getDefaultDataDirectory()) {
node.softSetArg("-datadir", GUIUtil::qstringToBoostPath(dataDir).string()); // use OS locale for path setting
gArgs.SoftSetArg("-datadir", GUIUtil::qstringToBoostPath(dataDir).string()); // use OS locale for path setting
}
return true;
}
Expand Down
20 changes: 10 additions & 10 deletions src/qt/optionsmodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,12 @@ void OptionsModel::Init(bool resetSettings)

if (!settings.contains("nDatabaseCache"))
settings.setValue("nDatabaseCache", (qint64)nDefaultDbCache);
if (!m_node.softSetArg("-dbcache", settings.value("nDatabaseCache").toString().toStdString()))
if (!gArgs.SoftSetArg("-dbcache", settings.value("nDatabaseCache").toString().toStdString()))
addOverriddenOption("-dbcache");

if (!settings.contains("nThreadsScriptVerif"))
settings.setValue("nThreadsScriptVerif", DEFAULT_SCRIPTCHECK_THREADS);
if (!m_node.softSetArg("-par", settings.value("nThreadsScriptVerif").toString().toStdString()))
if (!gArgs.SoftSetArg("-par", settings.value("nThreadsScriptVerif").toString().toStdString()))
addOverriddenOption("-par");

if (!settings.contains("strDataDir"))
Expand All @@ -112,27 +112,27 @@ void OptionsModel::Init(bool resetSettings)
#ifdef ENABLE_WALLET
if (!settings.contains("bSpendZeroConfChange"))
settings.setValue("bSpendZeroConfChange", true);
if (!m_node.softSetBoolArg("-spendzeroconfchange", settings.value("bSpendZeroConfChange").toBool()))
if (!gArgs.SoftSetBoolArg("-spendzeroconfchange", settings.value("bSpendZeroConfChange").toBool()))
addOverriddenOption("-spendzeroconfchange");
#endif

// Network
if (!settings.contains("fUseUPnP"))
settings.setValue("fUseUPnP", DEFAULT_UPNP);
if (!m_node.softSetBoolArg("-upnp", settings.value("fUseUPnP").toBool()))
if (!gArgs.SoftSetBoolArg("-upnp", settings.value("fUseUPnP").toBool()))
addOverriddenOption("-upnp");

if (!settings.contains("fListen"))
settings.setValue("fListen", DEFAULT_LISTEN);
if (!m_node.softSetBoolArg("-listen", settings.value("fListen").toBool()))
if (!gArgs.SoftSetBoolArg("-listen", settings.value("fListen").toBool()))
addOverriddenOption("-listen");

if (!settings.contains("fUseProxy"))
settings.setValue("fUseProxy", false);
if (!settings.contains("addrProxy"))
settings.setValue("addrProxy", GetDefaultProxyAddress());
// Only try to set -proxy, if user has enabled fUseProxy
if (settings.value("fUseProxy").toBool() && !m_node.softSetArg("-proxy", settings.value("addrProxy").toString().toStdString()))
if ((settings.value("fUseProxy").toBool() && !gArgs.SoftSetArg("-proxy", settings.value("addrProxy").toString().toStdString())))
addOverriddenOption("-proxy");
else if(!settings.value("fUseProxy").toBool() && !gArgs.GetArg("-proxy", "").empty())
addOverriddenOption("-proxy");
Expand All @@ -142,15 +142,15 @@ void OptionsModel::Init(bool resetSettings)
if (!settings.contains("addrSeparateProxyTor"))
settings.setValue("addrSeparateProxyTor", GetDefaultProxyAddress());
// Only try to set -onion, if user has enabled fUseSeparateProxyTor
if (settings.value("fUseSeparateProxyTor").toBool() && !m_node.softSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString()))
if ((settings.value("fUseSeparateProxyTor").toBool() && !gArgs.SoftSetArg("-onion", settings.value("addrSeparateProxyTor").toString().toStdString())))
addOverriddenOption("-onion");
else if(!settings.value("fUseSeparateProxyTor").toBool() && !gArgs.GetArg("-onion", "").empty())
addOverriddenOption("-onion");

// Display
if (!settings.contains("language"))
settings.setValue("language", "");
if (!m_node.softSetArg("-lang", settings.value("language").toString().toStdString()))
if (!gArgs.SoftSetArg("-lang", settings.value("language").toString().toStdString()))
addOverriddenOption("-lang");

language = settings.value("language").toString();
Expand Down Expand Up @@ -244,10 +244,10 @@ void OptionsModel::SetPruneEnabled(bool prune, bool force)
const int64_t prune_target_mib = PruneGBtoMiB(settings.value("nPruneSize").toInt());
std::string prune_val = prune ? ToString(prune_target_mib) : "0";
if (force) {
m_node.forceSetArg("-prune", prune_val);
gArgs.ForceSetArg("-prune", prune_val);
return;
}
if (!m_node.softSetArg("-prune", prune_val)) {
if (!gArgs.SoftSetArg("-prune", prune_val)) {
addOverriddenOption("-prune");
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/qt/paymentserver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,11 @@ void PaymentServer::ipcParseCommandLine(interfaces::Node& node, int argc, char*
auto tempChainParams = CreateChainParams(CBaseChainParams::MAIN);

if (IsValidDestinationString(r.address.toStdString(), *tempChainParams)) {
node.selectParams(CBaseChainParams::MAIN);
SelectParams(CBaseChainParams::MAIN);
} else {
tempChainParams = CreateChainParams(CBaseChainParams::TESTNET);
if (IsValidDestinationString(r.address.toStdString(), *tempChainParams)) {
node.selectParams(CBaseChainParams::TESTNET);
SelectParams(CBaseChainParams::TESTNET);
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/qt/test/test_main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ int main(int argc, char* argv[])
BitcoinApplication app(*node);
app.setApplicationName("Bitcoin-Qt-test");

node->setupServerArgs(); // Make gArgs available in the NodeContext
node->context()->args->ClearArgs(); // Clear added args again
node->context()->args = &gArgs; // Make gArgs available in the NodeContext
AppTests app_tests(app);
if (QTest::qExec(&app_tests) != 0) {
fInvalid = true;
Expand Down

0 comments on commit e133631

Please sign in to comment.