diff --git a/configure.ac b/configure.ac index 65285366328b..00701c533945 100644 --- a/configure.ac +++ b/configure.ac @@ -398,6 +398,7 @@ if test "x$enable_werror" = "xyes"; then AC_MSG_ERROR("enable-werror set but -Werror is not usable") fi AX_CHECK_COMPILE_FLAG([-Werror=vla],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=vla"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Werror=shadow-field],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=shadow-field"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=switch],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=switch"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=thread-safety],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=thread-safety"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Werror=unused-variable],[ERROR_CXXFLAGS="$ERROR_CXXFLAGS -Werror=unused-variable"],,[[$CXXFLAG_WERROR]]) @@ -416,6 +417,7 @@ if test "x$CXXFLAGS_overridden" = "xno"; then AX_CHECK_COMPILE_FLAG([-Wextra],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wextra"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wvla],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wvla"],,[[$CXXFLAG_WERROR]]) + AX_CHECK_COMPILE_FLAG([-Wshadow-field],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wshadow-field"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wswitch],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wswitch"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wformat-security],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wformat-security"],,[[$CXXFLAG_WERROR]]) AX_CHECK_COMPILE_FLAG([-Wthread-safety],[WARN_CXXFLAGS="$WARN_CXXFLAGS -Wthread-safety"],,[[$CXXFLAG_WERROR]]) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index bf18fe7b58b5..e5efb3012286 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -212,6 +212,7 @@ BITCOIN_TESTS =\ test/script_standard_tests.cpp \ test/scriptnum_tests.cpp \ test/serialize_tests.cpp \ + test/settings_tests.cpp \ test/sighash_tests.cpp \ test/sigopcount_tests.cpp \ test/skiplist_tests.cpp \ diff --git a/src/bench/bench.cpp b/src/bench/bench.cpp index 8f26d92d5181..c168b1c48141 100644 --- a/src/bench/bench.cpp +++ b/src/bench/bench.cpp @@ -53,11 +53,6 @@ void benchmark::BenchRunner::RunAll(const Args& args) std::vector benchmarkResults; for (const auto& p : benchmarks()) { - RegTestingSetup test{}; - { - assert(::ChainActive().Height() == 0); - } - if (!std::regex_match(p.first, baseMatch, reFilter)) { continue; } diff --git a/src/bench/bench_dash.cpp b/src/bench/bench_dash.cpp index 744a0650b848..612327bae7f0 100644 --- a/src/bench/bench_dash.cpp +++ b/src/bench/bench_dash.cpp @@ -61,6 +61,8 @@ int main(int argc, char** argv) args.output_csv = gArgs.GetArg("-output_csv", ""); args.output_json = gArgs.GetArg("-output_json", ""); + gArgs.ClearArgs(); // gArgs no longer needed. Clear it here to avoid interactions with the testing setup in the benches + benchmark::BenchRunner::RunAll(args); return EXIT_SUCCESS; diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 1c48c16bef0f..222f57c8c214 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -41,7 +41,7 @@ static void AssembleBlock(benchmark::Bench& bench) for (const auto& txr : txs) { CValidationState state; - bool ret{::AcceptToMemoryPool(::mempool, state, txr, nullptr /* pfMissingInputs */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; + bool ret{::AcceptToMemoryPool(*test_setup.m_node.mempool, state, txr, nullptr /* pfMissingInputs */, false /* bypass_limits */, /* nAbsurdFee */ 0)}; assert(ret); } } diff --git a/src/bench/wallet_balance.cpp b/src/bench/wallet_balance.cpp index 7f7317e54eb9..e88acebd9f83 100644 --- a/src/bench/wallet_balance.cpp +++ b/src/bench/wallet_balance.cpp @@ -23,9 +23,8 @@ static void WalletBalance(benchmark::Bench& bench, const bool set_dirty, const b { bool first_run; if (wallet.LoadWallet(first_run) != DBErrors::LOAD_OK) assert(false); - wallet.handleNotifications(); } - + auto handler = chain->handleNotifications({ &wallet, [](CWallet*) {} }); const std::optional address_mine{add_mine ? std::optional{getnewaddress(wallet)} : std::nullopt}; if (add_watchonly) importaddress(wallet, ADDRESS_WATCHONLY); diff --git a/src/dash-cli.cpp b/src/dash-cli.cpp index 0e8ddca6669f..de7a522c3cd4 100644 --- a/src/dash-cli.cpp +++ b/src/dash-cli.cpp @@ -138,8 +138,7 @@ static int AppInitRPC(int argc, char* argv[]) } return EXIT_SUCCESS; } - bool datadirFromCmdLine = gArgs.IsArgSet("-datadir"); - if (datadirFromCmdLine && !fs::is_directory(GetDataDir(false))) { + if (!CheckDataDirOption()) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")); return EXIT_FAILURE; } @@ -147,10 +146,6 @@ static int AppInitRPC(int argc, char* argv[]) tfm::format(std::cerr, "Error reading configuration file: %s\n", error); return EXIT_FAILURE; } - if (!datadirFromCmdLine && !fs::is_directory(GetDataDir(false))) { - tfm::format(std::cerr, "Error: Specified data directory \"%s\" from config file does not exist.\n", gArgs.GetArg("-datadir", "")); - return EXIT_FAILURE; - } // Check for -testnet or -regtest parameter (BaseParams() calls are only valid after this clause) try { SelectBaseParams(gArgs.GetChainName()); diff --git a/src/dash-wallet.cpp b/src/dash-wallet.cpp index a8fa26f8ef84..b35280ebc5cc 100644 --- a/src/dash-wallet.cpp +++ b/src/dash-wallet.cpp @@ -59,7 +59,7 @@ static bool WalletAppInit(int argc, char* argv[]) // check for printtoconsole, allow -debug LogInstance().m_print_to_console = gArgs.GetBoolArg("-printtoconsole", gArgs.GetBoolArg("-debug", false)); - if (!fs::is_directory(GetDataDir(false))) { + if (!CheckDataDirOption()) { tfm::format(std::cerr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")); return false; } diff --git a/src/dashd.cpp b/src/dashd.cpp index effadee7f10a..5e16b3c89afb 100644 --- a/src/dashd.cpp +++ b/src/dashd.cpp @@ -55,7 +55,7 @@ static bool AppInit(int argc, char* argv[]) // Parameters // // If Qt is used, parameters/dash.conf are parsed in qt/dash.cpp's main() - SetupServerArgs(); + SetupServerArgs(node); std::string error; if (!gArgs.ParseParameters(argc, argv, error)) { return InitError(Untranslated(strprintf("Error parsing command line arguments: %s\n", error))); @@ -87,19 +87,12 @@ static bool AppInit(int argc, char* argv[]) util::Ref context{node}; try { - bool datadirFromCmdLine = gArgs.IsArgSet("-datadir"); - if (datadirFromCmdLine && !fs::is_directory(GetDataDir(false))) - { + if (!CheckDataDirOption()) { return InitError(Untranslated(strprintf("Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "")))); } if (!gArgs.ReadConfigFiles(error, true)) { return InitError(Untranslated(strprintf("Error reading configuration file: %s\n", error))); } - if (!datadirFromCmdLine && !fs::is_directory(GetDataDir(false))) - { - tfm::format(std::cerr, "Error: Specified data directory \"%s\" from config file does not exist.\n", gArgs.GetArg("-datadir", "")); - return EXIT_FAILURE; - } // Check for -testnet or -regtest parameter (Params() calls are only valid after this clause) try { SelectParams(gArgs.GetChainName()); diff --git a/src/init.cpp b/src/init.cpp index f687dfb98c9b..4653f819d71e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -395,6 +395,7 @@ void Shutdown(NodeContext& node) // Shutdown part 2: delete wallet instance globalVerifyHandle.reset(); ECC_Stop(); + node.args = nullptr; node.mempool = nullptr; node.chainman = nullptr; node.scheduler.reset(); @@ -465,8 +466,11 @@ std::string GetSupportedSocketEventsStr() return strSupportedModes; } -void SetupServerArgs() +void SetupServerArgs(NodeContext& node) { + assert(!node.args); + node.args = &gArgs; + const auto defaultBaseParams = CreateBaseChainParams(CBaseChainParams::MAIN); const auto testnetBaseParams = CreateBaseChainParams(CBaseChainParams::TESTNET); const auto regtestBaseParams = CreateBaseChainParams(CBaseChainParams::REGTEST); @@ -2023,13 +2027,15 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA // If the loaded chain has a wrong genesis, bail out immediately // (we're likely using a testnet datadir, or the other way around). - if (!::BlockIndex().empty() && + if (!chainman.BlockIndex().empty() && !LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) { return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?")); } - if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !::BlockIndex().empty() && ::BlockIndex().count(chainparams.GetConsensus().hashDevnetGenesisBlock) == 0) + if (!chainparams.GetConsensus().hashDevnetGenesisBlock.IsNull() && !chainman.BlockIndex().empty() && + !LookupBlockIndex(chainparams.GetConsensus().hashDevnetGenesisBlock)) { return InitError(_("Incorrect or no devnet genesis block found. Wrong datadir for devnet specified?")); + } // Check for changed -addressindex state if (fAddressIndex != gArgs.GetBoolArg("-addressindex", DEFAULT_ADDRESSINDEX)) { @@ -2448,13 +2454,13 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA //// debug print { LOCK(cs_main); - LogPrintf("block tree size = %u\n", ::BlockIndex().size()); - chain_active_height = ::ChainActive().Height(); + LogPrintf("block tree size = %u\n", chainman.BlockIndex().size()); + chain_active_height = chainman.ActiveChain().Height(); if (tip_info) { tip_info->block_height = chain_active_height; - tip_info->block_time = ::ChainActive().Tip() ? ::ChainActive().Tip()->GetBlockTime() : Params().GenesisBlock().GetBlockTime(); - tip_info->block_hash = ::ChainActive().Tip() ? ::ChainActive().Tip()->GetBlockHash() : Params().GenesisBlock().GetHash(); - tip_info->verification_progress = GuessVerificationProgress(Params().TxData(), ::ChainActive().Tip()); + tip_info->block_time = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockTime() : Params().GenesisBlock().GetBlockTime(); + tip_info->block_hash = chainman.ActiveChain().Tip() ? chainman.ActiveChain().Tip()->GetBlockHash() : Params().GenesisBlock().GetHash(); + tip_info->verification_progress = GuessVerificationProgress(Params().TxData(), chainman.ActiveChain().Tip()); } if (tip_info && ::pindexBestHeader) { tip_info->header_height = ::pindexBestHeader->nHeight; diff --git a/src/init.h b/src/init.h index fbd38feb4f1e..81a0ebe0561c 100644 --- a/src/init.h +++ b/src/init.h @@ -61,9 +61,9 @@ bool AppInitMain(const util::Ref& context, NodeContext& node, interfaces::BlockA void PrepareShutdown(NodeContext& node); /** - * Setup the arguments for gArgs + * Register all arguments with the ArgsManager */ -void SetupServerArgs(); +void SetupServerArgs(NodeContext& node); /** Returns licensing information (for -version) */ std::string LicenseInfo(); diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 0b68ced5453f..f59c2d41e281 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -36,22 +36,12 @@ namespace interfaces { namespace { -class NotificationsHandlerImpl : public Handler, CValidationInterface +class NotificationsProxy : public CValidationInterface { public: - explicit NotificationsHandlerImpl(Chain& chain, Chain::Notifications& notifications) - : m_chain(chain), m_notifications(¬ifications) - { - RegisterValidationInterface(this); - } - ~NotificationsHandlerImpl() override { disconnect(); } - void disconnect() override - { - if (m_notifications) { - m_notifications = nullptr; - UnregisterValidationInterface(this); - } - } + explicit NotificationsProxy(std::shared_ptr notifications) + : m_notifications(std::move(notifications)) {} + virtual ~NotificationsProxy() = default; void TransactionAddedToMempool(const CTransactionRef& tx, int64_t nAcceptTime) override { m_notifications->TransactionAddedToMempool(tx, nAcceptTime); @@ -83,8 +73,27 @@ class NotificationsHandlerImpl : public Handler, CValidationInterface { m_notifications->NotifyTransactionLock(tx, islock); } - Chain& m_chain; - Chain::Notifications* m_notifications; + std::shared_ptr m_notifications; +}; + + +class NotificationsHandlerImpl : public Handler +{ +public: + explicit NotificationsHandlerImpl(std::shared_ptr notifications) + : m_proxy(std::make_shared(std::move(notifications))) + { + RegisterSharedValidationInterface(m_proxy); + } + ~NotificationsHandlerImpl() override { disconnect(); } + void disconnect() override + { + if (m_proxy) { + UnregisterSharedValidationInterface(m_proxy); + m_proxy.reset(); + } + } + std::shared_ptr m_proxy; }; class RpcHandlerImpl : public Handler @@ -325,9 +334,9 @@ class ChainImpl : public Chain { ::uiInterface.ShowProgress(title, progress, resume_possible); } - std::unique_ptr handleNotifications(Notifications& notifications) override + std::unique_ptr handleNotifications(std::shared_ptr notifications) override { - return MakeUnique(*this, notifications); + return MakeUnique(std::move(notifications)); } void waitForNotificationsIfTipChanged(const uint256& old_tip) override { diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 7b49773dc08d..3c0d2d1d0194 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -223,7 +223,7 @@ class Chain }; //! Register handler for notifications. - virtual std::unique_ptr handleNotifications(Notifications& notifications) = 0; + virtual std::unique_ptr handleNotifications(std::shared_ptr notifications) = 0; //! Wait for pending notifications to be processed unless block hash points to the current //! chain tip. diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp index 86c1e9ff74da..3058133ca0f8 100644 --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -217,7 +217,7 @@ 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(); } + 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 { diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index cae27e10fd49..52e228b77e56 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -45,11 +45,11 @@ class CSigShare : virtual public CSigBase return quorumMember; } - CSigShare(Consensus::LLMQType llmqType, const uint256 &quorumHash, const uint256 &id, const uint256 &msgHash, - uint16_t quorumMember, const CBLSLazySignature &sigShare) : - CSigBase(llmqType, quorumHash, id, msgHash), - quorumMember(quorumMember), - sigShare(sigShare) {}; + CSigShare(Consensus::LLMQType _llmqType, const uint256& _quorumHash, const uint256& _id, const uint256& _msgHash, + uint16_t _quorumMember, const CBLSLazySignature& _sigShare) : + CSigBase(_llmqType, _quorumHash, _id, _msgHash), + quorumMember(_quorumMember), + sigShare(_sigShare) {}; // This should only be used for serialization CSigShare() = default; @@ -83,8 +83,8 @@ class CSigSesAnn : virtual public CSigBase uint32_t sessionId{UNINITIALIZED_SESSION_ID}; public: - CSigSesAnn(uint32_t sessionId, Consensus::LLMQType llmqType, const uint256& quorumHash, const uint256& id, - const uint256& msgHash) : CSigBase(llmqType, quorumHash, id, msgHash), sessionId(sessionId) {}; + CSigSesAnn(uint32_t _sessionId, Consensus::LLMQType _llmqType, const uint256& _quorumHash, const uint256& _id, + const uint256& _msgHash) : CSigBase(_llmqType, _quorumHash, _id, _msgHash), sessionId(_sessionId) {}; // ONLY FOR SERIALIZATION CSigSesAnn() = default; diff --git a/src/node/context.h b/src/node/context.h index 7cbd4ab24ff0..44fd78e81215 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -10,6 +10,7 @@ #include #include +class ArgsManager; class BanMan; class CConnman; class CScheduler; @@ -37,6 +38,7 @@ struct NodeContext { std::unique_ptr peer_logic; ChainstateManager* chainman{nullptr}; // Currently a raw pointer because the memory is not managed by this struct std::unique_ptr banman; + ArgsManager* args{nullptr}; // Currently a raw pointer because the memory is not managed by this struct std::unique_ptr chain; std::vector> chain_clients; std::unique_ptr scheduler; diff --git a/src/qt/dash.cpp b/src/qt/dash.cpp index 0e6468d507b7..9d862046c12c 100644 --- a/src/qt/dash.cpp +++ b/src/qt/dash.cpp @@ -516,10 +516,9 @@ int GuiMain(int argc, char* argv[]) if (!Intro::pickDataDirectory(*node)) return EXIT_SUCCESS; - /// 6. Determine availability of data and blocks directory and parse dash.conf + /// 6. Determine availability of data directory and parse dash.conf /// - Do not call GetDataDir(true) before this step finishes - if (!fs::is_directory(GetDataDir(false))) - { + if (!CheckDataDirOption()) { node->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", "")))); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 19f04fd295a1..481e099afab8 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -824,10 +824,10 @@ void RPCConsole::buildParameterlist(QString arg) for (const auto& [key, values] : gArgs.GetCommandLineArgs()) { for (const auto& value : values) { - if (value.empty()) { - args << QString::fromStdString(key); + if (value.getValStr().empty()) { + args << QString::fromStdString("-" + key); } else { - args << QString::fromStdString(key + "=" + value); + args << QString::fromStdString("-" + key + "=" + value.getValStr()); } } } diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp index 8e3a0b59c531..9e5847a1c7f8 100644 --- a/src/qt/walletcontroller.cpp +++ b/src/qt/walletcontroller.cpp @@ -115,7 +115,7 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr 0); + result.pushKV("known_block", LookupBlockIndex(clsig.getBlockHash()) != nullptr); return result; } @@ -1616,36 +1616,35 @@ static UniValue getchaintips(const JSONRPCRequest& request) }, }.Check(request); + ChainstateManager& chainman = EnsureChainman(request.context); LOCK(cs_main); /* - * Idea: the set of chain tips is ::ChainActive().tip, plus orphan blocks which do not have another orphan building off of them. + * Idea: The set of chain tips is the active chain tip, plus orphan blocks which do not have another orphan building off of them. * Algorithm: * - Make one pass through BlockIndex(), picking out the orphan blocks, and also storing a set of the orphan block's pprev pointers. * - Iterate through the orphan blocks. If the block isn't pointed to by another orphan, it is a chain tip. - * - add ::ChainActive().Tip() + * - Add the active chain tip */ std::set setTips; std::set setOrphans; std::set setPrevs; - for (const std::pair& item : ::BlockIndex()) - { - if (!::ChainActive().Contains(item.second)) { + for (const std::pair& item : chainman.BlockIndex()) { + if (!chainman.ActiveChain().Contains(item.second)) { setOrphans.insert(item.second); setPrevs.insert(item.second->pprev); } } - for (std::set::iterator it = setOrphans.begin(); it != setOrphans.end(); ++it) - { + for (std::set::iterator it = setOrphans.begin(); it != setOrphans.end(); ++it) { if (setPrevs.erase(*it) == 0) { setTips.insert(*it); } } // Always report the currently active tip. - setTips.insert(::ChainActive().Tip()); + setTips.insert(chainman.ActiveChain().Tip()); int nBranchMin = -1; int nCountMax = INT_MAX; @@ -1660,7 +1659,7 @@ static UniValue getchaintips(const JSONRPCRequest& request) UniValue res(UniValue::VARR); for (const CBlockIndex* block : setTips) { - const CBlockIndex* pindexFork = ::ChainActive().FindFork(block); + const CBlockIndex* pindexFork = chainman.ActiveChain().FindFork(block); const int branchLen = block->nHeight - pindexFork->nHeight; if(branchLen < nBranchMin) continue; @@ -1675,7 +1674,7 @@ static UniValue getchaintips(const JSONRPCRequest& request) obj.pushKV("forkpoint", pindexFork->phashBlock->GetHex()); std::string status; - if (::ChainActive().Contains(block)) { + if (chainman.ActiveChain().Contains(block)) { // This block is part of the currently active chain. status = "active"; } else if (block->nStatus & BLOCK_FAILED_MASK) { diff --git a/src/test/block_reward_reallocation_tests.cpp b/src/test/block_reward_reallocation_tests.cpp index 804eb95e8aac..6005d66fa47b 100644 --- a/src/test/block_reward_reallocation_tests.cpp +++ b/src/test/block_reward_reallocation_tests.cpp @@ -36,7 +36,7 @@ struct TestChainBRRBeforeActivationSetup : public TestChainSetup // Force fast DIP3 activation gArgs.ForceSetArg("-dip3params", "30:50"); SelectParams(CBaseChainParams::REGTEST); - gArgs.ForceRemoveArg("-dip3params"); + gArgs.ForceRemoveArg("dip3params"); } }; @@ -164,7 +164,7 @@ BOOST_FIXTURE_TEST_CASE(block_reward_reallocation, TestChainBRRBeforeActivationS LOCK(cs_main); deterministicMNManager->UpdatedBlockTip(::ChainActive().Tip()); } - gArgs.ForceRemoveArg("-blockversion"); + gArgs.ForceRemoveArg("blockversion"); if (num_blocks > 0) { // Mine signalling blocks for (int i = 0; i < num_blocks; ++i) { diff --git a/src/test/dynamic_activation_thresholds_tests.cpp b/src/test/dynamic_activation_thresholds_tests.cpp index 82ffab667d6c..e4e71bf8ec88 100644 --- a/src/test/dynamic_activation_thresholds_tests.cpp +++ b/src/test/dynamic_activation_thresholds_tests.cpp @@ -37,7 +37,7 @@ struct TestChainDATSetup : public TestChainSetup for (int i = 0; i < window - num_blocks; ++i) { CreateAndProcessBlock({}, coinbaseKey); } - gArgs.ForceRemoveArg("-blockversion"); + gArgs.ForceRemoveArg("blockversion"); if (num_blocks > 0) { // Mine signalling blocks for (int i = 0; i < num_blocks; ++i) { diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 2a6f8f262264..1dfd606e6936 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -57,12 +57,17 @@ const std::map> EXPECTED_DESERIALIZATION_EXCE {"Unknown transaction optional data: iostream error", {"block", "blocktxn", "cmpctblock", "tx"}}, }; -const RegTestingSetup* g_setup; +const TestingSetup* g_setup; } // namespace void initialize() { - static RegTestingSetup setup{}; + static TestingSetup setup{ + CBaseChainParams::REGTEST, + { + "-nodebuglogfile", + }, + }; g_setup = &setup; for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp index 8fdb26181f54..2651886ac874 100644 --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -30,7 +30,7 @@ static void ResetArgs(const std::string& strArg) vecChar.push_back(s.c_str()); std::string error; - gArgs.ParseParameters(vecChar.size(), vecChar.data(), error); + BOOST_CHECK(gArgs.ParseParameters(vecChar.size(), vecChar.data(), error)); } static void SetupArgs(const std::vector>& args) diff --git a/src/test/settings_tests.cpp b/src/test/settings_tests.cpp new file mode 100644 index 000000000000..607e2ad2ddad --- /dev/null +++ b/src/test/settings_tests.cpp @@ -0,0 +1,163 @@ +// Copyright (c) 2011-2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +#include +#include +#include +#include + +BOOST_FIXTURE_TEST_SUITE(settings_tests, BasicTestingSetup) + +//! Check settings struct contents against expected json strings. +static void CheckValues(const util::Settings& settings, const std::string& single_val, const std::string& list_val) +{ + util::SettingsValue single_value = GetSetting(settings, "section", "name", false, false); + util::SettingsValue list_value(util::SettingsValue::VARR); + for (const auto& item : GetSettingsList(settings, "section", "name", false)) { + list_value.push_back(item); + } + BOOST_CHECK_EQUAL(single_value.write().c_str(), single_val); + BOOST_CHECK_EQUAL(list_value.write().c_str(), list_val); +}; + +// Simple settings merge test case. +BOOST_AUTO_TEST_CASE(Simple) +{ + util::Settings settings; + settings.command_line_options["name"].push_back("val1"); + settings.command_line_options["name"].push_back("val2"); + settings.ro_config["section"]["name"].push_back(2); + + // The last given arg takes precedence when specified via commandline. + CheckValues(settings, R"("val2")", R"(["val1","val2",2])"); + + util::Settings settings2; + settings2.ro_config["section"]["name"].push_back("val2"); + settings2.ro_config["section"]["name"].push_back("val3"); + + // The first given arg takes precedence when specified via config file. + CheckValues(settings2, R"("val2")", R"(["val2","val3"])"); +} + +// Test different ways settings can be merged, and verify results. This test can +// be used to confirm that updates to settings code don't change behavior +// unintentionally. +struct MergeTestingSetup : public BasicTestingSetup { + //! Max number of actions to sequence together. Can decrease this when + //! debugging to make test results easier to understand. + static constexpr int MAX_ACTIONS = 3; + + enum Action { END, SET, NEGATE, SECTION_SET, SECTION_NEGATE }; + using ActionList = Action[MAX_ACTIONS]; + + //! Enumerate all possible test configurations. + template + void ForEachMergeSetup(Fn&& fn) + { + ActionList arg_actions = {}; + // command_line_options do not have sections. Only iterate over SET and NEGATE + ForEachNoDup(arg_actions, SET, NEGATE, [&]{ + ActionList conf_actions = {}; + ForEachNoDup(conf_actions, SET, SECTION_NEGATE, [&]{ + for (bool force_set : {false, true}) { + for (bool ignore_default_section_config : {false, true}) { + fn(arg_actions, conf_actions, force_set, ignore_default_section_config); + } + } + }); + }); + } +}; + +// Regression test covering different ways config settings can be merged. The +// test parses and merges settings, representing the results as strings that get +// compared against an expected hash. To debug, the result strings can be dumped +// to a file (see comments below). +BOOST_FIXTURE_TEST_CASE(Merge, MergeTestingSetup) +{ + CHash256 out_sha; + FILE* out_file = nullptr; + if (const char* out_path = getenv("SETTINGS_MERGE_TEST_OUT")) { + out_file = fsbridge::fopen(out_path, "w"); + if (!out_file) throw std::system_error(errno, std::generic_category(), "fopen failed"); + } + + const std::string& network = CBaseChainParams::MAIN; + ForEachMergeSetup([&](const ActionList& arg_actions, const ActionList& conf_actions, bool force_set, + bool ignore_default_section_config) { + std::string desc; + int value_suffix = 0; + util::Settings settings; + + const std::string& name = ignore_default_section_config ? "wallet" : "server"; + auto push_values = [&](Action action, const char* value_prefix, const std::string& name_prefix, + std::vector& dest) { + if (action == SET || action == SECTION_SET) { + for (int i = 0; i < 2; ++i) { + dest.push_back(value_prefix + std::to_string(++value_suffix)); + desc += " " + name_prefix + name + "=" + dest.back().get_str(); + } + } else if (action == NEGATE || action == SECTION_NEGATE) { + dest.push_back(false); + desc += " " + name_prefix + "no" + name; + } + }; + + if (force_set) { + settings.forced_settings[name] = "forced"; + desc += " " + name + "=forced"; + } + for (Action arg_action : arg_actions) { + push_values(arg_action, "a", "-", settings.command_line_options[name]); + } + for (Action conf_action : conf_actions) { + bool use_section = conf_action == SECTION_SET || conf_action == SECTION_NEGATE; + push_values(conf_action, "c", use_section ? network + "." : "", + settings.ro_config[use_section ? network : ""][name]); + } + + desc += " || "; + desc += GetSetting(settings, network, name, ignore_default_section_config, /* get_chain_name= */ false).write(); + desc += " |"; + for (const auto& s : GetSettingsList(settings, network, name, ignore_default_section_config)) { + desc += " "; + desc += s.write(); + } + desc += " |"; + if (OnlyHasDefaultSectionSetting(settings, network, name)) desc += " ignored"; + desc += "\n"; + + out_sha.Write(MakeUCharSpan(desc)); + if (out_file) { + BOOST_REQUIRE(fwrite(desc.data(), 1, desc.size(), out_file) == desc.size()); + } + }); + + if (out_file) { + if (fclose(out_file)) throw std::system_error(errno, std::generic_category(), "fclose failed"); + out_file = nullptr; + } + + unsigned char out_sha_bytes[CSHA256::OUTPUT_SIZE]; + out_sha.Finalize(out_sha_bytes); + std::string out_sha_hex = HexStr(out_sha_bytes); + + // If check below fails, should manually dump the results with: + // + // SETTINGS_MERGE_TEST_OUT=results.txt ./test_bitcoin --run_test=settings_tests/Merge + // + // And verify diff against previous results to make sure the changes are expected. + // + // Results file is formatted like: + // + // || GetSetting() | GetSettingsList() | OnlyHasDefaultSectionSetting() + BOOST_CHECK_EQUAL(out_sha_hex, "79db02d74e3e193196541b67c068b40ebd0c124a24b3ecbe9cbf7e85b1c4ba7a"); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 700b07a02523..12ea07db57fc 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -72,17 +73,34 @@ std::ostream& operator<<(std::ostream& os, const uint256& num) return os; } -BasicTestingSetup::BasicTestingSetup(const std::string& chainName) +BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::vector& extra_args) : m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()} { + const std::vector arguments = Cat( + { + "dummy", + "-printtoconsole=0", + "-logtimemicros", + "-debug", + "-debugexclude=libevent", + "-debugexclude=leveldb", + }, + extra_args); fs::create_directories(m_path_root); gArgs.ForceSetArg("-datadir", m_path_root.string()); ClearDatadirCache(); + { + SetupServerArgs(m_node); + std::string error; + const bool success{m_node.args->ParseParameters(arguments.size(), arguments.data(), error)}; + assert(success); + assert(error.empty()); + } SelectParams(chainName); SeedInsecureRand(); - gArgs.ForceSetArg("-printtoconsole", "0"); if (G_TEST_LOG_FUN) LogInstance().PushBackCallback(G_TEST_LOG_FUN); InitLogging(); + AppInitParameterInteraction(); LogInstance().StartLogging(); SHA256AutoDetect(); ECC_Start(); @@ -112,10 +130,12 @@ BasicTestingSetup::~BasicTestingSetup() LogInstance().DisconnectTestLogger(); fs::remove_all(m_path_root); + gArgs.ClearArgs(); ECC_Stop(); } -TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(chainName) +TestingSetup::TestingSetup(const std::string& chainName, const std::vector& extra_args) + : BasicTestingSetup(chainName, extra_args) { const CChainParams& chainparams = Params(); // Ideally we'd move all the RPC tests to the functional testing framework @@ -128,11 +148,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha // from blocking due to queue overrun. threadGroup.create_thread([&]{ m_node.scheduler->serviceQueue(); }); GetMainSignals().RegisterBackgroundSignalScheduler(*m_node.scheduler); - m_node.mempool = &::mempool; - m_node.mempool->setSanityCheck(1.0); - m_node.banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); - m_node.connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. - m_node.peer_logic = MakeUnique(m_node.connman.get(), m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); + pblocktree.reset(new CBlockTreeDB(1 << 20, true)); m_node.chainman = &::g_chainman; @@ -140,23 +156,30 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha ::ChainstateActive().InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); assert(!::ChainstateActive().CanFlushToDisk()); - deterministicMNManager.reset(new CDeterministicMNManager(*evoDb, *m_node.connman)); - llmq::InitLLMQSystem(*evoDb, *m_node.mempool, *m_node.connman, true); ::ChainstateActive().InitCoinsCache(1 << 23); assert(::ChainstateActive().CanFlushToDisk()); if (!LoadGenesisBlock(chainparams)) { throw std::runtime_error("LoadGenesisBlock failed."); } - { - CValidationState state; - if (!ActivateBestChain(state, chainparams)) { - throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", FormatStateMessage(state))); - } + + CValidationState state; + if (!ActivateBestChain(state, chainparams)) { + throw std::runtime_error(strprintf("ActivateBestChain failed. (%s)", FormatStateMessage(state))); } + // Start script-checking threads. Set g_parallel_script_checks to true so they are used. constexpr int script_check_threads = 2; StartScriptCheckWorkerThreads(script_check_threads); g_parallel_script_checks = true; + + m_node.mempool = &::mempool; + m_node.mempool->setSanityCheck(1.0); + m_node.banman = MakeUnique(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME); + m_node.connman = MakeUnique(0x1337, 0x1337); // Deterministic randomness for tests. + m_node.peer_logic = MakeUnique(m_node.connman.get(), m_node.banman.get(), *m_node.scheduler, *m_node.chainman, *m_node.mempool, false); + + deterministicMNManager.reset(new CDeterministicMNManager(*evoDb, *m_node.connman)); + llmq::InitLLMQSystem(*evoDb, *m_node.mempool, *m_node.connman, true); } TestingSetup::~TestingSetup() @@ -174,6 +197,7 @@ TestingSetup::~TestingSetup() m_node.banman.reset(); UnloadBlockIndex(m_node.mempool); m_node.mempool = nullptr; + m_node.args = nullptr; m_node.scheduler.reset(); llmq::DestroyLLMQSystem(); m_node.chainman->Reset(); diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index a711cba012f7..bbc455496d34 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -74,9 +74,11 @@ static constexpr CAmount CENT{1000000}; */ struct BasicTestingSetup { ECCVerifyHandle globalVerifyHandle; + NodeContext m_node; - explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); + explicit BasicTestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector& extra_args = {}); ~BasicTestingSetup(); + private: std::unique_ptr connman; const fs::path m_path_root; @@ -86,10 +88,9 @@ struct BasicTestingSetup { * Included are coins database, script check threads setup. */ struct TestingSetup : public BasicTestingSetup { - NodeContext m_node; boost::thread_group threadGroup; - explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN); + explicit TestingSetup(const std::string& chainName = CBaseChainParams::MAIN, const std::vector& extra_args = {}); ~TestingSetup(); }; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 97085e912a44..beb553cd90ec 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -17,8 +17,9 @@ #include #include -#include #include +#include +#include #include #ifndef WIN32 #include @@ -153,14 +154,12 @@ BOOST_AUTO_TEST_CASE(util_FormatISO8601Time) struct TestArgsManager : public ArgsManager { TestArgsManager() { m_network_only_args.clear(); } - std::map >& GetOverrideArgs() { return m_override_args; } - std::map >& GetConfigArgs() { return m_config_args; } void ReadConfigString(const std::string str_config) { std::istringstream streamConfig(str_config); { LOCK(cs_args); - m_config_args.clear(); + m_settings.ro_config.clear(); m_config_sections.clear(); } std::string error; @@ -180,6 +179,7 @@ struct TestArgsManager : public ArgsManager using ArgsManager::ReadConfigStream; using ArgsManager::cs_args; using ArgsManager::m_network; + using ArgsManager::m_settings; }; BOOST_AUTO_TEST_CASE(util_ParseParameters) @@ -193,28 +193,29 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters) const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"}; std::string error; + LOCK(testArgs.cs_args); testArgs.SetupArgs({a, b, ccc, d}); - testArgs.ParseParameters(0, (char**)argv_test, error); - BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.ParseParameters(0, (char**)argv_test, error)); + BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && testArgs.m_settings.ro_config.empty()); - testArgs.ParseParameters(1, (char**)argv_test, error); - BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error)); + BOOST_CHECK(testArgs.m_settings.command_line_options.empty() && testArgs.m_settings.ro_config.empty()); - testArgs.ParseParameters(7, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error)); // expectation: -ignored is ignored (program name argument), // -a, -b and -ccc end up in map, -d ignored because it is after // a non-option argument (non-GNU option parsing) - BOOST_CHECK(testArgs.GetOverrideArgs().size() == 3 && testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 3 && testArgs.m_settings.ro_config.empty()); BOOST_CHECK(testArgs.IsArgSet("-a") && testArgs.IsArgSet("-b") && testArgs.IsArgSet("-ccc") && !testArgs.IsArgSet("f") && !testArgs.IsArgSet("-d")); - BOOST_CHECK(testArgs.GetOverrideArgs().count("-a") && testArgs.GetOverrideArgs().count("-b") && testArgs.GetOverrideArgs().count("-ccc") - && !testArgs.GetOverrideArgs().count("f") && !testArgs.GetOverrideArgs().count("-d")); - - BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].size() == 1); - BOOST_CHECK(testArgs.GetOverrideArgs()["-a"].front() == ""); - BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].size() == 2); - BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].front() == "argument"); - BOOST_CHECK(testArgs.GetOverrideArgs()["-ccc"].back() == "multiple"); + BOOST_CHECK(testArgs.m_settings.command_line_options.count("a") && testArgs.m_settings.command_line_options.count("b") && testArgs.m_settings.command_line_options.count("ccc") + && !testArgs.m_settings.command_line_options.count("f") && !testArgs.m_settings.command_line_options.count("d")); + + BOOST_CHECK(testArgs.m_settings.command_line_options["a"].size() == 1); + BOOST_CHECK(testArgs.m_settings.command_line_options["a"].front().get_str() == ""); + BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].size() == 2); + BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].front().get_str() == "argument"); + BOOST_CHECK(testArgs.m_settings.command_line_options["ccc"].back().get_str() == "multiple"); BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2); } @@ -285,16 +286,17 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg) const char *argv_test[] = { "ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"}; std::string error; + LOCK(testArgs.cs_args); testArgs.SetupArgs({a, b, c, d, e, f}); - testArgs.ParseParameters(7, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(7, (char**)argv_test, error)); // Each letter should be set. for (const char opt : "abcdef") BOOST_CHECK(testArgs.IsArgSet({'-', opt}) || !opt); // Nothing else should be in the map - BOOST_CHECK(testArgs.GetOverrideArgs().size() == 6 && - testArgs.GetConfigArgs().empty()); + BOOST_CHECK(testArgs.m_settings.command_line_options.size() == 6 && + testArgs.m_settings.ro_config.empty()); // The -no prefix should get stripped on the way in. BOOST_CHECK(!testArgs.IsArgSet("-nob")); @@ -323,7 +325,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"}; testArgs.SetupArgs({foo, bar}); std::string error; - testArgs.ParseParameters(4, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(4, (char**)argv_test, error)); // This was passed twice, second one overrides the negative setting. BOOST_CHECK(!testArgs.IsArgNegated("-foo")); @@ -335,7 +337,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) // Config test const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n"; - testArgs.ParseParameters(1, (char**)argv_test, error); + BOOST_CHECK(testArgs.ParseParameters(1, (char**)argv_test, error)); testArgs.ReadConfigString(conf_test); // This was passed twice, second one overrides the negative setting, @@ -350,7 +352,7 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArgEdgeCases) // Combined test const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"}; const char *combo_test_conf = "foo=1\nnobar=1\n"; - testArgs.ParseParameters(3, (char**)combo_test_args, error); + BOOST_CHECK(testArgs.ParseParameters(3, (char**)combo_test_args, error)); testArgs.ReadConfigString(combo_test_conf); // Command line overrides, but doesn't erase old setting @@ -390,6 +392,7 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) "iii=2\n"; TestArgsManager test_args; + LOCK(test_args.cs_args); const auto a = std::make_pair("-a", ArgsManager::ALLOW_BOOL); const auto b = std::make_pair("-b", ArgsManager::ALLOW_BOOL); const auto ccc = std::make_pair("-ccc", ArgsManager::ALLOW_STRING); @@ -406,22 +409,25 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) // expectation: a, b, ccc, d, fff, ggg, h, i end up in map // so do sec1.ccc, sec1.d, sec1.h, sec2.ccc, sec2.iii - BOOST_CHECK(test_args.GetOverrideArgs().empty()); - BOOST_CHECK(test_args.GetConfigArgs().size() == 13); - - BOOST_CHECK(test_args.GetConfigArgs().count("-a") - && test_args.GetConfigArgs().count("-b") - && test_args.GetConfigArgs().count("-ccc") - && test_args.GetConfigArgs().count("-d") - && test_args.GetConfigArgs().count("-fff") - && test_args.GetConfigArgs().count("-ggg") - && test_args.GetConfigArgs().count("-h") - && test_args.GetConfigArgs().count("-i") + BOOST_CHECK(test_args.m_settings.command_line_options.empty()); + BOOST_CHECK(test_args.m_settings.ro_config.size() == 3); + BOOST_CHECK(test_args.m_settings.ro_config[""].size() == 8); + BOOST_CHECK(test_args.m_settings.ro_config["sec1"].size() == 3); + BOOST_CHECK(test_args.m_settings.ro_config["sec2"].size() == 2); + + BOOST_CHECK(test_args.m_settings.ro_config[""].count("a") + && test_args.m_settings.ro_config[""].count("b") + && test_args.m_settings.ro_config[""].count("ccc") + && test_args.m_settings.ro_config[""].count("d") + && test_args.m_settings.ro_config[""].count("fff") + && test_args.m_settings.ro_config[""].count("ggg") + && test_args.m_settings.ro_config[""].count("h") + && test_args.m_settings.ro_config[""].count("i") ); - BOOST_CHECK(test_args.GetConfigArgs().count("-sec1.ccc") - && test_args.GetConfigArgs().count("-sec1.h") - && test_args.GetConfigArgs().count("-sec2.ccc") - && test_args.GetConfigArgs().count("-sec2.iii") + BOOST_CHECK(test_args.m_settings.ro_config["sec1"].count("ccc") + && test_args.m_settings.ro_config["sec1"].count("h") + && test_args.m_settings.ro_config["sec2"].count("ccc") + && test_args.m_settings.ro_config["sec2"].count("iii") ); BOOST_CHECK(test_args.IsArgSet("-a") @@ -560,24 +566,25 @@ BOOST_AUTO_TEST_CASE(util_ReadConfigStream) BOOST_AUTO_TEST_CASE(util_GetArg) { TestArgsManager testArgs; - testArgs.GetOverrideArgs().clear(); - testArgs.GetOverrideArgs()["strtest1"] = {"string..."}; + LOCK(testArgs.cs_args); + testArgs.m_settings.command_line_options.clear(); + testArgs.m_settings.command_line_options["strtest1"] = {"string..."}; // strtest2 undefined on purpose - testArgs.GetOverrideArgs()["inttest1"] = {"12345"}; - testArgs.GetOverrideArgs()["inttest2"] = {"81985529216486895"}; + testArgs.m_settings.command_line_options["inttest1"] = {"12345"}; + testArgs.m_settings.command_line_options["inttest2"] = {"81985529216486895"}; // inttest3 undefined on purpose - testArgs.GetOverrideArgs()["booltest1"] = {""}; + testArgs.m_settings.command_line_options["booltest1"] = {""}; // booltest2 undefined on purpose - testArgs.GetOverrideArgs()["booltest3"] = {"0"}; - testArgs.GetOverrideArgs()["booltest4"] = {"1"}; + testArgs.m_settings.command_line_options["booltest3"] = {"0"}; + testArgs.m_settings.command_line_options["booltest4"] = {"1"}; // priorities - testArgs.GetOverrideArgs()["pritest1"] = {"a", "b"}; - testArgs.GetConfigArgs()["pritest2"] = {"a", "b"}; - testArgs.GetOverrideArgs()["pritest3"] = {"a"}; - testArgs.GetConfigArgs()["pritest3"] = {"b"}; - testArgs.GetOverrideArgs()["pritest4"] = {"a","b"}; - testArgs.GetConfigArgs()["pritest4"] = {"c","d"}; + testArgs.m_settings.command_line_options["pritest1"] = {"a", "b"}; + testArgs.m_settings.ro_config[""]["pritest2"] = {"a", "b"}; + testArgs.m_settings.command_line_options["pritest3"] = {"a"}; + testArgs.m_settings.ro_config[""]["pritest3"] = {"b"}; + testArgs.m_settings.command_line_options["pritest4"] = {"a","b"}; + testArgs.m_settings.ro_config[""]["pritest4"] = {"c","d"}; BOOST_CHECK_EQUAL(testArgs.GetArg("strtest1", "default"), "string..."); BOOST_CHECK_EQUAL(testArgs.GetArg("strtest2", "default"), "default"); @@ -612,38 +619,38 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) const char* testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1"; std::string error; - test_args.ParseParameters(0, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "main"); - test_args.ParseParameters(2, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_regtest, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest"); - test_args.ParseParameters(3, (char**)argv_test_no_reg, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_test_no_reg, error)); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char**)argv_both, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error)); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(0, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_regtest, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(3, (char**)argv_test_no_reg, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_test_no_reg, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char**)argv_both, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); @@ -651,23 +658,23 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) // [test] regtest=1 potentially relevant) doesn't break things test_args.SelectConfigNetwork("test"); - test_args.ParseParameters(0, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(0, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_testnet, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_testnet, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char**)argv_regtest, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_regtest, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(2, (char**)argv_test_no_reg, error); + BOOST_CHECK(test_args.ParseParameters(2, (char**)argv_test_no_reg, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char**)argv_both, error); + BOOST_CHECK(test_args.ParseParameters(3, (char**)argv_both, error)); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); } diff --git a/src/util/system.cpp b/src/util/system.cpp index e43cad82455b..fc2a1bb58932 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -69,6 +69,7 @@ #endif #include +#include // Application startup time (used for uptime calculation) const int64_t nStartupTime = GetTime(); @@ -182,11 +183,14 @@ static bool InterpretBool(const std::string& strValue) return (atoi(strValue) != 0); } +static std::string SettingName(const std::string& arg) +{ + return arg.size() > 0 && arg[0] == '-' ? arg.substr(1) : arg; +} + /** Internal helper functions for ArgsManager */ class ArgsManagerHelper { public: - typedef std::map> MapArgs; - /** Determine whether to use config settings in the default section, * See also comments around ArgsManager::ArgsManager() below. */ static inline bool UseDefaultSection(const ArgsManager& am, const std::string& arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) @@ -194,91 +198,10 @@ class ArgsManagerHelper { return (am.m_network == CBaseChainParams::MAIN || am.m_network_only_args.count(arg) == 0); } - /** Convert regular argument into the network-specific setting */ - static inline std::string NetworkArg(const ArgsManager& am, const std::string& arg) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) - { - assert(arg.length() > 1 && arg[0] == '-'); - return "-" + am.m_network + "." + arg.substr(1); - } - - /** Find arguments in a map and add them to a vector */ - static inline void AddArgs(std::vector& res, const MapArgs& map_args, const std::string& arg) - { - auto it = map_args.find(arg); - if (it != map_args.end()) { - res.insert(res.end(), it->second.begin(), it->second.end()); - } - } - - /** Return true/false if an argument is set in a map, and also - * return the first (or last) of the possibly multiple values it has - */ - static inline std::pair GetArgHelper(const MapArgs& map_args, const std::string& arg, bool getLast = false) - { - auto it = map_args.find(arg); - - if (it == map_args.end() || it->second.empty()) { - return std::make_pair(false, std::string()); - } - - if (getLast) { - return std::make_pair(true, it->second.back()); - } else { - return std::make_pair(true, it->second.front()); - } - } - - /* Get the string value of an argument, returning a pair of a boolean - * indicating the argument was found, and the value for the argument - * if it was found (or the empty string if not found). - */ - static inline std::pair GetArg(const ArgsManager &am, const std::string& arg) + static util::SettingsValue Get(const ArgsManager& am, const std::string& arg) { LOCK(am.cs_args); - std::pair found_result(false, std::string()); - - // We pass "true" to GetArgHelper in order to return the last - // argument value seen from the command line (so "dashd -foo=bar - // -foo=baz" gives GetArg(am,"foo")=={true,"baz"} - found_result = GetArgHelper(am.m_override_args, arg, true); - if (found_result.first) { - return found_result; - } - - // But in contrast we return the first argument seen in a config file, - // so "foo=bar \n foo=baz" in the config file gives - // GetArg(am,"foo")={true,"bar"} - if (!am.m_network.empty()) { - found_result = GetArgHelper(am.m_config_args, NetworkArg(am, arg)); - if (found_result.first) { - return found_result; - } - } - - if (UseDefaultSection(am, arg)) { - found_result = GetArgHelper(am.m_config_args, arg); - if (found_result.first) { - return found_result; - } - } - - return found_result; - } - - /* Special test for -testnet and -regtest args, because we - * don't want to be confused by craziness like "[regtest] testnet=1" - */ - static inline bool GetNetBoolArg(const ArgsManager &am, const std::string& net_arg, bool interpret_bool) EXCLUSIVE_LOCKS_REQUIRED(am.cs_args) - { - std::pair found_result(false,std::string()); - found_result = GetArgHelper(am.m_override_args, net_arg, true); - if (!found_result.first) { - found_result = GetArgHelper(am.m_config_args, net_arg, true); - if (!found_result.first) { - return false; // not set - } - } - return !interpret_bool || InterpretBool(found_result.second); // is set, so evaluate + return GetSetting(am.m_settings, am.m_network, SettingName(arg), !UseDefaultSection(am, arg), /* get_chain_name= */ false); } }; @@ -289,13 +212,12 @@ class ArgsManagerHelper { * checks whether there was a double-negative (-nofoo=0 -> -foo=1). * * If there was not a double negative, it removes the "no" from the key - * and clears the args vector to indicate a negated option. + * and returns false. * - * If there was a double negative, it removes "no" from the key, sets the - * value to "1" and pushes the key and the updated value to the args vector. + * If there was a double negative, it removes "no" from the key, and + * returns true. * - * If there was no "no", it leaves key and value untouched and pushes them - * to the args vector. + * If there was no "no", it returns the string value untouched. * * Where an option was negated can be later checked using the * IsArgNegated() method. One use case for this is to have a way to disable @@ -303,34 +225,39 @@ class ArgsManagerHelper { * that debug log output is not sent to any file at all). */ -[[nodiscard]] static bool InterpretOption(std::string key, std::string val, unsigned int flags, - std::map>& args, - std::string& error) +static util::SettingsValue InterpretOption(std::string& section, std::string& key, const std::string& value) { - assert(key[0] == '-'); - + // Split section name from key name for keys like "testnet.foo" or "regtest.bar" size_t option_index = key.find('.'); - if (option_index == std::string::npos) { - option_index = 1; - } else { - ++option_index; + if (option_index != std::string::npos) { + section = key.substr(0, option_index); + key.erase(0, option_index + 1); } - if (key.substr(option_index, 2) == "no") { - key.erase(option_index, 2); - if (flags & ArgsManager::ALLOW_BOOL) { - if (InterpretBool(val)) { - args[key].clear(); - return true; - } - // Double negatives like -nofoo=0 are supported (but discouraged) - LogPrintf("Warning: parsed potentially confusing double-negative %s=%s\n", key, val); - val = "1"; - } else { - error = strprintf("Negating of %s is meaningless and therefore forbidden", key); - return false; + if (key.substr(0, 2) == "no") { + key.erase(0, 2); + // Double negatives like -nofoo=0 are supported (but discouraged) + if (!InterpretBool(value)) { + LogPrintf("Warning: parsed potentially confusing double-negative -%s=%s\n", key, value); + return true; } + return false; + } + return value; +} + +/** + * Check settings value validity according to flags. + * + * TODO: Add more meaningful error checks here in the future + * See "here's how the flags are meant to behave" in + * https://github.com/bitcoin/bitcoin/pull/16097#issuecomment-514627823 + */ +static bool CheckValid(const std::string& key, const util::SettingsValue& val, unsigned int flags, std::string& error) +{ + if (val.isBool() && !(flags & ArgsManager::ALLOW_BOOL)) { + error = strprintf("Negating of -%s is meaningless and therefore forbidden", key); + return false; } - args[key].push_back(val); return true; } @@ -352,22 +279,9 @@ const std::set ArgsManager::GetUnsuitableSectionOnlyArgs() const if (m_network == CBaseChainParams::MAIN) return std::set {}; for (const auto& arg : m_network_only_args) { - std::pair found_result; - - // if this option is overridden it's fine - found_result = ArgsManagerHelper::GetArgHelper(m_override_args, arg); - if (found_result.first) continue; - - // if there's a network-specific value for this option, it's fine - found_result = ArgsManagerHelper::GetArgHelper(m_config_args, ArgsManagerHelper::NetworkArg(*this, arg)); - if (found_result.first) continue; - - // if there isn't a default value for this option, it's fine - found_result = ArgsManagerHelper::GetArgHelper(m_config_args, arg); - if (!found_result.first) continue; - - // otherwise, issue a warning - unsuitables.insert(arg); + if (OnlyHasDefaultSectionSetting(m_settings, m_network, SettingName(arg))) { + unsuitables.insert(arg); + } } return unsuitables; } @@ -388,6 +302,11 @@ const std::list ArgsManager::GetUnrecognizedSections() const return unrecognized; } +const std::map> ArgsManager::GetCommandLineArgs() const { + LOCK(cs_args); + return m_settings.command_line_options; +} + void ArgsManager::SelectConfigNetwork(const std::string& network) { LOCK(cs_args); @@ -397,8 +316,7 @@ void ArgsManager::SelectConfigNetwork(const std::string& network) bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::string& error) { LOCK(cs_args); - m_override_args.clear(); - m_command_line_args.clear(); + m_settings.command_line_options.clear(); for (int i = 1; i < argc; i++) { std::string key(argv[i]); @@ -431,56 +349,44 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin if (key.length() > 1 && key[1] == '-') key.erase(0, 1); + // Transform -foo to foo + key.erase(0, 1); + std::string section; + util::SettingsValue value = InterpretOption(section, key, val); const unsigned int flags = FlagsOfKnownArg(key); if (flags) { - if (!InterpretOption(key, val, flags, m_override_args, error)) { + if (!CheckValid(key, value, flags, error)) { return false; } - m_command_line_args[key].push_back(val); + // Weird behavior preserved for backwards compatibility: command + // line options with section prefixes are allowed but ignored. It + // would be better if these options triggered the Invalid parameter + // error below. + if (section.empty()) { + m_settings.command_line_options[key].push_back(value); + } } else { - error = strprintf("Invalid parameter %s", key); + error = strprintf("Invalid parameter -%s", key); return false; } } // we do not allow -includeconf from command line, so we clear it here - auto it = m_override_args.find("-includeconf"); - if (it != m_override_args.end()) { - if (it->second.size() > 0) { - for (const auto& ic : it->second) { - error += "-includeconf cannot be used from commandline; -includeconf=" + ic + "\n"; - } - return false; + bool success = true; + if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { + for (const auto& include : util::SettingsSpan(*includes)) { + error += "-includeconf cannot be used from commandline; -includeconf=" + include.get_str() + "\n"; + success = false; } } - return true; -} - -const std::map> ArgsManager::GetCommandLineArgs() const -{ - LOCK(cs_args); - return m_command_line_args; + return success; } unsigned int ArgsManager::FlagsOfKnownArg(const std::string& key) const { - assert(key[0] == '-'); - - size_t option_index = key.find('.'); - if (option_index == std::string::npos) { - option_index = 1; - } else { - ++option_index; - } - if (key.substr(option_index, 2) == "no") { - option_index += 2; - } - - const std::string base_arg_name = '-' + key.substr(option_index); - LOCK(cs_args); for (const auto& arg_map : m_available_args) { - const auto search = arg_map.second.find(base_arg_name); + const auto search = arg_map.second.find('-' + key); if (search != arg_map.second.end()) { return search->second.m_flags; } @@ -490,69 +396,42 @@ unsigned int ArgsManager::FlagsOfKnownArg(const std::string& key) const std::vector ArgsManager::GetArgs(const std::string& strArg) const { - std::vector result = {}; - if (IsArgNegated(strArg)) return result; // special case - LOCK(cs_args); - - ArgsManagerHelper::AddArgs(result, m_override_args, strArg); - if (!m_network.empty()) { - ArgsManagerHelper::AddArgs(result, m_config_args, ArgsManagerHelper::NetworkArg(*this, strArg)); + bool ignore_default_section_config = !ArgsManagerHelper::UseDefaultSection(*this, strArg); + std::vector result; + for (const util::SettingsValue& value : + util::GetSettingsList(m_settings, m_network, SettingName(strArg), ignore_default_section_config)) { + result.push_back(value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str()); } - - if (ArgsManagerHelper::UseDefaultSection(*this, strArg)) { - ArgsManagerHelper::AddArgs(result, m_config_args, strArg); - } - return result; } bool ArgsManager::IsArgSet(const std::string& strArg) const { - if (IsArgNegated(strArg)) return true; // special case - return ArgsManagerHelper::GetArg(*this, strArg).first; + return !ArgsManagerHelper::Get(*this, strArg).isNull(); } bool ArgsManager::IsArgNegated(const std::string& strArg) const { - LOCK(cs_args); - - const auto& ov = m_override_args.find(strArg); - if (ov != m_override_args.end()) return ov->second.empty(); - - if (!m_network.empty()) { - const auto& cfs = m_config_args.find(ArgsManagerHelper::NetworkArg(*this, strArg)); - if (cfs != m_config_args.end()) return cfs->second.empty(); - } - - const auto& cf = m_config_args.find(strArg); - if (cf != m_config_args.end()) return cf->second.empty(); - - return false; + return ArgsManagerHelper::Get(*this, strArg).isFalse(); } std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const { - if (IsArgNegated(strArg)) return "0"; - std::pair found_res = ArgsManagerHelper::GetArg(*this, strArg); - if (found_res.first) return found_res.second; - return strDefault; + const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + return value.isNull() ? strDefault : value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str(); } int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const { - if (IsArgNegated(strArg)) return 0; - std::pair found_res = ArgsManagerHelper::GetArg(*this, strArg); - if (found_res.first) return atoi64(found_res.second); - return nDefault; + const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + return value.isNull() ? nDefault : value.isFalse() ? 0 : value.isTrue() ? 1 : value.isNum() ? value.get_int64() : atoi64(value.get_str()); } bool ArgsManager::GetBoolArg(const std::string& strArg, bool fDefault) const { - if (IsArgNegated(strArg)) return false; - std::pair found_res = ArgsManagerHelper::GetArg(*this, strArg); - if (found_res.first) return InterpretBool(found_res.second); - return fDefault; + const util::SettingsValue value = ArgsManagerHelper::Get(*this, strArg); + return value.isNull() ? fDefault : value.isBool() ? value.get_bool() : InterpretBool(value.get_str()); } bool ArgsManager::SoftSetArg(const std::string& strArg, const std::string& strValue) @@ -574,7 +453,7 @@ bool ArgsManager::SoftSetBoolArg(const std::string& strArg, bool fValue) void ArgsManager::ForceSetArg(const std::string& strArg, const std::string& strValue) { LOCK(cs_args); - m_override_args[strArg] = {strValue}; + m_settings.forced_settings[SettingName(strArg)] = strValue; } void ArgsManager::AddArg(const std::string& name, const std::string& help, unsigned int flags, const OptionsCategory& cat) @@ -693,22 +572,18 @@ std::string ArgsManager::GetHelpMessage() const void ArgsManager::ForceRemoveArg(const std::string& strArg) { LOCK(cs_args); - const auto& ov = m_override_args.find(strArg); - if (ov != m_override_args.end()) { - m_override_args.erase(ov); + const auto& ov = m_settings.forced_settings.find(strArg); + if (ov != m_settings.forced_settings.end()) { + m_settings.forced_settings.erase(ov); } - if (!m_network.empty()) { - const auto& cfs = m_config_args.find(ArgsManagerHelper::NetworkArg(*this, strArg)); - if (cfs != m_config_args.end()) { - m_config_args.erase(cfs); + for (const auto& network : { CBaseChainParams::MAIN, CBaseChainParams::TESTNET, CBaseChainParams::REGTEST, CBaseChainParams::DEVNET }) { + if (auto* section = util::FindKey(m_settings.ro_config, network)) { + if (util::FindKey(*section, strArg)) { + section->erase(strArg); + } } } - - const auto& cf = m_config_args.find(strArg); - if (cf != m_config_args.end()) { - m_config_args.erase(cf); - } } bool HelpRequested(const ArgsManager& args) @@ -803,8 +678,9 @@ const fs::path &GetDataDir(bool fNetSpecific) // this function if (!path.empty()) return path; - if (gArgs.IsArgSet("-datadir")) { - path = fs::system_complete(gArgs.GetArg("-datadir", "")); + std::string datadir = gArgs.GetArg("-datadir", ""); + if (!datadir.empty()) { + path = fs::system_complete(datadir); if (!fs::is_directory(path)) { path = ""; return path; @@ -831,6 +707,12 @@ fs::path GetBackupsDir() return fs::absolute(gArgs.GetArg("-walletbackupsdir", "")); } +bool CheckDataDirOption() +{ + std::string datadir = gArgs.GetArg("-datadir", ""); + return datadir.empty() || fs::is_directory(fs::system_complete(datadir)); +} + void ClearDatadirCache() { LOCK(csPathCached); @@ -898,12 +780,15 @@ bool ArgsManager::ReadConfigStream(std::istream& stream, const std::string& file return false; } for (const std::pair& option : options) { - const std::string strKey = std::string("-") + option.first; - const unsigned int flags = FlagsOfKnownArg(strKey); + std::string section; + std::string key = option.first; + util::SettingsValue value = InterpretOption(section, key, option.second); + const unsigned int flags = FlagsOfKnownArg(key); if (flags) { - if (!InterpretOption(strKey, option.second, flags, m_config_args, error)) { + if (!CheckValid(key, value, flags, error)) { return false; } + m_settings.ro_config[section][key].push_back(value); } else { if (ignore_invalid_keys) { LogPrintf("Ignoring unknown configuration value %s\n", option.first); @@ -920,7 +805,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) { { LOCK(cs_args); - m_config_args.clear(); + m_settings.ro_config.clear(); m_config_sections.clear(); } @@ -936,30 +821,34 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) bool use_conf_file{true}; { LOCK(cs_args); - auto it = m_override_args.find("-includeconf"); - if (it != m_override_args.end()) { + if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { // ParseParameters() fails if a non-negated -includeconf is passed on the command-line - assert(it->second.empty()); + assert(util::SettingsSpan(*includes).last_negated()); use_conf_file = false; } } if (use_conf_file) { std::string chain_id = GetChainName(); - std::vector conf_file_names(GetArgs("-includeconf")); - { - // We haven't set m_network yet (that happens in SelectParams()), so manually check - // for network.includeconf args. - std::vector includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf")); - conf_file_names.insert(conf_file_names.end(), includeconf_net.begin(), includeconf_net.end()); - } + std::vector conf_file_names; - // Remove -includeconf from configuration, so we can warn about recursion - // later - { + auto add_includes = [&](const std::string& network, size_t skip = 0) { + size_t num_values = 0; LOCK(cs_args); - m_config_args.erase("-includeconf"); - m_config_args.erase(std::string("-") + chain_id + ".includeconf"); - } + if (auto* section = util::FindKey(m_settings.ro_config, network)) { + if (auto* values = util::FindKey(*section, "includeconf")) { + for (size_t i = std::max(skip, util::SettingsSpan(*values).negated()); i < values->size(); ++i) { + conf_file_names.push_back((*values)[i].get_str()); + } + num_values = values->size(); + } + } + return num_values; + }; + + // We haven't set m_network yet (that happens in SelectParams()), so manually check + // for network.includeconf args. + const size_t chain_includes = add_includes(chain_id); + const size_t default_includes = add_includes({}); for (const std::string& conf_file_name : conf_file_names) { fsbridge::ifstream conf_file_stream(GetConfigFile(conf_file_name)); @@ -975,14 +864,13 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) } // Warn about recursive -includeconf - conf_file_names = GetArgs("-includeconf"); - std::vector includeconf_net(GetArgs(std::string("-") + chain_id + ".includeconf")); - conf_file_names.insert(conf_file_names.end(), includeconf_net.begin(), includeconf_net.end()); + conf_file_names.clear(); + add_includes(chain_id, /* skip= */ chain_includes); + add_includes({}, /* skip= */ default_includes); std::string chain_id_final = GetChainName(); if (chain_id_final != chain_id) { // Also warn about recursive includeconf for the chain that was specified in one of the includeconfs - includeconf_net = GetArgs(std::string("-") + chain_id_final + ".includeconf"); - conf_file_names.insert(conf_file_names.end(), includeconf_net.begin(), includeconf_net.end()); + add_includes(chain_id_final); } for (const std::string& conf_file_name : conf_file_names) { tfm::format(std::cerr, "warning: -includeconf cannot be used from included files; ignoring -includeconf=%s\n", conf_file_name); @@ -998,7 +886,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) // If datadir is changed in .conf file: ClearDatadirCache(); - if (!fs::is_directory(GetDataDir(false))) { + if (!CheckDataDirOption()) { error = strprintf("specified data directory \"%s\" does not exist.", gArgs.GetArg("-datadir", "")); return false; } @@ -1007,10 +895,17 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) std::string ArgsManager::GetChainName() const { - LOCK(cs_args); - bool fRegTest = ArgsManagerHelper::GetNetBoolArg(*this, "-regtest", true); - bool fDevNet = ArgsManagerHelper::GetNetBoolArg(*this, "-devnet", false); - bool fTestNet = ArgsManagerHelper::GetNetBoolArg(*this, "-testnet", true); + auto get_net = [&](const std::string& arg, bool interpret_bool = true) { + LOCK(cs_args); + util::SettingsValue value = GetSetting(m_settings, /* section= */ "", SettingName(arg), + /* ignore_default_section_config= */ false, + /* get_chain_name= */ true); + return value.isNull() ? false : value.isBool() ? value.get_bool() : (!interpret_bool || InterpretBool(value.get_str())); + }; + + const bool fRegTest = get_net("-regtest"); + const bool fTestNet = get_net("-testnet"); + const bool fDevNet = get_net("-devnet", false); int nameParamsCount = (fRegTest ? 1 : 0) + (fDevNet ? 1 : 0) + (fTestNet ? 1 : 0); if (nameParamsCount > 1) @@ -1322,6 +1217,9 @@ int64_t GetStartupTime() fs::path AbsPathForConfigVal(const fs::path& path, bool net_specific) { + if (path.is_absolute()) { + return path; + } return fs::absolute(path, GetDataDir(net_specific)); } diff --git a/src/util/system.h b/src/util/system.h index 6892727d310f..b7143fae6089 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -92,6 +93,8 @@ fs::path GetDefaultDataDir(); const fs::path &GetBlocksDir(); const fs::path &GetDataDir(bool fNetSpecific = true); fs::path GetBackupsDir(); +// Return true if -datadir option points to a valid directory or is not specified. +bool CheckDataDirOption(); /** Tests only */ void ClearDatadirCache(); fs::path GetConfigFile(const std::string& confPath); @@ -180,9 +183,7 @@ class ArgsManager }; mutable CCriticalSection cs_args; - std::map> m_override_args GUARDED_BY(cs_args); - std::map> m_command_line_args GUARDED_BY(cs_args); - std::map> m_config_args GUARDED_BY(cs_args); + util::Settings m_settings GUARDED_BY(cs_args); std::string m_network GUARDED_BY(cs_args); std::set m_network_only_args GUARDED_BY(cs_args); std::map> m_available_args GUARDED_BY(cs_args); @@ -215,9 +216,9 @@ class ArgsManager const std::list GetUnrecognizedSections() const; /** - * Return the map of all the args passed via cmd line + * Return the map of all the args passed via the command line */ - const std::map> GetCommandLineArgs() const; + const std::map> GetCommandLineArgs() const; /** * Return a vector of strings of the given argument diff --git a/src/validation.cpp b/src/validation.cpp index ff0af55c0edd..5c157b79e462 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1187,18 +1187,6 @@ bool CChainState::IsInitialBlockDownload() const static CBlockIndex *pindexBestForkTip = nullptr, *pindexBestForkBase = nullptr; -BlockMap& BlockIndex() -{ - LOCK(::cs_main); - return g_chainman.m_blockman.m_block_index; -} - -PrevBlockMap& PrevBlockIndex() -{ - LOCK(::cs_main); - return g_chainman.m_blockman.m_prev_block_index; -} - static void AlertNotify(const std::string& strMessage) { uiInterface.NotifyAlertChanged(); @@ -3365,7 +3353,7 @@ void CChainState::EnforceBlock(CValidationState& state, const CChainParams& chai while (pindex_walk && !::ChainActive().Contains(pindex_walk)) { // Mark all blocks that have the same prevBlockHash but are not equal to blockHash as conflicting - auto itp = ::PrevBlockIndex().equal_range(pindex_walk->pprev->GetBlockHash()); + auto itp = g_chainman.PrevBlockIndex().equal_range(pindex_walk->pprev->GetBlockHash()); for (auto jt = itp.first; jt != itp.second; ++jt) { if (jt->second == pindex_walk) { continue; diff --git a/src/validation.h b/src/validation.h index 0da950c5e262..59878c23f7a5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -861,6 +861,11 @@ class ChainstateManager return m_blockman.m_block_index; } + PrevBlockMap& PrevBlockIndex() EXCLUSIVE_LOCKS_REQUIRED(::cs_main) + { + return m_blockman.m_prev_block_index; + } + bool IsSnapshotActive() const; Optional SnapshotBlockhash() const; @@ -943,12 +948,6 @@ CChainState& ChainstateActive(); /** Please prefer the identical ChainstateManager::ActiveChain */ CChain& ChainActive(); -/** Please prefer the identical ChainstateManager::BlockIndex */ -BlockMap& BlockIndex(); - -/** @returns the global previous block index map. */ -PrevBlockMap& PrevBlockIndex(); - /** Global variable that points to the active block tree (protected by cs_main) */ extern std::unique_ptr pblocktree; diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 429548857005..dc4c87d0baaa 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -107,8 +107,10 @@ CMainSignals& GetMainSignals() return g_signals; } -void RegisterValidationInterface(CValidationInterface* pwalletIn) { - ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn]; +void RegisterSharedValidationInterface(std::shared_ptr pwalletIn) { + // Each connection captures pwalletIn to ensure that each callback is + // executed before pwalletIn is destroyed. For more details see #18338. + ValidationInterfaceConnections& conns = g_signals.m_internals->m_connMainSignals[pwalletIn.get()]; conns.AcceptedBlockHeader = g_signals.m_internals->AcceptedBlockHeader.connect(std::bind(&CValidationInterface::AcceptedBlockHeader, pwalletIn, std::placeholders::_1)); conns.NotifyHeaderTip = g_signals.m_internals->NotifyHeaderTip.connect(std::bind(&CValidationInterface::NotifyHeaderTip, pwalletIn, std::placeholders::_1, std::placeholders::_2)); conns.UpdatedBlockTip = g_signals.m_internals->UpdatedBlockTip.connect(std::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3)); @@ -129,6 +131,18 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { conns.NotifyMasternodeListChanged = g_signals.m_internals->NotifyMasternodeListChanged.connect(std::bind(&CValidationInterface::NotifyMasternodeListChanged, pwalletIn, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, std::placeholders::_4)); } +void RegisterValidationInterface(CValidationInterface* callbacks) +{ + // Create a shared_ptr with a no-op deleter - CValidationInterface lifecycle + // is managed by the caller. + RegisterSharedValidationInterface({callbacks, [](CValidationInterface*){}}); +} + +void UnregisterSharedValidationInterface(std::shared_ptr callbacks) +{ + UnregisterValidationInterface(callbacks.get()); +} + void UnregisterValidationInterface(CValidationInterface* pwalletIn) { if (g_signals.m_internals) { g_signals.m_internals->m_connMainSignals.erase(pwalletIn); diff --git a/src/validationinterface.h b/src/validationinterface.h index 9f2ca0ec00df..bed43795c1f1 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -43,6 +43,14 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn); void UnregisterValidationInterface(CValidationInterface* pwalletIn); /** Unregister all wallets from core */ void UnregisterAllValidationInterfaces(); + +// Alternate registration functions that release a shared_ptr after the last +// notification is sent. These are useful for race-free cleanup, since +// unregistration is nonblocking and can return before the last notification is +// processed. +void RegisterSharedValidationInterface(std::shared_ptr callbacks); +void UnregisterSharedValidationInterface(std::shared_ptr callbacks); + /** * Pushes a function to callback onto the notification queue, guaranteeing any * callbacks generated prior to now are finished when the function is called. @@ -167,7 +175,7 @@ class CValidationInterface { * Notifies listeners that a block which builds directly on our current tip * has been received and connected to the headers tree, though not validated yet */ virtual void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& block) {}; - friend void ::RegisterValidationInterface(CValidationInterface*); + friend void ::RegisterSharedValidationInterface(std::shared_ptr); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); }; @@ -177,7 +185,7 @@ class CMainSignals { private: std::unique_ptr m_internals; - friend void ::RegisterValidationInterface(CValidationInterface*); + friend void ::RegisterSharedValidationInterface(std::shared_ptr); friend void ::UnregisterValidationInterface(CValidationInterface*); friend void ::UnregisterAllValidationInterfaces(); friend void ::CallFunctionInValidationInterfaceQueue(std::function func); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index f8deedda3549..d3b8edbb5a76 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -120,7 +120,7 @@ bool WalletInit::ParameterInteraction() const if (rescan_mode < 0 || rescan_mode > 2) { LogPrintf("%s: Warning: incorrect -rescan mode, falling back to default value.\n", __func__); InitWarning(_("Incorrect -rescan mode, falling back to default value")); - gArgs.ForceRemoveArg("-rescan"); + gArgs.ForceRemoveArg("rescan"); } if (is_multiwallet) { @@ -137,14 +137,14 @@ bool WalletInit::ParameterInteraction() const if (gArgs.IsArgSet("-walletbackupsdir")) { if (!fs::is_directory(gArgs.GetArg("-walletbackupsdir", ""))) { InitWarning(strprintf(_("Warning: incorrect parameter %s, path must exist! Using default path."), "-walletbackupsdir")); - gArgs.ForceRemoveArg("-walletbackupsdir"); + gArgs.ForceRemoveArg("walletbackupsdir"); } } if (gArgs.IsArgSet("-hdseed") && IsHex(gArgs.GetArg("-hdseed", "not hex")) && (gArgs.IsArgSet("-mnemonic") || gArgs.IsArgSet("-mnemonicpassphrase"))) { InitWarning(strprintf(_("Warning: can't use %s and %s together, will prefer %s"), "-hdseed", "-mnemonic/-mnemonicpassphrase", "-hdseed")); - gArgs.ForceRemoveArg("-mnemonic"); - gArgs.ForceRemoveArg("-mnemonicpassphrase"); + gArgs.ForceRemoveArg("mnemonic"); + gArgs.ForceRemoveArg("mnemonicpassphrase"); } if (gArgs.GetArg("-coinjoindenomshardcap", DEFAULT_COINJOIN_DENOMS_HARDCAP) < gArgs.GetArg("-coinjoindenomsgoal", DEFAULT_COINJOIN_DENOMS_GOAL)) { diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h index f75a9e614931..f55688de0e6b 100644 --- a/src/wallet/test/init_test_fixture.h +++ b/src/wallet/test/init_test_fixture.h @@ -18,7 +18,6 @@ struct InitWalletDirTestingSetup: public BasicTestingSetup { fs::path m_datadir; fs::path m_cwd; std::map m_walletdir_path_cases; - NodeContext m_node; std::unique_ptr m_chain = interfaces::MakeChain(m_node); std::unique_ptr m_chain_client; }; diff --git a/src/wallet/test/wallet_test_fixture.cpp b/src/wallet/test/wallet_test_fixture.cpp index 8548a3875740..b00298df49ec 100644 --- a/src/wallet/test/wallet_test_fixture.cpp +++ b/src/wallet/test/wallet_test_fixture.cpp @@ -11,7 +11,6 @@ WalletTestingSetup::WalletTestingSetup(const std::string& chainName) { bool fFirstRun; m_wallet.LoadWallet(fFirstRun); - m_wallet.handleNotifications(); - + m_chain_notifications_handler = m_chain->handleNotifications({ &m_wallet, [](CWallet*) {} }); m_chain_client->registerRpcs(); } diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index e37ed7475735..5c10c5c1ed00 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -19,10 +19,10 @@ struct WalletTestingSetup: public TestingSetup { explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN); - NodeContext m_node; std::unique_ptr m_chain = interfaces::MakeChain(m_node); std::unique_ptr m_chain_client = interfaces::MakeWalletClient(*m_chain, {}); CWallet m_wallet; + std::unique_ptr m_chain_notifications_handler; }; #endif // BITCOIN_WALLET_TEST_WALLET_TEST_FIXTURE_H diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 74bcb5eb690f..db0c67695735 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -293,14 +293,15 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) BOOST_CHECK_EQUAL(wtx.GetImmatureCredit(), 500*COIN); } -static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) +static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) { CMutableTransaction tx; tx.nLockTime = lockTime; SetMockTime(mockTime); CBlockIndex* block = nullptr; if (blockTime > 0) { - auto inserted = ::BlockIndex().emplace(GetRandHash(), new CBlockIndex); + LOCK(cs_main); + auto inserted = chainman.BlockIndex().emplace(GetRandHash(), new CBlockIndex); assert(inserted.second); const uint256& hash = inserted.first->first; block = inserted.first->second; @@ -330,24 +331,24 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64 BOOST_AUTO_TEST_CASE(ComputeTimeSmart) { // New transaction should use clock time if lower than block time. - BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 100, 120), 100); + BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 1, 100, 120), 100); // Test that updating existing transaction does not change smart time. - BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 200, 220), 100); + BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 1, 200, 220), 100); // New transaction should use clock time if there's no block time. - BOOST_CHECK_EQUAL(AddTx(m_wallet, 2, 300, 0), 300); + BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 2, 300, 0), 300); // New transaction should use block time if lower than clock time. - BOOST_CHECK_EQUAL(AddTx(m_wallet, 3, 420, 400), 400); + BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 3, 420, 400), 400); // New transaction should use latest entry time if higher than // min(block time, clock time). - BOOST_CHECK_EQUAL(AddTx(m_wallet, 4, 500, 390), 400); + BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 4, 500, 390), 400); // If there are future entries, new transaction should use time of the // newest entry that is no more than 300 seconds ahead of the clock time. - BOOST_CHECK_EQUAL(AddTx(m_wallet, 5, 50, 600), 300); + BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 5, 50, 600), 300); // Reset mock time for other tests. SetMockTime(0); @@ -423,7 +424,6 @@ class ListCoinsTestingSetup : public TestChain100Setup return it->second; } - NodeContext m_node; std::unique_ptr m_chain = interfaces::MakeChain(m_node); std::unique_ptr wallet; }; @@ -530,7 +530,6 @@ class CreateTransactionTestSetup : public TestChain100Setup BOOST_CHECK_EQUAL(result.status, CWallet::ScanResult::SUCCESS); } - NodeContext m_node; std::unique_ptr m_chain = interfaces::MakeChain(m_node); ~CreateTransactionTestSetup() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8b4c646117d3..56217b7adbc0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -72,8 +72,10 @@ bool AddWallet(const std::shared_ptr& wallet) bool RemoveWallet(const std::shared_ptr& wallet) { - LOCK(cs_wallets); assert(wallet); + // Unregister with the validation interface which also drops shared ponters. + wallet->m_chain_notifications_handler.reset(); + LOCK(cs_wallets); std::vector>::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet); if (i == vpwallets.end()) return false; vpwallets.erase(i); @@ -113,13 +115,9 @@ static std::set g_unloading_wallet_set GUARDED_BY(g_wallet_release_ // Custom deleter for shared_ptr. static void ReleaseWallet(CWallet* wallet) { - // Unregister and delete the wallet right after BlockUntilSyncedToCurrentChain - // so that it's in sync with the current chainstate. const std::string name = wallet->GetName(); wallet->WalletLogPrintf("Releasing wallet\n"); - wallet->BlockUntilSyncedToCurrentChain(); wallet->Flush(); - wallet->m_chain_notifications_handler.reset(); delete wallet; // Wallet is now released, notify UnloadWallet, if any. { @@ -145,6 +143,7 @@ void UnloadWallet(std::shared_ptr&& wallet) // Notify the unload intent so that all remaining shared pointers are // released. wallet->NotifyUnload(); + // Time to ditch our shared_ptr and wait for ReleaseWallet call. wallet.reset(); { @@ -5081,9 +5080,9 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->SetMinVersion(FEATURE_HD); // clean up - gArgs.ForceRemoveArg("-hdseed"); - gArgs.ForceRemoveArg("-mnemonic"); - gArgs.ForceRemoveArg("-mnemonicpassphrase"); + gArgs.ForceRemoveArg("hdseed"); + gArgs.ForceRemoveArg("mnemonic"); + gArgs.ForceRemoveArg("mnemonicpassphrase"); } } // Otherwise, do not create a new HD chain @@ -5224,7 +5223,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // but we guarantee at least than wallet state is correct after notifications delivery. // This is temporary until rescan and notifications delivery are unified under same // interface. - walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(*walletInstance); + walletInstance->m_chain_notifications_handler = walletInstance->chain().handleNotifications(walletInstance); int rescan_height = 0; if (!gArgs.GetBoolArg("-rescan", false)) @@ -5310,11 +5309,6 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, return walletInstance; } -void CWallet::handleNotifications() -{ - m_chain_notifications_handler = m_chain->handleNotifications(*this); -} - void CWallet::postInitProcess() { LOCK(cs_wallet); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7e10cc9f468f..caa40145c85b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -741,7 +741,7 @@ class WalletRescanReserver; //forward declarations for ScanForWalletTransactions * A CWallet is an extension of a keystore, which also maintains a set of transactions and balances, * and provides the ability to create new transactions. */ -class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notifications +class CWallet final : public CCryptoKeyStore, public interfaces::Chain::Notifications { private: std::atomic fAbortRescan{false}; @@ -954,9 +954,6 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific /** Registered interfaces::Chain::Notifications handler. */ std::unique_ptr m_chain_notifications_handler; - /** Register the wallet for chain notifications */ - void handleNotifications(); - /** Interface for accessing chain state. */ interfaces::Chain& chain() const { assert(m_chain); return *m_chain; } diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 315b92199fed..30129440219d 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -111,17 +111,16 @@ def run_test(self): f.write("datadir=" + new_data_dir + "\n") f.write(conf_file_contents) - # Temporarily disabled, because this test would access the user's home dir (~/.bitcoin) - #self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.') + self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error: Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.') # Create the directory and ensure the config file now works os.mkdir(new_data_dir) # Temporarily disabled, because this test would access the user's home dir (~/.bitcoin) - #self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) - #self.stop_node(0) - #assert os.path.exists(os.path.join(new_data_dir, self.chain, 'blocks')) - #if self.is_wallet_compiled(): - #assert os.path.exists(os.path.join(new_data_dir, self.chain, 'wallets', 'w1')) + self.start_node(0, ['-conf='+conf_file, '-wallet=w1']) + self.stop_node(0) + assert os.path.exists(os.path.join(new_data_dir, self.chain, 'blocks')) + if self.is_wallet_compiled(): + assert os.path.exists(os.path.join(new_data_dir, self.chain, 'wallets', 'w1')) # Ensure command line argument overrides datadir in conf os.mkdir(new_data_dir_2)