From d7a20b3ee6882049858a958f2e32efa85978eb93 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Thu, 19 Aug 2021 10:03:18 +1200 Subject: [PATCH] Merge bitcoin/bitcoin#22217: refactor: Avoid wallet code writing node settings file 49ee2a0ad88e0e656234b769d806987784ff1e28 Avoid wallet code writing node settings file (Russell Yanofsky) Pull request description: Change wallet loading code to access settings through the Chain interface instead of writing settings.json directly. This is for running wallet and node in separate processes, since multiprocess code wouldn't easily work with different processes updating the same file. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102. ACKs for top commit: jamesob: ACK 49ee2a0ad88e0e656234b769d806987784ff1e28 ([`jamesob/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co`](https://github.com/jamesob/bitcoin/tree/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co)) ryanofsky: > ACK [49ee2a0](https://github.com/bitcoin/bitcoin/commit/49ee2a0ad88e0e656234b769d806987784ff1e28) ([`jamesob/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co`](https://github.com/jamesob/bitcoin/tree/ackr/22217.1.ryanofsky.refactor_avoid_wallet_co)) Zero-1729: crACK 49ee2a0ad88e0e656234b769d806987784ff1e28 meshcollider: Code review ACK 49ee2a0ad88e0e656234b769d806987784ff1e28 Tree-SHA512: a81c63b87816f739e02e3992808f314294d6c7213babaafdaaf3c4650ebc97ee4f98f9a4684ce4ff87372df59989b8ad5929159c5686293a7cce04e97e2fabba --- src/interfaces/chain.h | 11 +++++++++-- src/node/interfaces.cpp | 12 ++++++++++-- src/util/system.h | 2 +- src/wallet/load.cpp | 17 ++++++++++------- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 37f434591d..ea5fe712ab 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -285,11 +285,18 @@ public: //! Run function after given number of seconds. Cancel any previous calls with same name. virtual void rpcRunLater(const std::string& name, std::function fn, int64_t seconds) = 0; + //! Get settings value. + virtual util::SettingsValue getSetting(const std::string& arg) = 0; + + //! Get list of settings values. + virtual std::vector getSettingsList(const std::string& arg) = 0; + //! Return /settings.json setting value. virtual util::SettingsValue getRwSetting(const std::string& name) = 0; - //! Write a setting to /settings.json. - virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0; + //! Write a setting to /settings.json. Optionally just update the + //! setting in memory and do not write the file. + virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write=true) = 0; //! Synchronously send transactionAddedToMempool notifications about all //! current mempool transactions to the specified handler and return after diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index d29e95a3e4..d680ab8488 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -971,6 +971,14 @@ public: { RPCRunLater(name, std::move(fn), seconds); } + util::SettingsValue getSetting(const std::string& name) override + { + return gArgs.GetSetting(name); + } + std::vector getSettingsList(const std::string& name) override + { + return gArgs.GetSettingsList(name); + } util::SettingsValue getRwSetting(const std::string& name) override { util::SettingsValue result; @@ -981,7 +989,7 @@ public: }); return result; } - bool updateRwSetting(const std::string& name, const util::SettingsValue& value) override + bool updateRwSetting(const std::string& name, const util::SettingsValue& value, bool write) override { gArgs.LockSettings([&](util::Settings& settings) { if (value.isNull()) { @@ -990,7 +998,7 @@ public: settings.rw_settings[name] = value; } }); - return gArgs.WriteSettingsFile(); + return !write || gArgs.WriteSettingsFile(); } void requestMempoolTransactions(Notifications& notifications) override { diff --git a/src/util/system.h b/src/util/system.h index 591d50099f..85c46eaa50 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -225,6 +225,7 @@ protected: */ bool UseDefaultSection(const std::string& arg) const EXCLUSIVE_LOCKS_REQUIRED(cs_args); + public: /** * Get setting value. * @@ -239,7 +240,6 @@ protected: */ std::vector GetSettingsList(const std::string& arg) const; -public: ArgsManager(); ~ArgsManager(); diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index f5091b3794..bef1c57471 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -57,18 +57,20 @@ bool VerifyWallets(interfaces::Chain& chain) options.require_existing = true; options.verify = false; if (MakeWalletDatabase("", options, status, error_string)) { - gArgs.LockSettings([&](util::Settings& settings) { - util::SettingsValue wallets(util::SettingsValue::VARR); - wallets.push_back(""); // Default wallet name is "" - settings.rw_settings["wallet"] = wallets; - }); + util::SettingsValue wallets(util::SettingsValue::VARR); + wallets.push_back(""); // Default wallet name is "" + // Pass write=false because no need to write file and probably + // better not to. If unnamed wallet needs to be added next startup + // and the setting is empty, this code will just run again. + chain.updateRwSetting("wallet", wallets, /* write= */ false); } } // Keep track of each wallet absolute path to detect duplicates. std::set wallet_paths; - for (const auto& wallet_file : gArgs.GetArgs("-wallet")) { + for (const auto& wallet : chain.getSettingsList("wallet")) { + const auto& wallet_file = wallet.get_str(); const fs::path path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(wallet_file)); if (!wallet_paths.insert(path).second) { @@ -98,7 +100,8 @@ bool LoadWallets(interfaces::Chain& chain, interfaces::CoinJoin::Loader& coinjoi { try { std::set wallet_paths; - for (const std::string& name : gArgs.GetArgs("-wallet")) { + for (const auto& wallet : chain.getSettingsList("wallet")) { + const auto& name = wallet.get_str(); if (!wallet_paths.insert(fs::PathFromString(name)).second) { continue; }