Skip to content

Commit

Permalink
Merge bitcoin-core/gui#35: Parse params directly instead of through n…
Browse files Browse the repository at this point in the history 
…ode (partial revert bitcoin#10244)

519cae8 gui: Delay interfaces::Node initialization (Russell Yanofsky)
102abff gui: Replace interface::Node references with pointers (Russell Yanofsky)
91aced7 gui: Remove unused interfaces::Node references (Russell Yanofsky)
e133631 gui: Partially revert bitcoin#10244 gArgs and Params changes (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  ---

  This is a partial revert of bitcoin#10244. It changes gui code to go back to using gArgs and Params() functions directly instead of using interfaces::Node to handle arguments.

  These changes were originally pushed as part of bitcoin#19461. Motivation is to support a new GUI process connecting to an already running node process. Details are explained in commit messages, but in addition to spawning a new bitcoin-node process, we want bitcoin-gui to connect to an existing bitcoin-node process. So for that reason it should be able to parse its own parameters, rather than rely on the node.

ACKs for top commit:
  MarcoFalke:
    re-ACK 519cae8, only change is rebase and addressed nits of my previous review last week 🌄

Tree-SHA512: 9c339dd82ba78bcc7b887b84d872f35ccc7dfa3d271691e6eafe8a2048cbbe3bdde1e810ce33d0714d75d048c9de3470e9e9b6f8306a6047d1cb3548f6858dc8
  • Loading branch information
@knst
knst committed Jan 16, 2024
1 parent cdd0874 commit b64c59c
Show file tree
Hide file tree
Showing 18 changed files with 152 additions and 170 deletions.
38 changes: 0 additions & 38 deletions src/interfaces/node.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,41 +130,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 @@ -195,9 +160,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, bool use_natpmp) = 0;

Expand Down
19 changes: 2 additions & 17 deletions src/node/interfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,22 +230,8 @@ class NodeImpl : public Node
CoinJoinOptionsImpl m_coinjoin;

explicit 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); }
uint64_t getLogCategories() override { return LogInstance().GetCategoryMask(); }
bool baseInitialize() override
Expand All @@ -271,7 +257,6 @@ class NodeImpl : public Node
void startShutdown() override { StartShutdown(); }
bool shutdownRequested() override { return ShutdownRequested(); }
void mapPort(bool use_upnp, bool use_natpmp) override { StartMapPort(use_upnp, use_natpmp); }
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
72 changes: 42 additions & 30 deletions src/qt/bitcoin.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@
#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 <stacktraces.h>
#include <uint256.h>
Expand All @@ -40,6 +42,7 @@
#include <util/translation.h>
#include <validation.h>

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

#include <QApplication>
Expand Down Expand Up @@ -198,10 +201,9 @@ void BitcoinCore::shutdown()
static int qt_argc = 1;
static const char* qt_argv = "dash-qt";

BitcoinApplication::BitcoinApplication(interfaces::Node& node):
BitcoinApplication::BitcoinApplication():
QApplication(qt_argc, const_cast<char **>(&qt_argv)),
coreThread(nullptr),
m_node(node),
optionsModel(nullptr),
clientModel(nullptr),
window(nullptr),
Expand Down Expand Up @@ -241,19 +243,19 @@ void BitcoinApplication::createPaymentServer()

void BitcoinApplication::createOptionsModel(bool resetSettings)
{
optionsModel = new OptionsModel(m_node, this, resetSettings);
optionsModel = new OptionsModel(this, resetSettings);
}

void BitcoinApplication::createWindow(const NetworkStyle *networkStyle)
{
window = new BitcoinGUI(m_node, networkStyle, nullptr);
window = new BitcoinGUI(node(), networkStyle, nullptr);
pollShutdownTimer = new QTimer(window);
connect(pollShutdownTimer, &QTimer::timeout, window, &BitcoinGUI::detectShutdown);
}

void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle)
{
SplashScreen *splash = new SplashScreen(m_node, networkStyle);
SplashScreen *splash = new SplashScreen(networkStyle);
// We don't hold a direct pointer to the splash screen after creation, but the splash
// screen will take care of deleting itself when finish() happens.
splash->show();
Expand All @@ -262,17 +264,25 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle)
connect(this, &BitcoinApplication::requestedShutdown, splash, &QWidget::close);
}

void BitcoinApplication::setNode(interfaces::Node& node)
{
assert(!m_node);
m_node = &node;
if (optionsModel) optionsModel->setNode(*m_node);
if (m_splash) m_splash->setNode(*m_node);
}

bool BitcoinApplication::baseInitialize()
{
return m_node.baseInitialize();
return node().baseInitialize();
}

void BitcoinApplication::startThread()
{
if(coreThread)
return;
coreThread = new QThread(this);
BitcoinCore *executor = new BitcoinCore(m_node);
BitcoinCore *executor = new BitcoinCore(node());
executor->moveToThread(coreThread);

/* communication to and from thread */
Expand All @@ -294,8 +304,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 All @@ -317,7 +327,7 @@ void BitcoinApplication::requestShutdown()
// Show a simple window indicating shutdown status
// Do this first as some of the steps may take some time below,
// for example the RPC console may still be executing a command.
shutdownWindow.reset(ShutdownWindow::showShutdownWindow(m_node, window));
shutdownWindow.reset(ShutdownWindow::showShutdownWindow(window));

qDebug() << __func__ << ": Requesting shutdown";
startThread();
Expand All @@ -327,7 +337,7 @@ void BitcoinApplication::requestShutdown()
window->unsubscribeFromCoreSignals();
// Request node shutdown, which can interrupt long operations, like
// rescanning a wallet.
m_node.startShutdown();
node().startShutdown();
// Unsetting the client model can cause the current thread to wait for node
// to complete an operation, like wait for a RPC execution to complete.
window->setClientModel(nullptr);
Expand Down Expand Up @@ -360,7 +370,7 @@ void BitcoinApplication::initializeResult(bool success, interfaces::BlockAndHead
{
// Log this only after AppInitMain finishes, as then logging setup is guaranteed complete
qInfo() << "Platform customization:" << gArgs.GetArg("-uiplatform", BitcoinGUI::DEFAULT_UIPLATFORM).c_str();
clientModel = new ClientModel(m_node, optionsModel);
clientModel = new ClientModel(node(), optionsModel);
window->setClientModel(clientModel, &tip_info);
#ifdef ENABLE_WALLET
if (WalletModel::isWalletEnabled()) {
Expand Down Expand Up @@ -467,9 +477,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 @@ -481,7 +491,7 @@ int GuiMain(int argc, char* argv[])
QApplication::setAttribute(Qt::AA_UseHighDpiPixmaps);
QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling);

BitcoinApplication app(*node);
BitcoinApplication app;

// Register meta types used for QMetaObject::invokeMethod and Qt::QueuedConnection
qRegisterMetaType<bool*>();
Expand All @@ -500,11 +510,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 @@ -534,7 +544,7 @@ int GuiMain(int argc, char* argv[])
// Show help message immediately after parsing command-line options (for "-lang") and setting locale,
// but before showing splash screen.
if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
HelpMessageDialog help(*node, nullptr, gArgs.IsArgSet("-version") ? HelpMessageDialog::about : HelpMessageDialog::cmdline);
HelpMessageDialog help(nullptr, gArgs.IsArgSet("-version") ? HelpMessageDialog::about : HelpMessageDialog::cmdline);
help.showOrPrint();
return EXIT_SUCCESS;
}
Expand All @@ -547,18 +557,18 @@ int GuiMain(int argc, char* argv[])
bool did_show_intro = false;
bool prune = false; // Intro dialog prune check box
// Gracefully exit if the user cancels
if (!Intro::showIfNeeded(*node, did_show_intro, prune)) return EXIT_SUCCESS;
if (!Intro::showIfNeeded(did_show_intro, prune)) return EXIT_SUCCESS;

/// 6. Determine availability of data directory and parse dash.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 @@ -572,18 +582,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);
PaymentServer::ipcParseCommandLine(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 Expand Up @@ -725,6 +735,8 @@ int GuiMain(int argc, char* argv[])
if (gArgs.GetBoolArg("-splash", DEFAULT_SPLASHSCREEN) && !gArgs.GetBoolArg("-min", false))
app.createSplashScreen(networkStyle.data());

app.setNode(*node);

int rv = EXIT_SUCCESS;
try
{
Expand All @@ -747,7 +759,7 @@ int GuiMain(int argc, char* argv[])
}
} catch (...) {
PrintExceptionContinue(std::current_exception(), "Runaway exception");
app.handleRunawayException(QString::fromStdString(node->getWarnings().translated));
app.handleRunawayException(QString::fromStdString(app.node().getWarnings().translated));
}
return rv;
}
10 changes: 8 additions & 2 deletions src/qt/bitcoin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#endif

#include <QApplication>
#include <assert.h>
#include <memory>

#include <interfaces/node.h>
Expand All @@ -19,6 +20,7 @@ class ClientModel;
class NetworkStyle;
class OptionsModel;
class PaymentServer;
class SplashScreen;
class WalletController;
class WalletModel;

Expand Down Expand Up @@ -54,7 +56,7 @@ class BitcoinApplication: public QApplication
{
Q_OBJECT
public:
explicit BitcoinApplication(interfaces::Node& node);
explicit BitcoinApplication();
~BitcoinApplication();

#ifdef ENABLE_WALLET
Expand Down Expand Up @@ -85,6 +87,9 @@ class BitcoinApplication: public QApplication
/// Get window identifier of QMainWindow (BitcoinGUI)
WId getMainWinId() const;

interfaces::Node& node() const { assert(m_node); return *m_node; }
void setNode(interfaces::Node& node);

public Q_SLOTS:
void initializeResult(bool success, interfaces::BlockAndHeaderTipInfo tip_info);
void shutdownResult();
Expand All @@ -103,7 +108,6 @@ public Q_SLOTS:

private:
QThread *coreThread;
interfaces::Node& m_node;
OptionsModel *optionsModel;
ClientModel *clientModel;
BitcoinGUI *window;
Expand All @@ -114,6 +118,8 @@ public Q_SLOTS:
#endif
int returnValue;
std::unique_ptr<QWidget> shutdownWindow;
SplashScreen* m_splash = nullptr;
interfaces::Node* m_node = nullptr;

void startThread();
};
Expand Down
6 changes: 3 additions & 3 deletions src/qt/bitcoingui.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const NetworkStyle* networkStyle,
updateWindowTitle();

rpcConsole = new RPCConsole(node, this, enableWallet ? Qt::Window : Qt::Widget);
helpMessageDialog = new HelpMessageDialog(node, this, HelpMessageDialog::cmdline);
helpMessageDialog = new HelpMessageDialog(this, HelpMessageDialog::cmdline);
#ifdef ENABLE_WALLET
if(enableWallet)
{
Expand Down Expand Up @@ -1078,7 +1078,7 @@ void BitcoinGUI::aboutClicked()
if(!clientModel)
return;

HelpMessageDialog dlg(m_node, this, HelpMessageDialog::about);
HelpMessageDialog dlg(this, HelpMessageDialog::about);
dlg.exec();
}

Expand Down Expand Up @@ -1138,7 +1138,7 @@ void BitcoinGUI::showCoinJoinHelpClicked()
if(!clientModel)
return;

HelpMessageDialog dlg(m_node, this, HelpMessageDialog::pshelp);
HelpMessageDialog dlg(this, HelpMessageDialog::pshelp);
dlg.exec();
}

Expand Down
Loading

0 comments on commit b64c59c

Please sign in to comment.