diff options
Diffstat (limited to 'src')
76 files changed, 341 insertions, 303 deletions
diff --git a/src/Makefile.am b/src/Makefile.am index d25b27dd3e..b6f0daaaba 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -271,7 +271,6 @@ BITCOIN_CORE_H = \ script/sign.h \ script/signingprovider.h \ script/solver.h \ - shutdown.h \ signet.h \ streams.h \ support/allocators/pool.h \ @@ -458,7 +457,6 @@ libbitcoin_node_a_SOURCES = \ rpc/signmessage.cpp \ rpc/txoutproof.cpp \ script/sigcache.cpp \ - shutdown.cpp \ signet.cpp \ timedata.cpp \ torcontrol.cpp \ diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index ba3c25d25e..993c4c4b3f 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -34,7 +34,7 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) fs::path wallet_path = test_setup->m_path_root / strprintf("test_wallet_%d", random.rand32()).c_str(); bench.run([&] { - auto wallet = CreateWallet(context, wallet_path.u8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); + auto wallet = CreateWallet(context, wallet_path.utf8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); assert(status == DatabaseStatus::SUCCESS); assert(wallet != nullptr); diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index c0ca8f983d..84e02d2a26 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -129,7 +129,7 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type bench.epochIterations(5).run([&] { LOCK(wallet.cs_wallet); - const auto& tx_res = CreateTransaction(wallet, recipients, -1, coin_control); + const auto& tx_res = CreateTransaction(wallet, recipients, /*change_pos=*/std::nullopt, coin_control); assert(tx_res); }); } diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 31edb86ce2..ee8b0a44c5 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -125,14 +125,14 @@ int main(int argc, char* argv[]) .blocks_dir = abs_datadir / "blocks", .notifications = chainman_opts.notifications, }; - ChainstateManager chainman{kernel_context.interrupt, chainman_opts, blockman_opts}; + util::SignalInterrupt interrupt; + ChainstateManager chainman{interrupt, chainman_opts, blockman_opts}; node::CacheSizes cache_sizes; cache_sizes.block_tree_db = 2 << 20; cache_sizes.coins_db = 2 << 22; cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22); node::ChainstateLoadOptions options; - options.check_interrupt = [] { return false; }; auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options); if (status != node::ChainstateLoadStatus::SUCCESS) { std::cerr << "Failed to load Chain state from your datadir." << std::endl; diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 8fe2881f6f..320624c419 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -66,7 +66,9 @@ static void SetupBitcoinTxArgs(ArgsManager &argsman) argsman.AddArg("outscript=VALUE:SCRIPT[:FLAGS]", "Add raw script output to TX. " "Optionally add the \"W\" flag to produce a pay-to-witness-script-hash output. " "Optionally add the \"S\" flag to wrap the output in a pay-to-script-hash.", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); - argsman.AddArg("replaceable(=N)", "Set RBF opt-in sequence number for input N (if not provided, opt-in all available inputs)", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); + argsman.AddArg("replaceable(=N)", "Sets Replace-By-Fee (RBF) opt-in sequence number for input N. " + "If N is not provided, the command attempts to opt-in all available inputs for RBF. " + "If the transaction has no inputs, this option is ignored.", ArgsManager::ALLOW_ANY, OptionsCategory::COMMANDS); argsman.AddArg("sign=SIGHASH-FLAGS", "Add zero or more signatures to transaction. " "This command requires JSON registers:" "prevtxs=JSON object, " @@ -223,8 +225,8 @@ static void MutateTxLocktime(CMutableTransaction& tx, const std::string& cmdVal) static void MutateTxRBFOptIn(CMutableTransaction& tx, const std::string& strInIdx) { // parse requested index - int64_t inIdx; - if (!ParseInt64(strInIdx, &inIdx) || inIdx < 0 || inIdx >= static_cast<int64_t>(tx.vin.size())) { + int64_t inIdx = -1; + if (strInIdx != "" && (!ParseInt64(strInIdx, &inIdx) || inIdx < 0 || inIdx >= static_cast<int64_t>(tx.vin.size()))) { throw std::runtime_error("Invalid TX input index '" + strInIdx + "'"); } diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index e2224befef..4f0a816388 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -20,7 +20,6 @@ #include <node/context.h> #include <node/interface_ui.h> #include <noui.h> -#include <shutdown.h> #include <util/check.h> #include <util/exception.h> #include <util/strencodings.h> @@ -272,9 +271,7 @@ MAIN_FUNCTION if (ProcessInitCommands(args)) return EXIT_SUCCESS; // Start application - if (AppInit(node)) { - WaitForShutdown(); - } else { + if (!AppInit(node) || !Assert(node.shutdown)->wait()) { node.exit_status = EXIT_FAILURE; } Interrupt(node); diff --git a/src/common/args.cpp b/src/common/args.cpp index 1f25d13bee..a9108e5916 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -277,7 +277,7 @@ fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) return result.has_filename() ? result : result.parent_path(); } -const fs::path& ArgsManager::GetBlocksDirPath() const +fs::path ArgsManager::GetBlocksDirPath() const { LOCK(cs_args); fs::path& path = m_cached_blocks_path; @@ -302,7 +302,7 @@ const fs::path& ArgsManager::GetBlocksDirPath() const return path; } -const fs::path& ArgsManager::GetDataDir(bool net_specific) const +fs::path ArgsManager::GetDataDir(bool net_specific) const { LOCK(cs_args); fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path; diff --git a/src/common/args.h b/src/common/args.h index 1c5db718f4..6451b194d1 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -215,21 +215,21 @@ protected: * * @return Blocks path which is network specific */ - const fs::path& GetBlocksDirPath() const; + fs::path GetBlocksDirPath() const; /** * Get data directory path * * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned */ - const fs::path& GetDataDirBase() const { return GetDataDir(false); } + fs::path GetDataDirBase() const { return GetDataDir(false); } /** * Get data directory path with appended network identifier * * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned */ - const fs::path& GetDataDirNet() const { return GetDataDir(true); } + fs::path GetDataDirNet() const { return GetDataDir(true); } /** * Clear cached directory paths @@ -420,7 +420,7 @@ private: * @param net_specific Append network identifier to the returned path * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned */ - const fs::path& GetDataDir(bool net_specific) const; + fs::path GetDataDir(bool net_specific) const; /** * Return -regtest/-signet/-testnet/-chain= setting as a ChainType enum if a diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 4c14c5b939..e26ea6a596 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -15,9 +15,9 @@ #include <netbase.h> #include <node/interface_ui.h> #include <rpc/protocol.h> // For HTTP status codes -#include <shutdown.h> #include <sync.h> #include <util/check.h> +#include <util/signalinterrupt.h> #include <util/strencodings.h> #include <util/threadnames.h> #include <util/translation.h> @@ -284,7 +284,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg) } } } - std::unique_ptr<HTTPRequest> hreq(new HTTPRequest(req)); + auto hreq{std::make_unique<HTTPRequest>(req, *static_cast<const util::SignalInterrupt*>(arg))}; // Early address-based allow check if (!ClientAllowed(hreq->GetPeer())) { @@ -425,7 +425,7 @@ static void libevent_log_cb(int severity, const char *msg) LogPrintLevel(BCLog::LIBEVENT, level, "%s\n", msg); } -bool InitHTTPServer() +bool InitHTTPServer(const util::SignalInterrupt& interrupt) { if (!InitHTTPAllowList()) return false; @@ -454,7 +454,7 @@ bool InitHTTPServer() evhttp_set_timeout(http, gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)); evhttp_set_max_headers_size(http, MAX_HEADERS_SIZE); evhttp_set_max_body_size(http, MAX_SIZE); - evhttp_set_gencb(http, http_request_cb, nullptr); + evhttp_set_gencb(http, http_request_cb, (void*)&interrupt); if (!HTTPBindAddresses(http)) { LogPrintf("Unable to bind any endpoint for RPC server\n"); @@ -579,7 +579,8 @@ void HTTPEvent::trigger(struct timeval* tv) else evtimer_add(ev, tv); // trigger after timeval passed } -HTTPRequest::HTTPRequest(struct evhttp_request* _req, bool _replySent) : req(_req), replySent(_replySent) +HTTPRequest::HTTPRequest(struct evhttp_request* _req, const util::SignalInterrupt& interrupt, bool _replySent) + : req(_req), m_interrupt(interrupt), replySent(_replySent) { } @@ -639,7 +640,7 @@ void HTTPRequest::WriteHeader(const std::string& hdr, const std::string& value) void HTTPRequest::WriteReply(int nStatus, const std::string& strReply) { assert(!replySent && req); - if (ShutdownRequested()) { + if (m_interrupt) { WriteHeader("Connection", "close"); } // Send event to main http thread to send reply message diff --git a/src/httpserver.h b/src/httpserver.h index 036a39a023..9a49877f09 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -9,6 +9,10 @@ #include <optional> #include <string> +namespace util { +class SignalInterrupt; +} // namespace util + static const int DEFAULT_HTTP_THREADS=4; static const int DEFAULT_HTTP_WORKQUEUE=16; static const int DEFAULT_HTTP_SERVER_TIMEOUT=30; @@ -21,7 +25,7 @@ class HTTPRequest; /** Initialize HTTP server. * Call this before RegisterHTTPHandler or EventBase(). */ -bool InitHTTPServer(); +bool InitHTTPServer(const util::SignalInterrupt& interrupt); /** Start HTTP server. * This is separate from InitHTTPServer to give users race-condition-free time * to register their handlers between InitHTTPServer and StartHTTPServer. @@ -57,10 +61,11 @@ class HTTPRequest { private: struct evhttp_request* req; + const util::SignalInterrupt& m_interrupt; bool replySent; public: - explicit HTTPRequest(struct evhttp_request* req, bool replySent = false); + explicit HTTPRequest(struct evhttp_request* req, const util::SignalInterrupt& interrupt, bool replySent = false); ~HTTPRequest(); enum RequestMethod { diff --git a/src/index/base.cpp b/src/index/base.cpp index 8474d01c41..bcfe7215be 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -13,7 +13,6 @@ #include <node/context.h> #include <node/database_args.h> #include <node/interface_ui.h> -#include <shutdown.h> #include <tinyformat.h> #include <util/thread.h> #include <util/translation.h> @@ -32,7 +31,7 @@ template <typename... Args> void BaseIndex::FatalErrorf(const char* fmt, const Args&... args) { auto message = tfm::format(fmt, args...); - node::AbortNode(m_chain->context()->exit_status, message); + node::AbortNode(m_chain->context()->shutdown, m_chain->context()->exit_status, message); } CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) diff --git a/src/init.cpp b/src/init.cpp index d1a0ca1c3b..481d5d398d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -66,7 +66,6 @@ #include <rpc/util.h> #include <scheduler.h> #include <script/sigcache.h> -#include <shutdown.h> #include <sync.h> #include <timedata.h> #include <torcontrol.h> @@ -192,6 +191,16 @@ static void RemovePidFile(const ArgsManager& args) } } +static std::optional<util::SignalInterrupt> g_shutdown; + +void InitContext(NodeContext& node) +{ + assert(!g_shutdown); + g_shutdown.emplace(); + + node.args = &gArgs; + node.shutdown = &*g_shutdown; +} ////////////////////////////////////////////////////////////////////////////// // @@ -204,11 +213,9 @@ static void RemovePidFile(const ArgsManager& args) // The network-processing threads are all part of a thread group // created by AppInit() or the Qt main() function. // -// A clean exit happens when StartShutdown() or the SIGTERM -// signal handler sets ShutdownRequested(), which makes main thread's -// WaitForShutdown() interrupts the thread group. -// And then, WaitForShutdown() makes all other on-going threads -// in the thread group join the main thread. +// A clean exit happens when the SignalInterrupt object is triggered, which +// makes the main thread's SignalInterrupt::wait() call return, and join all +// other ongoing threads in the thread group to the main thread. // Shutdown() is then called to clean up database connections, and stop other // threads that should only be stopped after the main network-processing // threads have exited. @@ -218,6 +225,11 @@ static void RemovePidFile(const ArgsManager& args) // shutdown thing. // +bool ShutdownRequested(node::NodeContext& node) +{ + return bool{*Assert(node.shutdown)}; +} + #if HAVE_SYSTEM static void ShutdownNotify(const ArgsManager& args) { @@ -382,7 +394,9 @@ void Shutdown(NodeContext& node) #ifndef WIN32 static void HandleSIGTERM(int) { - StartShutdown(); + // Return value is intentionally ignored because there is not a better way + // of handling this failure in a signal handler. + (void)(*Assert(g_shutdown))(); } static void HandleSIGHUP(int) @@ -392,7 +406,10 @@ static void HandleSIGHUP(int) #else static BOOL WINAPI consoleCtrlHandler(DWORD dwCtrlType) { - StartShutdown(); + if (!(*Assert(g_shutdown))()) { + LogPrintf("Error: failed to send shutdown signal on Ctrl-C\n"); + return false; + } Sleep(INFINITE); return true; } @@ -690,8 +707,9 @@ static bool AppInitServers(NodeContext& node) const ArgsManager& args = *Assert(node.args); RPCServer::OnStarted(&OnRPCStarted); RPCServer::OnStopped(&OnRPCStopped); - if (!InitHTTPServer()) + if (!InitHTTPServer(*Assert(node.shutdown))) { return false; + } StartRPC(); node.rpc_interruption_point = RpcInterruptionPoint; if (!StartHTTPRPC(&node)) @@ -1141,11 +1159,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) }, std::chrono::minutes{1}); // Check disk space every 5 minutes to avoid db corruption. - node.scheduler->scheduleEvery([&args]{ + node.scheduler->scheduleEvery([&args, &node]{ constexpr uint64_t min_disk_space = 50 << 20; // 50 MB if (!CheckDiskSpace(args.GetBlocksDirPath(), min_disk_space)) { LogPrintf("Shutting down due to lack of disk space!\n"); - StartShutdown(); + if (!(*Assert(node.shutdown))()) { + LogPrintf("Error: failed to send shutdown signal after disk space check\n"); + } } }, std::chrono::minutes{5}); @@ -1432,7 +1452,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 7: load block chain - node.notifications = std::make_unique<KernelNotifications>(node.exit_status); + node.notifications = std::make_unique<KernelNotifications>(*Assert(node.shutdown), node.exit_status); ReadNotificationArgs(args, *node.notifications); fReindex = args.GetBoolArg("-reindex", false); bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false); @@ -1483,10 +1503,10 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), mempool_opts.max_size_bytes * (1.0 / 1024 / 1024)); - for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) { + for (bool fLoaded = false; !fLoaded && !ShutdownRequested(node);) { node.mempool = std::make_unique<CTxMemPool>(mempool_opts); - node.chainman = std::make_unique<ChainstateManager>(node.kernel->interrupt, chainman_opts, blockman_opts); + node.chainman = std::make_unique<ChainstateManager>(*Assert(node.shutdown), chainman_opts, blockman_opts); ChainstateManager& chainman = *node.chainman; // This is defined and set here instead of inline in validation.h to avoid a hard @@ -1516,7 +1536,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS); options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL); options.require_full_verification = args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel"); - options.check_interrupt = ShutdownRequested; options.coins_error_cb = [] { uiInterface.ThreadSafeMessageBox( _("Error reading from database, shutting down."), @@ -1551,7 +1570,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) return InitError(error); } - if (!fLoaded && !ShutdownRequested()) { + if (!fLoaded && !ShutdownRequested(node)) { // first suggest a reindex if (!options.reindex) { bool fRet = uiInterface.ThreadSafeQuestion( @@ -1560,7 +1579,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) "", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT); if (fRet) { fReindex = true; - AbortShutdown(); + if (!Assert(node.shutdown)->reset()) { + LogPrintf("Internal error: failed to reset shutdown signal.\n"); + } } else { LogPrintf("Aborted block database rebuild. Exiting.\n"); return false; @@ -1574,7 +1595,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // As LoadBlockIndex can take several minutes, it's possible the user // requested to kill the GUI during the last operation. If so, exit. // As the program has not fully started yet, Shutdown() is possibly overkill. - if (ShutdownRequested()) { + if (ShutdownRequested(node)) { LogPrintf("Shutdown requested. Exiting.\n"); return false; } @@ -1695,7 +1716,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) ImportBlocks(chainman, vImportFiles); if (args.GetBoolArg("-stopafterblockimport", DEFAULT_STOPAFTERBLOCKIMPORT)) { LogPrintf("Stopping after block import\n"); - StartShutdown(); + if (!(*Assert(node.shutdown))()) { + LogPrintf("Error: failed to send shutdown signal after finishing block import\n"); + } return; } @@ -1715,16 +1738,16 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Wait for genesis block to be processed { WAIT_LOCK(g_genesis_wait_mutex, lock); - // We previously could hang here if StartShutdown() is called prior to + // We previously could hang here if shutdown was requested prior to // ImportBlocks getting started, so instead we just wait on a timer to // check ShutdownRequested() regularly. - while (!fHaveGenesis && !ShutdownRequested()) { + while (!fHaveGenesis && !ShutdownRequested(node)) { g_genesis_wait_cv.wait_for(lock, std::chrono::milliseconds(500)); } block_notify_genesis_wait_connection.disconnect(); } - if (ShutdownRequested()) { + if (ShutdownRequested(node)) { return false; } diff --git a/src/init.h b/src/init.h index f27d6120ef..ead5f5e0d2 100644 --- a/src/init.h +++ b/src/init.h @@ -26,6 +26,11 @@ namespace node { struct NodeContext; } // namespace node +/** Initialize node context shutdown and args variables. */ +void InitContext(node::NodeContext& node); +/** Return whether node shutdown was requested. */ +bool ShutdownRequested(node::NodeContext& node); + /** Interrupt threads */ void Interrupt(node::NodeContext& node); void Shutdown(node::NodeContext& node); diff --git a/src/init/bitcoin-gui.cpp b/src/init/bitcoin-gui.cpp index ddbdaa6cd0..aceff1e40f 100644 --- a/src/init/bitcoin-gui.cpp +++ b/src/init/bitcoin-gui.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <common/args.h> +#include <init.h> #include <interfaces/chain.h> #include <interfaces/echo.h> #include <interfaces/init.h> @@ -23,7 +23,7 @@ class BitcoinGuiInit : public interfaces::Init public: BitcoinGuiInit(const char* arg0) : m_ipc(interfaces::MakeIpc(EXE_NAME, arg0, *this)) { - m_node.args = &gArgs; + InitContext(m_node); m_node.init = this; } std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); } diff --git a/src/init/bitcoin-node.cpp b/src/init/bitcoin-node.cpp index b04596f986..97b8dc1161 100644 --- a/src/init/bitcoin-node.cpp +++ b/src/init/bitcoin-node.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <common/args.h> +#include <init.h> #include <interfaces/chain.h> #include <interfaces/echo.h> #include <interfaces/init.h> @@ -25,7 +25,7 @@ public: : m_node(node), m_ipc(interfaces::MakeIpc(EXE_NAME, arg0, *this)) { - m_node.args = &gArgs; + InitContext(m_node); m_node.init = this; } std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); } diff --git a/src/init/bitcoin-qt.cpp b/src/init/bitcoin-qt.cpp index dd5826d982..3003a8fde1 100644 --- a/src/init/bitcoin-qt.cpp +++ b/src/init/bitcoin-qt.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <common/args.h> +#include <init.h> #include <interfaces/chain.h> #include <interfaces/echo.h> #include <interfaces/init.h> @@ -20,7 +20,7 @@ class BitcoinQtInit : public interfaces::Init public: BitcoinQtInit() { - m_node.args = &gArgs; + InitContext(m_node); m_node.init = this; } std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); } diff --git a/src/init/bitcoind.cpp b/src/init/bitcoind.cpp index 210608370c..b5df764017 100644 --- a/src/init/bitcoind.cpp +++ b/src/init/bitcoind.cpp @@ -2,7 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include <common/args.h> +#include <init.h> #include <interfaces/chain.h> #include <interfaces/echo.h> #include <interfaces/init.h> @@ -22,7 +22,7 @@ class BitcoindInit : public interfaces::Init public: BitcoindInit(NodeContext& node) : m_node(node) { - m_node.args = &gArgs; + InitContext(m_node); m_node.init = this; } std::unique_ptr<interfaces::Node> makeNode() override { return interfaces::MakeNode(m_node); } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index aeb3797392..328399c4ad 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -8,6 +8,7 @@ #include <blockfilter.h> #include <common/settings.h> #include <primitives/transaction.h> // For CTransactionRef +#include <util/result.h> #include <functional> #include <memory> @@ -260,7 +261,7 @@ public: virtual void getPackageLimits(unsigned int& limit_ancestor_count, unsigned int& limit_descendant_count) = 0; //! Check if transaction will pass the mempool's chain limits. - virtual bool checkChainLimits(const CTransactionRef& tx) = 0; + virtual util::Result<void> checkChainLimits(const CTransactionRef& tx) = 0; //! Estimate smart fee. virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; diff --git a/src/kernel/context.cpp b/src/kernel/context.cpp index 3f4a423531..c60f1638d1 100644 --- a/src/kernel/context.cpp +++ b/src/kernel/context.cpp @@ -14,12 +14,8 @@ namespace kernel { -Context* g_context; - Context::Context() { - assert(!g_context); - g_context = this; std::string sha256_algo = SHA256AutoDetect(); LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo); RandomInit(); @@ -29,8 +25,6 @@ Context::Context() Context::~Context() { ECC_Stop(); - assert(g_context); - g_context = nullptr; } } // namespace kernel diff --git a/src/kernel/context.h b/src/kernel/context.h index e74e0a6f08..ff4df20473 100644 --- a/src/kernel/context.h +++ b/src/kernel/context.h @@ -18,24 +18,9 @@ namespace kernel { //! State stored directly in this struct should be simple. More complex state //! should be stored to std::unique_ptr members pointing to opaque types. struct Context { - //! Interrupt object that can be used to stop long-running kernel operations. - util::SignalInterrupt interrupt; - - //! Declare default constructor and destructor that are not inline, so code - //! instantiating the kernel::Context struct doesn't need to #include class - //! definitions for all the unique_ptr members. Context(); ~Context(); }; - -//! Global pointer to kernel::Context for legacy code. New code should avoid -//! using this, and require state it needs to be passed to it directly. -//! -//! Having this pointer is useful because it allows state be moved out of global -//! variables into the kernel::Context struct before all global references to -//! that state are removed. This allows the global references to be removed -//! incrementally, instead of all at once. -extern Context* g_context; } // namespace kernel #endif // BITCOIN_KERNEL_CONTEXT_H diff --git a/src/node/abort.cpp b/src/node/abort.cpp index a554126b86..1bdc91670d 100644 --- a/src/node/abort.cpp +++ b/src/node/abort.cpp @@ -6,7 +6,7 @@ #include <logging.h> #include <node/interface_ui.h> -#include <shutdown.h> +#include <util/signalinterrupt.h> #include <util/translation.h> #include <warnings.h> @@ -16,12 +16,14 @@ namespace node { -void AbortNode(std::atomic<int>& exit_status, const std::string& debug_message, const bilingual_str& user_message, bool shutdown) +void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const std::string& debug_message, const bilingual_str& user_message) { SetMiscWarning(Untranslated(debug_message)); LogPrintf("*** %s\n", debug_message); InitError(user_message.empty() ? _("A fatal internal error occurred, see debug.log for details") : user_message); exit_status.store(EXIT_FAILURE); - if (shutdown) StartShutdown(); + if (shutdown && !(*shutdown)()) { + LogPrintf("Error: failed to send shutdown signal\n"); + }; } } // namespace node diff --git a/src/node/abort.h b/src/node/abort.h index d6bb0c14d5..28d021cc78 100644 --- a/src/node/abort.h +++ b/src/node/abort.h @@ -10,8 +10,12 @@ #include <atomic> #include <string> +namespace util { +class SignalInterrupt; +} // namespace util + namespace node { -void AbortNode(std::atomic<int>& exit_status, const std::string& debug_message, const bilingual_str& user_message = {}, bool shutdown = true); +void AbortNode(util::SignalInterrupt* shutdown, std::atomic<int>& exit_status, const std::string& debug_message, const bilingual_str& user_message = {}); } // namespace node #endif // BITCOIN_NODE_ABORT_H diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index eb1994177a..bf1fc06b0b 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -17,6 +17,7 @@ #include <txdb.h> #include <uint256.h> #include <util/fs.h> +#include <util/signalinterrupt.h> #include <util/time.h> #include <util/translation.h> #include <validation.h> @@ -55,14 +56,14 @@ static ChainstateLoadResult CompleteChainstateInitialization( } } - if (options.check_interrupt && options.check_interrupt()) return {ChainstateLoadStatus::INTERRUPTED, {}}; + if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}}; // LoadBlockIndex will load m_have_pruned if we've ever removed a // block file from disk. // Note that it also sets fReindex global based on the disk flag! // From here on, fReindex and options.reindex values may be different! if (!chainman.LoadBlockIndex()) { - if (options.check_interrupt && options.check_interrupt()) return {ChainstateLoadStatus::INTERRUPTED, {}}; + if (chainman.m_interrupt) return {ChainstateLoadStatus::INTERRUPTED, {}}; return {ChainstateLoadStatus::FAILURE, _("Error loading block database")}; } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index 2e35035c28..a6e9a0331b 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -32,7 +32,6 @@ struct ChainstateLoadOptions { bool require_full_verification{true}; int64_t check_blocks{DEFAULT_CHECKBLOCKS}; int64_t check_level{DEFAULT_CHECKLEVEL}; - std::function<bool()> check_interrupt; std::function<void()> coins_error_cb; }; diff --git a/src/node/context.h b/src/node/context.h index dae900ff7f..4f3b640b2d 100644 --- a/src/node/context.h +++ b/src/node/context.h @@ -50,6 +50,8 @@ struct NodeContext { std::unique_ptr<kernel::Context> kernel; //! Init interface for initializing current process and connecting to other processes. interfaces::Init* init{nullptr}; + //! Interrupt object used to track whether node shutdown was requested. + util::SignalInterrupt* shutdown{nullptr}; std::unique_ptr<AddrMan> addrman; std::unique_ptr<CConnman> connman; std::unique_ptr<CTxMemPool> mempool; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index b5691f0e57..6963e928fe 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -39,13 +39,14 @@ #include <primitives/transaction.h> #include <rpc/protocol.h> #include <rpc/server.h> -#include <shutdown.h> #include <support/allocators/secure.h> #include <sync.h> #include <txmempool.h> #include <uint256.h> #include <univalue.h> #include <util/check.h> +#include <util/result.h> +#include <util/signalinterrupt.h> #include <util/translation.h> #include <validation.h> #include <validationinterface.h> @@ -120,14 +121,16 @@ public: } void startShutdown() override { - StartShutdown(); + if (!(*Assert(Assert(m_context)->shutdown))()) { + LogPrintf("Error: failed to send shutdown signal\n"); + } // Stop RPC for clean shutdown if any of waitfor* commands is executed. if (args().GetBoolArg("-server", false)) { InterruptRPC(); StopRPC(); } } - bool shutdownRequested() override { return ShutdownRequested(); } + bool shutdownRequested() override { return ShutdownRequested(*Assert(m_context)); }; bool isSettingIgnored(const std::string& name) override { bool ignored = false; @@ -700,14 +703,13 @@ public: limit_ancestor_count = limits.ancestor_count; limit_descendant_count = limits.descendant_count; } - bool checkChainLimits(const CTransactionRef& tx) override + util::Result<void> checkChainLimits(const CTransactionRef& tx) override { - if (!m_node.mempool) return true; + if (!m_node.mempool) return {}; LockPoints lp; CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); LOCK(m_node.mempool->cs); - std::string err_string; - return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize(), err_string); + return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize()); } CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc) override { @@ -749,7 +751,7 @@ public: { return chainman().IsInitialBlockDownload(); } - bool shutdownRequested() override { return ShutdownRequested(); } + bool shutdownRequested() override { return ShutdownRequested(m_node); } void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } void initWarning(const bilingual_str& message) override { InitWarning(message); } void initError(const bilingual_str& message) override { InitError(message); } diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 7224127c72..1fd3bad296 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -15,7 +15,6 @@ #include <logging.h> #include <node/abort.h> #include <node/interface_ui.h> -#include <shutdown.h> #include <util/check.h> #include <util/strencodings.h> #include <util/string.h> @@ -62,7 +61,9 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state { uiInterface.NotifyBlockTip(state, &index); if (m_stop_at_height && index.nHeight >= m_stop_at_height) { - StartShutdown(); + if (!m_shutdown()) { + LogPrintf("Error: failed to send shutdown signal after reaching stop height\n"); + } return kernel::Interrupted{}; } return {}; @@ -85,12 +86,13 @@ void KernelNotifications::warning(const bilingual_str& warning) void KernelNotifications::flushError(const std::string& debug_message) { - AbortNode(m_exit_status, debug_message); + AbortNode(&m_shutdown, m_exit_status, debug_message); } void KernelNotifications::fatalError(const std::string& debug_message, const bilingual_str& user_message) { - node::AbortNode(m_exit_status, debug_message, user_message, m_shutdown_on_fatal_error); + node::AbortNode(m_shutdown_on_fatal_error ? &m_shutdown : nullptr, + m_exit_status, debug_message, user_message); } void ReadNotificationArgs(const ArgsManager& args, KernelNotifications& notifications) diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index b2dfc03398..38d8600ac6 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -16,6 +16,10 @@ class CBlockIndex; enum class SynchronizationState; struct bilingual_str; +namespace util { +class SignalInterrupt; +} // namespace util + namespace node { static constexpr int DEFAULT_STOPATHEIGHT{0}; @@ -23,7 +27,7 @@ static constexpr int DEFAULT_STOPATHEIGHT{0}; class KernelNotifications : public kernel::Notifications { public: - KernelNotifications(std::atomic<int>& exit_status) : m_exit_status{exit_status} {} + KernelNotifications(util::SignalInterrupt& shutdown, std::atomic<int>& exit_status) : m_shutdown(shutdown), m_exit_status{exit_status} {} [[nodiscard]] kernel::InterruptResult blockTip(SynchronizationState state, CBlockIndex& index) override; @@ -42,6 +46,7 @@ public: //! Useful for tests, can be set to false to avoid shutdown on fatal error. bool m_shutdown_on_fatal_error{true}; private: + util::SignalInterrupt& m_shutdown; std::atomic<int>& m_exit_status; }; diff --git a/src/policy/feerate.h b/src/policy/feerate.h index 41f4a4d06b..2e50172914 100644 --- a/src/policy/feerate.h +++ b/src/policy/feerate.h @@ -71,6 +71,8 @@ public: friend bool operator!=(const CFeeRate& a, const CFeeRate& b) { return a.nSatoshisPerK != b.nSatoshisPerK; } CFeeRate& operator+=(const CFeeRate& a) { nSatoshisPerK += a.nSatoshisPerK; return *this; } std::string ToString(const FeeEstimateMode& fee_estimate_mode = FeeEstimateMode::BTC_KVB) const; + friend CFeeRate operator*(const CFeeRate& f, int a) { return CFeeRate(a * f.nSatoshisPerK); } + friend CFeeRate operator*(int a, const CFeeRate& f) { return CFeeRate(a * f.nSatoshisPerK); } SERIALIZE_METHODS(CFeeRate, obj) { READWRITE(obj.nSatoshisPerK); } }; diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index a9a9b39bf4..33c305f0d4 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -661,7 +661,10 @@ int GuiMain(int argc, char* argv[]) app.installEventFilter(new GUIUtil::LabelOutOfFocusEventFilter(&app)); #if defined(Q_OS_WIN) // Install global event filter for processing Windows session related Windows messages (WM_QUERYENDSESSION and WM_ENDSESSION) - qApp->installNativeEventFilter(new WinShutdownMonitor()); + // Note: it is safe to call app.node() in the lambda below despite the fact + // that app.createNode() hasn't been called yet, because native events will + // not be processed until the Qt event loop is executed. + qApp->installNativeEventFilter(new WinShutdownMonitor([&app] { app.node().startShutdown(); })); #endif // Install qDebug() message handler to route to debug.log qInstallMessageHandler(DebugMessageHandler); diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 6fdc4c60d8..ba91eeb1ff 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -315,7 +315,7 @@ public Q_SLOTS: /** Simply calls showNormalIfMinimized(true) */ void toggleHidden(); - /** called by a timer to check if ShutdownRequested() has been set **/ + /** called by a timer to check if shutdown has been requested */ void detectShutdown(); /** Show progress dialog e.g. for verifychain */ diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index eb9c65caf2..ebfd14cc25 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -669,7 +669,7 @@ fs::path QStringToPath(const QString &path) QString PathToQString(const fs::path &path) { - return QString::fromStdString(path.u8string()); + return QString::fromStdString(path.utf8string()); } QString NetworkToQString(Network net) @@ -723,8 +723,7 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction QString formatDurationStr(std::chrono::seconds dur) { - using days = std::chrono::duration<int, std::ratio<86400>>; // can remove this line after C++20 - const auto d{std::chrono::duration_cast<days>(dur)}; + const auto d{std::chrono::duration_cast<std::chrono::days>(dur)}; const auto h{std::chrono::duration_cast<std::chrono::hours>(dur - d)}; const auto m{std::chrono::duration_cast<std::chrono::minutes>(dur - d - h)}; const auto s{std::chrono::duration_cast<std::chrono::seconds>(dur - d - h - m)}; diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index b05009965f..9007b183aa 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -11,7 +11,6 @@ #include <qt/bitcoingui.h> #include <qt/networkstyle.h> #include <qt/rpcconsole.h> -#include <shutdown.h> #include <test/util/setup_common.h> #include <validation.h> diff --git a/src/qt/winshutdownmonitor.cpp b/src/qt/winshutdownmonitor.cpp index 97a9ec318c..0b5278c192 100644 --- a/src/qt/winshutdownmonitor.cpp +++ b/src/qt/winshutdownmonitor.cpp @@ -5,7 +5,6 @@ #include <qt/winshutdownmonitor.h> #if defined(Q_OS_WIN) -#include <shutdown.h> #include <windows.h> @@ -25,7 +24,7 @@ bool WinShutdownMonitor::nativeEventFilter(const QByteArray &eventType, void *pM { // Initiate a client shutdown after receiving a WM_QUERYENDSESSION and block // Windows session end until we have finished client shutdown. - StartShutdown(); + m_shutdown_fn(); *pnResult = FALSE; return true; } diff --git a/src/qt/winshutdownmonitor.h b/src/qt/winshutdownmonitor.h index 72655da3da..78f287637f 100644 --- a/src/qt/winshutdownmonitor.h +++ b/src/qt/winshutdownmonitor.h @@ -8,6 +8,7 @@ #ifdef WIN32 #include <QByteArray> #include <QString> +#include <functional> #include <windef.h> // for HWND @@ -16,11 +17,16 @@ class WinShutdownMonitor : public QAbstractNativeEventFilter { public: + WinShutdownMonitor(std::function<void()> shutdown_fn) : m_shutdown_fn{std::move(shutdown_fn)} {} + /** Implements QAbstractNativeEventFilter interface for processing Windows messages */ bool nativeEventFilter(const QByteArray &eventType, void *pMessage, long *pnResult) override; /** Register the reason for blocking shutdown on Windows to allow clean client exit */ static void registerShutdownBlockReason(const QString& strReason, const HWND& mainWinId); + +private: + std::function<void()> m_shutdown_fn; }; #endif diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6f20b1031c..afd80d8788 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2601,7 +2601,7 @@ static RPCHelpMan dumptxoutset() if (fs::exists(path)) { throw JSONRPCError( RPC_INVALID_PARAMETER, - path.u8string() + " already exists. If you are sure this is what you want, " + path.utf8string() + " already exists. If you are sure this is what you want, " "move it out of the way first"); } @@ -2610,7 +2610,7 @@ static RPCHelpMan dumptxoutset() if (afile.IsNull()) { throw JSONRPCError( RPC_INVALID_PARAMETER, - "Couldn't open file " + temppath.u8string() + " for writing."); + "Couldn't open file " + temppath.utf8string() + " for writing."); } NodeContext& node = EnsureAnyNodeContext(request.context); @@ -2618,7 +2618,7 @@ static RPCHelpMan dumptxoutset() node, node.chainman->ActiveChainstate(), afile, path, temppath); fs::rename(temppath, path); - result.pushKV("path", path.u8string()); + result.pushKV("path", path.utf8string()); return result; }, }; @@ -2690,7 +2690,7 @@ UniValue CreateUTXOSnapshot( result.pushKV("coins_written", maybe_stats->coins_count); result.pushKV("base_hash", tip->GetBlockHash().ToString()); result.pushKV("base_height", tip->nHeight); - result.pushKV("path", path.u8string()); + result.pushKV("path", path.utf8string()); result.pushKV("txoutset_hash", maybe_stats->hashSerialized.ToString()); result.pushKV("nchaintx", tip->nChainTx); return result; @@ -2743,7 +2743,7 @@ static RPCHelpMan loadtxoutset() if (afile.IsNull()) { throw JSONRPCError( RPC_INVALID_PARAMETER, - "Couldn't open file " + path.u8string() + " for reading."); + "Couldn't open file " + path.utf8string() + " for reading."); } SnapshotMetadata metadata; diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 04bd7fa928..f6d9d42f0f 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -806,7 +806,7 @@ static RPCHelpMan savemempool() } UniValue ret(UniValue::VOBJ); - ret.pushKV("filename", dump_path.u8string()); + ret.pushKV("filename", dump_path.utf8string()); return ret; }, diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b2332c5fdc..fdb694b5f1 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -27,7 +27,6 @@ #include <script/descriptor.h> #include <script/script.h> #include <script/signingprovider.h> -#include <shutdown.h> #include <timedata.h> #include <txmempool.h> #include <univalue.h> @@ -129,11 +128,11 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& block_out.reset(); block.hashMerkleRoot = BlockMerkleRoot(block); - while (max_tries > 0 && block.nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(block.GetHash(), block.nBits, chainman.GetConsensus()) && !ShutdownRequested()) { + while (max_tries > 0 && block.nNonce < std::numeric_limits<uint32_t>::max() && !CheckProofOfWork(block.GetHash(), block.nBits, chainman.GetConsensus()) && !chainman.m_interrupt) { ++block.nNonce; --max_tries; } - if (max_tries == 0 || ShutdownRequested()) { + if (max_tries == 0 || chainman.m_interrupt) { return false; } if (block.nNonce == std::numeric_limits<uint32_t>::max()) { @@ -154,7 +153,7 @@ static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& static UniValue generateBlocks(ChainstateManager& chainman, const CTxMemPool& mempool, const CScript& coinbase_script, int nGenerate, uint64_t nMaxTries) { UniValue blockHashes(UniValue::VARR); - while (nGenerate > 0 && !ShutdownRequested()) { + while (nGenerate > 0 && !chainman.m_interrupt) { std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler{chainman.ActiveChainstate(), &mempool}.CreateNewBlock(coinbase_script)); if (!pblocktemplate.get()) throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 7adc28f416..4883341522 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -8,9 +8,11 @@ #include <common/args.h> #include <common/system.h> #include <logging.h> +#include <node/context.h> +#include <rpc/server_util.h> #include <rpc/util.h> -#include <shutdown.h> #include <sync.h> +#include <util/signalinterrupt.h> #include <util/strencodings.h> #include <util/string.h> #include <util/time.h> @@ -179,7 +181,7 @@ static RPCHelpMan stop() { // Event loop will exit after current HTTP requests have been handled, so // this reply will get back to the client. - StartShutdown(); + CHECK_NONFATAL((*CHECK_NONFATAL(EnsureAnyNodeContext(jsonRequest.context).shutdown))()); if (jsonRequest.params[0].isNum()) { UninterruptibleSleep(std::chrono::milliseconds{jsonRequest.params[0].getInt<int>()}); } @@ -243,7 +245,7 @@ static RPCHelpMan getrpcinfo() UniValue result(UniValue::VOBJ); result.pushKV("active_commands", active_commands); - const std::string path = LogInstance().m_file_path.u8string(); + const std::string path = LogInstance().m_file_path.utf8string(); UniValue log_path(UniValue::VSTR, path); result.pushKV("logpath", log_path); diff --git a/src/shutdown.cpp b/src/shutdown.cpp deleted file mode 100644 index fc18ccd207..0000000000 --- a/src/shutdown.cpp +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2022 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 <shutdown.h> - -#include <kernel/context.h> -#include <logging.h> -#include <util/check.h> -#include <util/signalinterrupt.h> - -#include <assert.h> -#include <system_error> - -void StartShutdown() -{ - try { - Assert(kernel::g_context)->interrupt(); - } catch (const std::system_error&) { - LogPrintf("Sending shutdown token failed\n"); - assert(0); - } -} - -void AbortShutdown() -{ - Assert(kernel::g_context)->interrupt.reset(); -} - -bool ShutdownRequested() -{ - return bool{Assert(kernel::g_context)->interrupt}; -} - -void WaitForShutdown() -{ - try { - Assert(kernel::g_context)->interrupt.wait(); - } catch (const std::system_error&) { - LogPrintf("Reading shutdown token failed\n"); - assert(0); - } -} diff --git a/src/shutdown.h b/src/shutdown.h deleted file mode 100644 index 2d6ace8d93..0000000000 --- a/src/shutdown.h +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2021 The Bitcoin Core developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef BITCOIN_SHUTDOWN_H -#define BITCOIN_SHUTDOWN_H - -/** Request shutdown of the application. */ -void StartShutdown(); - -/** Clear shutdown flag. Only use this during init (before calling WaitForShutdown in any - * thread), or in the unit tests. Calling it in other circumstances will cause a race condition. - */ -void AbortShutdown(); - -/** Returns true if a shutdown is requested, false otherwise. */ -bool ShutdownRequested(); - -/** Wait for StartShutdown to be called in any thread. This can only be used - * from a single thread. - */ -void WaitForShutdown(); - -#endif // BITCOIN_SHUTDOWN_H diff --git a/src/test/amount_tests.cpp b/src/test/amount_tests.cpp index 3815a5bba6..e5ab1cfb90 100644 --- a/src/test/amount_tests.cpp +++ b/src/test/amount_tests.cpp @@ -85,6 +85,32 @@ BOOST_AUTO_TEST_CASE(GetFeeTest) BOOST_CHECK(CFeeRate(CAmount(27), 789) == CFeeRate(34)); // Maximum size in bytes, should not crash CFeeRate(MAX_MONEY, std::numeric_limits<uint32_t>::max()).GetFeePerK(); + + // check multiplication operator + // check multiplying by zero + feeRate = CFeeRate(1000); + BOOST_CHECK(0 * feeRate == CFeeRate(0)); + BOOST_CHECK(feeRate * 0 == CFeeRate(0)); + // check multiplying by a positive integer + BOOST_CHECK(3 * feeRate == CFeeRate(3000)); + BOOST_CHECK(feeRate * 3 == CFeeRate(3000)); + // check multiplying by a negative integer + BOOST_CHECK(-3 * feeRate == CFeeRate(-3000)); + BOOST_CHECK(feeRate * -3 == CFeeRate(-3000)); + // check commutativity + BOOST_CHECK(2 * feeRate == feeRate * 2); + // check with large numbers + int largeNumber = 1000000; + BOOST_CHECK(largeNumber * feeRate == feeRate * largeNumber); + // check boundary values + int maxInt = std::numeric_limits<int>::max(); + feeRate = CFeeRate(maxInt); + BOOST_CHECK(feeRate * 2 == CFeeRate(static_cast<int64_t>(maxInt) * 2)); + BOOST_CHECK(2 * feeRate == CFeeRate(static_cast<int64_t>(maxInt) * 2)); + // check with zero fee rate + feeRate = CFeeRate(0); + BOOST_CHECK(feeRate * 5 == CFeeRate(0)); + BOOST_CHECK(5 * feeRate == CFeeRate(0)); } BOOST_AUTO_TEST_CASE(BinaryOperatorTest) diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index 97ea5cfbf3..a9009948ee 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -143,7 +143,7 @@ BOOST_FIXTURE_TEST_CASE(blockfilter_index_initial_sync, BuildChainTestingSetup) BOOST_REQUIRE(filter_index.StartBackgroundSync()); // Allow filter index to catch up with the block index. - IndexWaitSynced(filter_index); + IndexWaitSynced(filter_index, *Assert(m_node.shutdown)); // Check that filter index has all blocks that were in the chain before it started. { diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index e1b31c20f4..d7ac0bf823 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -27,13 +27,13 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) { const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)}; - KernelNotifications notifications{m_node.exit_status}; + KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status}; const BlockManager::Options blockman_opts{ .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, }; - BlockManager blockman{m_node.kernel->interrupt, blockman_opts}; + BlockManager blockman{*Assert(m_node.shutdown), blockman_opts}; // simulate adding a genesis block normally BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); // simulate what happens during reindex @@ -134,13 +134,13 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup) BOOST_AUTO_TEST_CASE(blockmanager_flush_block_file) { - KernelNotifications notifications{m_node.exit_status}; + KernelNotifications notifications{*Assert(m_node.shutdown), m_node.exit_status}; node::BlockManager::Options blockman_opts{ .chainparams = Params(), .blocks_dir = m_args.GetBlocksDirPath(), .notifications = notifications, }; - BlockManager blockman{m_node.kernel->interrupt, blockman_opts}; + BlockManager blockman{*Assert(m_node.shutdown), blockman_opts}; // Test blocks with no transactions, not even a coinbase CBlock block1; diff --git a/src/test/coinstatsindex_tests.cpp b/src/test/coinstatsindex_tests.cpp index 50f3f7d833..cc1ec49d41 100644 --- a/src/test/coinstatsindex_tests.cpp +++ b/src/test/coinstatsindex_tests.cpp @@ -35,7 +35,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_initial_sync, TestChain100Setup) BOOST_REQUIRE(coin_stats_index.StartBackgroundSync()); - IndexWaitSynced(coin_stats_index); + IndexWaitSynced(coin_stats_index, *Assert(m_node.shutdown)); // Check that CoinStatsIndex works for genesis block. const CBlockIndex* genesis_block_index; @@ -86,7 +86,7 @@ BOOST_FIXTURE_TEST_CASE(coinstatsindex_unclean_shutdown, TestChain100Setup) CoinStatsIndex index{interfaces::MakeChain(m_node), 1 << 20}; BOOST_REQUIRE(index.Init()); BOOST_REQUIRE(index.StartBackgroundSync()); - IndexWaitSynced(index); + IndexWaitSynced(index, *Assert(m_node.shutdown)); std::shared_ptr<const CBlock> new_block; CBlockIndex* new_block_index = nullptr; { diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index 0d25428b33..c237963af3 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -17,9 +17,12 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(fsbridge_pathtostring) { std::string u8_str = "fs_tests_₿_🏃"; + std::u8string str8{u8"fs_tests_₿_🏃"}; BOOST_CHECK_EQUAL(fs::PathToString(fs::PathFromString(u8_str)), u8_str); - BOOST_CHECK_EQUAL(fs::u8path(u8_str).u8string(), u8_str); - BOOST_CHECK_EQUAL(fs::PathFromString(u8_str).u8string(), u8_str); + BOOST_CHECK_EQUAL(fs::u8path(u8_str).utf8string(), u8_str); + BOOST_CHECK_EQUAL(fs::path(str8).utf8string(), u8_str); + BOOST_CHECK(fs::path(str8).u8string() == str8); + BOOST_CHECK_EQUAL(fs::PathFromString(u8_str).utf8string(), u8_str); BOOST_CHECK_EQUAL(fs::PathToString(fs::u8path(u8_str)), u8_str); #ifndef WIN32 // On non-windows systems, verify that arbitrary byte strings containing @@ -46,7 +49,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream) fs::path tmpfolder = m_args.GetDataDirBase(); // tmpfile1 should be the same as tmpfile2 fs::path tmpfile1 = tmpfolder / fs::u8path("fs_tests_₿_🏃"); - fs::path tmpfile2 = tmpfolder / fs::u8path("fs_tests_₿_🏃"); + fs::path tmpfile2 = tmpfolder / fs::path(u8"fs_tests_₿_🏃"); { std::ofstream file{tmpfile1}; file << "bitcoin"; diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index 9928c4a1ab..f13f1c72a5 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -7,6 +7,7 @@ #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> #include <test/fuzz/util.h> +#include <util/signalinterrupt.h> #include <util/strencodings.h> #include <event2/buffer.h> @@ -47,7 +48,8 @@ FUZZ_TARGET(http_request) return; } - HTTPRequest http_request{evreq, true}; + util::SignalInterrupt interrupt; + HTTPRequest http_request{evreq, interrupt, true}; const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod(); (void)RequestMethodString(request_method); (void)http_request.GetURI(); diff --git a/src/test/fuzz/minisketch.cpp b/src/test/fuzz/minisketch.cpp index a17be73f6c..698cb15fc9 100644 --- a/src/test/fuzz/minisketch.cpp +++ b/src/test/fuzz/minisketch.cpp @@ -12,14 +12,27 @@ #include <map> #include <numeric> -using node::MakeMinisketch32; +namespace { + +Minisketch MakeFuzzMinisketch32(size_t capacity, uint32_t impl) +{ + return Assert(Minisketch(32, impl, capacity)); +} + +} // namespace FUZZ_TARGET(minisketch) { FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; + const auto capacity{fuzzed_data_provider.ConsumeIntegralInRange<size_t>(1, 200)}; - Minisketch sketch_a{Assert(MakeMinisketch32(capacity))}; - Minisketch sketch_b{Assert(MakeMinisketch32(capacity))}; + const uint32_t impl{fuzzed_data_provider.ConsumeIntegralInRange<uint32_t>(0, Minisketch::MaxImplementation())}; + if (!Minisketch::ImplementationSupported(32, impl)) return; + + Minisketch sketch_a{MakeFuzzMinisketch32(capacity, impl)}; + Minisketch sketch_b{MakeFuzzMinisketch32(capacity, impl)}; + sketch_a.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); + sketch_b.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); // Fill two sets and keep the difference in a map std::map<uint32_t, bool> diff; @@ -47,8 +60,11 @@ FUZZ_TARGET(minisketch) } const auto num_diff{std::accumulate(diff.begin(), diff.end(), size_t{0}, [](auto n, const auto& e) { return n + e.second; })}; - Minisketch sketch_ar{MakeMinisketch32(capacity)}; - Minisketch sketch_br{MakeMinisketch32(capacity)}; + Minisketch sketch_ar{MakeFuzzMinisketch32(capacity, impl)}; + Minisketch sketch_br{MakeFuzzMinisketch32(capacity, impl)}; + sketch_ar.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); + sketch_br.SetSeed(fuzzed_data_provider.ConsumeIntegral<uint64_t>()); + sketch_ar.Deserialize(sketch_a.Serialize()); sketch_br.Deserialize(sketch_b.Serialize()); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index acb03ac5fc..56b391ed5c 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2020-2022 The Bitcoin Core developers +// Copyright (c) 2020-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -8,9 +8,6 @@ #include <primitives/transaction.h> #include <protocol.h> #include <script/script.h> -#include <serialize.h> -#include <span.h> -#include <streams.h> #include <sync.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> @@ -20,13 +17,10 @@ #include <test/util/net.h> #include <test/util/setup_common.h> #include <test/util/validation.h> -#include <util/chaintype.h> #include <util/check.h> #include <util/time.h> -#include <validation.h> #include <validationinterface.h> -#include <atomic> #include <cstdlib> #include <iostream> #include <memory> @@ -81,8 +75,7 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) CSerializedNetMsg net_msg; net_msg.m_type = random_message_type; - // fuzzed_data_provider is fully consumed after this call, don't use it - net_msg.data = fuzzed_data_provider.ConsumeRemainingBytes<unsigned char>(); + net_msg.data = ConsumeRandomLengthByteVector(fuzzed_data_provider, MAX_PROTOCOL_MESSAGE_LENGTH); connman.FlushSendBuffer(p2p_node); (void)connman.ReceiveMsgFrom(p2p_node, std::move(net_msg)); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 3f722f60ee..6b264907b5 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -16,7 +16,6 @@ #include <test/util/net.h> #include <test/util/setup_common.h> #include <test/util/validation.h> -#include <util/chaintype.h> #include <util/time.h> #include <validationinterface.h> @@ -72,7 +71,7 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) CSerializedNetMsg net_msg; net_msg.m_type = random_message_type; - net_msg.data = ConsumeRandomLengthByteVector(fuzzed_data_provider); + net_msg.data = ConsumeRandomLengthByteVector(fuzzed_data_provider, MAX_PROTOCOL_MESSAGE_LENGTH); CNode& random_node = *PickValue(fuzzed_data_provider, peers); diff --git a/src/test/script_p2sh_tests.cpp b/src/test/script_p2sh_tests.cpp index 739ab75de3..54dcc218b9 100644 --- a/src/test/script_p2sh_tests.cpp +++ b/src/test/script_p2sh_tests.cpp @@ -18,9 +18,15 @@ #include <boost/test/unit_test.hpp> // Helpers: +static bool IsStandardTx(const CTransaction& tx, bool permit_bare_multisig, std::string& reason) +{ + return IsStandardTx(tx, std::nullopt, permit_bare_multisig, CFeeRate{DUST_RELAY_TX_FEE}, reason); +} + static bool IsStandardTx(const CTransaction& tx, std::string& reason) { - return IsStandardTx(tx, std::nullopt, DEFAULT_PERMIT_BAREMULTISIG, CFeeRate{DUST_RELAY_TX_FEE}, reason); + return IsStandardTx(tx, std::nullopt, /*permit_bare_multisig=*/true, CFeeRate{DUST_RELAY_TX_FEE}, reason) && + IsStandardTx(tx, std::nullopt, /*permit_bare_multisig=*/false, CFeeRate{DUST_RELAY_TX_FEE}, reason); } static std::vector<unsigned char> Serialize(const CScript& s) @@ -201,7 +207,9 @@ BOOST_AUTO_TEST_CASE(set) { SignatureData empty; BOOST_CHECK_MESSAGE(SignSignature(keystore, CTransaction(txFrom), txTo[i], 0, SIGHASH_ALL, empty), strprintf("SignSignature %d", i)); - BOOST_CHECK_MESSAGE(IsStandardTx(CTransaction(txTo[i]), reason), strprintf("txTo[%d].IsStandard", i)); + BOOST_CHECK_MESSAGE(IsStandardTx(CTransaction(txTo[i]), /*permit_bare_multisig=*/true, reason), strprintf("txTo[%d].IsStandard", i)); + bool no_pbms_is_std = IsStandardTx(CTransaction(txTo[i]), /*permit_bare_multisig=*/false, reason); + BOOST_CHECK_MESSAGE((i == 0 ? no_pbms_is_std : !no_pbms_is_std), strprintf("txTo[%d].IsStandard(permbaremulti=false)", i)); } } diff --git a/src/test/txindex_tests.cpp b/src/test/txindex_tests.cpp index 9fa59bab57..e2432a4718 100644 --- a/src/test/txindex_tests.cpp +++ b/src/test/txindex_tests.cpp @@ -33,7 +33,7 @@ BOOST_FIXTURE_TEST_CASE(txindex_initial_sync, TestChain100Setup) BOOST_REQUIRE(txindex.StartBackgroundSync()); // Allow tx index to catch up with the block index. - IndexWaitSynced(txindex); + IndexWaitSynced(txindex, *Assert(m_node.shutdown)); // Check that txindex excludes genesis block transactions. const CBlock& genesis_block = Params().GenesisBlock(); diff --git a/src/test/util/index.cpp b/src/test/util/index.cpp index e653d5dbf0..cfeba35756 100644 --- a/src/test/util/index.cpp +++ b/src/test/util/index.cpp @@ -5,16 +5,16 @@ #include <test/util/index.h> #include <index/base.h> -#include <shutdown.h> #include <util/check.h> +#include <util/signalinterrupt.h> #include <util/time.h> -void IndexWaitSynced(const BaseIndex& index) +void IndexWaitSynced(const BaseIndex& index, const util::SignalInterrupt& interrupt) { while (!index.BlockUntilSyncedToCurrentChain()) { // Assert shutdown was not requested to abort the test, instead of looping forever, in case // there was an unexpected error in the index that caused it to stop syncing and request a shutdown. - Assert(!ShutdownRequested()); + Assert(!interrupt); UninterruptibleSleep(100ms); } diff --git a/src/test/util/index.h b/src/test/util/index.h index 95309f6273..a3bd1dddc3 100644 --- a/src/test/util/index.h +++ b/src/test/util/index.h @@ -6,8 +6,11 @@ #define BITCOIN_TEST_UTIL_INDEX_H class BaseIndex; +namespace util { +class SignalInterrupt; +} // namespace util /** Block until the index is synced to the current chain */ -void IndexWaitSynced(const BaseIndex& index); +void IndexWaitSynced(const BaseIndex& index, const util::SignalInterrupt& interrupt); #endif // BITCOIN_TEST_UTIL_INDEX_H diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index bc639da4dd..8789e86196 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -40,7 +40,6 @@ #include <rpc/server.h> #include <scheduler.h> #include <script/sigcache.h> -#include <shutdown.h> #include <streams.h> #include <test/util/net.h> #include <test/util/random.h> @@ -102,6 +101,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto : m_path_root{fs::temp_directory_path() / "test_common_" PACKAGE_NAME / g_insecure_rand_ctx_temp_path.rand256().ToString()}, m_args{} { + m_node.shutdown = &m_interrupt; m_node.args = &gArgs; std::vector<const char*> arguments = Cat( { @@ -179,7 +179,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto m_cache_sizes = CalculateCacheSizes(m_args); - m_node.notifications = std::make_unique<KernelNotifications>(m_node.exit_status); + m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status); const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, @@ -194,7 +194,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto .blocks_dir = m_args.GetBlocksDirPath(), .notifications = chainman_opts.notifications, }; - m_node.chainman = std::make_unique<ChainstateManager>(m_node.kernel->interrupt, chainman_opts, blockman_opts); + m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<BlockTreeDB>(DBParams{ .path = m_args.GetDataDirNet() / "blocks" / "index", .cache_bytes = static_cast<size_t>(m_cache_sizes.block_tree_db), diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h index c3a4df28bf..9ff4c372a5 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -47,6 +47,7 @@ static constexpr CAmount CENT{1000000}; * This just configures logging, data dir and chain parameters. */ struct BasicTestingSetup { + util::SignalInterrupt m_interrupt; node::NodeContext m_node; // keep as first member to be destructed last explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {}); diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 6969822ad7..368ba8bee4 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -379,7 +379,7 @@ struct SnapshotTestSetup : TestChain100Setup { LOCK(::cs_main); chainman.ResetChainstates(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); - m_node.notifications = std::make_unique<KernelNotifications>(m_node.exit_status); + m_node.notifications = std::make_unique<KernelNotifications>(*Assert(m_node.shutdown), m_node.exit_status); const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, @@ -394,7 +394,7 @@ struct SnapshotTestSetup : TestChain100Setup { // For robustness, ensure the old manager is destroyed before creating a // new one. m_node.chainman.reset(); - m_node.chainman = std::make_unique<ChainstateManager>(m_node.kernel->interrupt, chainman_opts, blockman_opts); + m_node.chainman = std::make_unique<ChainstateManager>(*Assert(m_node.shutdown), chainman_opts, blockman_opts); } return *Assert(m_node.chainman); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index b783181bb8..acee56fe78 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -196,25 +196,20 @@ util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateAncestorsAndCheckLimit return ancestors; } -bool CTxMemPool::CheckPackageLimits(const Package& package, - const int64_t total_vsize, - std::string &errString) const +util::Result<void> CTxMemPool::CheckPackageLimits(const Package& package, + const int64_t total_vsize) const { size_t pack_count = package.size(); // Package itself is busting mempool limits; should be rejected even if no staged_ancestors exist if (pack_count > static_cast<uint64_t>(m_limits.ancestor_count)) { - errString = strprintf("package count %u exceeds ancestor count limit [limit: %u]", pack_count, m_limits.ancestor_count); - return false; + return util::Error{Untranslated(strprintf("package count %u exceeds ancestor count limit [limit: %u]", pack_count, m_limits.ancestor_count))}; } else if (pack_count > static_cast<uint64_t>(m_limits.descendant_count)) { - errString = strprintf("package count %u exceeds descendant count limit [limit: %u]", pack_count, m_limits.descendant_count); - return false; + return util::Error{Untranslated(strprintf("package count %u exceeds descendant count limit [limit: %u]", pack_count, m_limits.descendant_count))}; } else if (total_vsize > m_limits.ancestor_size_vbytes) { - errString = strprintf("package size %u exceeds ancestor size limit [limit: %u]", total_vsize, m_limits.ancestor_size_vbytes); - return false; + return util::Error{Untranslated(strprintf("package size %u exceeds ancestor size limit [limit: %u]", total_vsize, m_limits.ancestor_size_vbytes))}; } else if (total_vsize > m_limits.descendant_size_vbytes) { - errString = strprintf("package size %u exceeds descendant size limit [limit: %u]", total_vsize, m_limits.descendant_size_vbytes); - return false; + return util::Error{Untranslated(strprintf("package size %u exceeds descendant size limit [limit: %u]", total_vsize, m_limits.descendant_size_vbytes))}; } CTxMemPoolEntry::Parents staged_ancestors; @@ -224,8 +219,7 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, if (piter) { staged_ancestors.insert(**piter); if (staged_ancestors.size() + package.size() > static_cast<uint64_t>(m_limits.ancestor_count)) { - errString = strprintf("too many unconfirmed parents [limit: %u]", m_limits.ancestor_count); - return false; + return util::Error{Untranslated(strprintf("too many unconfirmed parents [limit: %u]", m_limits.ancestor_count))}; } } } @@ -236,8 +230,8 @@ bool CTxMemPool::CheckPackageLimits(const Package& package, const auto ancestors{CalculateAncestorsAndCheckLimits(total_vsize, package.size(), staged_ancestors, m_limits)}; // It's possible to overestimate the ancestor/descendant totals. - if (!ancestors.has_value()) errString = "possibly " + util::ErrorString(ancestors).original; - return ancestors.has_value(); + if (!ancestors.has_value()) return util::Error{Untranslated("possibly " + util::ErrorString(ancestors).original)}; + return {}; } util::Result<CTxMemPool::setEntries> CTxMemPool::CalculateMemPoolAncestors( diff --git a/src/txmempool.h b/src/txmempool.h index bac7a445d6..9da51756e6 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -605,11 +605,10 @@ public: * to mempool. The transactions need not be direct * ancestors/descendants of each other. * @param[in] total_vsize Sum of virtual sizes for all transactions in package. - * @param[out] errString Populated with error reason if a limit is hit. + * @returns {} or the error reason if a limit is hit. */ - bool CheckPackageLimits(const Package& package, - int64_t total_vsize, - std::string &errString) const EXCLUSIVE_LOCKS_REQUIRED(cs); + util::Result<void> CheckPackageLimits(const Package& package, + int64_t total_vsize) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything diff --git a/src/util/fs.cpp b/src/util/fs.cpp index 14f7a44661..348c1b3383 100644 --- a/src/util/fs.cpp +++ b/src/util/fs.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022 The Bitcoin Core developers +// Copyright (c) 2017-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -18,6 +18,7 @@ #endif #include <cassert> +#include <cerrno> #include <string> namespace fsbridge { @@ -130,4 +131,4 @@ std::string get_filesystem_error_message(const fs::filesystem_error& e) #endif } -} // fsbridge +} // namespace fsbridge diff --git a/src/util/fs.h b/src/util/fs.h index 7e2803b6aa..f841e0d76c 100644 --- a/src/util/fs.h +++ b/src/util/fs.h @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2022 The Bitcoin Core developers +// Copyright (c) 2017-present The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -14,6 +14,8 @@ #include <ios> #include <ostream> #include <string> +#include <system_error> +#include <type_traits> #include <utility> /** Filesystem operations and types */ @@ -35,7 +37,7 @@ public: // Allow path objects arguments for compatibility. path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {} path& operator=(std::filesystem::path path) { std::filesystem::path::operator=(std::move(path)); return *this; } - path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(path); return *this; } + path& operator/=(const std::filesystem::path& path) { std::filesystem::path::operator/=(path); return *this; } // Allow literal string arguments, which are safe as long as the literals are ASCII. path(const char* c) : std::filesystem::path(c) {} @@ -52,12 +54,15 @@ public: // Disallow std::string conversion method to avoid locale-dependent encoding on windows. std::string string() const = delete; - std::string u8string() const + /** + * Return a UTF-8 representation of the path as a std::string, for + * compatibility with code using std::string. For code using the newer + * std::u8string type, it is more efficient to call the inherited + * std::filesystem::path::u8string method instead. + */ + std::string utf8string() const { - const auto& utf8_str{std::filesystem::path::u8string()}; - // utf8_str might either be std::string (C++17) or std::u8string - // (C++20). Convert both to std::string. This method can be removed - // after switching to C++20. + const std::u8string& utf8_str{std::filesystem::path::u8string()}; return std::string{utf8_str.begin(), utf8_str.end()}; } @@ -69,11 +74,7 @@ public: static inline path u8path(const std::string& utf8_str) { -#if __cplusplus < 202002L - return std::filesystem::u8path(utf8_str); -#else return std::filesystem::path(std::u8string{utf8_str.begin(), utf8_str.end()}); -#endif } // Disallow implicit std::string conversion for absolute to avoid @@ -97,9 +98,9 @@ static inline auto quoted(const std::string& s) } // Allow safe path append operations. -static inline path operator/(path p1, path p2) +static inline path operator/(path p1, const path& p2) { - p1 /= std::move(p2); + p1 /= p2; return p1; } static inline path operator/(path p1, const char* p2) @@ -140,7 +141,7 @@ static inline bool copy_file(const path& from, const path& to, copy_options opti * Because \ref PathToString and \ref PathFromString functions don't specify an * encoding, they are meant to be used internally, not externally. They are not * appropriate to use in applications requiring UTF-8, where - * fs::path::u8string() and fs::u8path() methods should be used instead. Other + * fs::path::u8string() / fs::path::utf8string() and fs::u8path() methods should be used instead. Other * applications could require still different encodings. For example, JSON, XML, * or URI applications might prefer to use higher-level escapes (\uXXXX or * &XXXX; or %XX) instead of multibyte encoding. Rust, Python, Java applications @@ -154,13 +155,13 @@ static inline std::string PathToString(const path& path) // use here, because these methods encode the path using C++'s narrow // multibyte encoding, which on Windows corresponds to the current "code // page", which is unpredictable and typically not able to represent all - // valid paths. So fs::path::u8string() and + // valid paths. So fs::path::utf8string() and // fs::u8path() functions are used instead on Windows. On - // POSIX, u8string/u8path functions are not safe to use because paths are + // POSIX, u8string/utf8string/u8path functions are not safe to use because paths are // not always valid UTF-8, so plain string methods which do not transform // the path there are used. #ifdef WIN32 - return path.u8string(); + return path.utf8string(); #else static_assert(std::is_same<path::string_type, std::string>::value, "PathToString not implemented on this platform"); return path.std::filesystem::path::string(); diff --git a/src/util/signalinterrupt.cpp b/src/util/signalinterrupt.cpp index c551ba8044..f36f3dbf9e 100644 --- a/src/util/signalinterrupt.cpp +++ b/src/util/signalinterrupt.cpp @@ -30,15 +30,16 @@ SignalInterrupt::operator bool() const return m_flag; } -void SignalInterrupt::reset() +bool SignalInterrupt::reset() { // Cancel existing interrupt by waiting for it, this will reset condition flags and remove // the token from the pipe. - if (*this) wait(); + if (*this && !wait()) return false; m_flag = false; + return true; } -void SignalInterrupt::operator()() +bool SignalInterrupt::operator()() { #ifdef WIN32 std::unique_lock<std::mutex> lk(m_mutex); @@ -52,13 +53,14 @@ void SignalInterrupt::operator()() // Write an arbitrary byte to the write end of the pipe. int res = m_pipe_w.TokenWrite('x'); if (res != 0) { - throw std::ios_base::failure("Could not write interrupt token"); + return false; } } #endif + return true; } -void SignalInterrupt::wait() +bool SignalInterrupt::wait() { #ifdef WIN32 std::unique_lock<std::mutex> lk(m_mutex); @@ -66,9 +68,10 @@ void SignalInterrupt::wait() #else int res = m_pipe_r.TokenRead(); if (res != 'x') { - throw std::ios_base::failure("Did not read expected interrupt token"); + return false; } #endif + return true; } } // namespace util diff --git a/src/util/signalinterrupt.h b/src/util/signalinterrupt.h index ca02feda91..605d124206 100644 --- a/src/util/signalinterrupt.h +++ b/src/util/signalinterrupt.h @@ -30,9 +30,9 @@ class SignalInterrupt public: SignalInterrupt(); explicit operator bool() const; - void operator()(); - void reset(); - void wait(); + [[nodiscard]] bool operator()(); + [[nodiscard]] bool reset(); + [[nodiscard]] bool wait(); private: std::atomic<bool> m_flag; diff --git a/src/validation.cpp b/src/validation.cpp index 0501499004..0f3d5d1454 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -51,6 +51,7 @@ #include <util/hasher.h> #include <util/moneystr.h> #include <util/rbf.h> +#include <util/result.h> #include <util/signalinterrupt.h> #include <util/strencodings.h> #include <util/time.h> @@ -1024,10 +1025,10 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector<CTransactionRef>& txn assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx) { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); - std::string err_string; - if (!m_pool.CheckPackageLimits(txns, total_vsize, err_string)) { + auto result = m_pool.CheckPackageLimits(txns, total_vsize); + if (!result) { // This is a package-wide error, separate from an individual transaction error. - return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", err_string); + return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package-mempool-limits", util::ErrorString(result).original); } return true; } diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index c6ed0fe11c..6a8453965b 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -316,7 +316,7 @@ Result CreateRateBumpTransaction(CWallet& wallet, const uint256& txid, const CCo // We cannot source new unconfirmed inputs(bip125 rule 2) new_coin_control.m_min_depth = 1; - auto res = CreateTransaction(wallet, recipients, std::nullopt, new_coin_control, false); + auto res = CreateTransaction(wallet, recipients, /*change_pos=*/std::nullopt, new_coin_control, false); if (!res) { errors.push_back(Untranslated("Unable to create transaction.") + Untranslated(" ") + util::ErrorString(res)); return Result::WALLET_ERROR; diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 3e1ec667e0..99c548bf1d 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -734,7 +734,7 @@ RPCHelpMan dumpwallet() * It may also avoid other security issues. */ if (fs::exists(filepath)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.u8string() + " already exists. If you are sure this is what you want, move it out of the way first"); + throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.utf8string() + " already exists. If you are sure this is what you want, move it out of the way first"); } std::ofstream file; @@ -828,7 +828,7 @@ RPCHelpMan dumpwallet() file.close(); UniValue reply(UniValue::VOBJ); - reply.pushKV("filename", filepath.u8string()); + reply.pushKV("filename", filepath.utf8string()); return reply; }, diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index b121c8e1a7..5a13b5ac8e 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -155,7 +155,7 @@ UniValue SendMoney(CWallet& wallet, const CCoinControl &coin_control, std::vecto std::shuffle(recipients.begin(), recipients.end(), FastRandomContext()); // Send - auto res = CreateTransaction(wallet, recipients, std::nullopt, coin_control, true); + auto res = CreateTransaction(wallet, recipients, /*change_pos=*/std::nullopt, coin_control, true); if (!res) { throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, util::ErrorString(res).original); } diff --git a/src/wallet/rpc/wallet.cpp b/src/wallet/rpc/wallet.cpp index 164ce9afed..391153a2a1 100644 --- a/src/wallet/rpc/wallet.cpp +++ b/src/wallet/rpc/wallet.cpp @@ -69,6 +69,7 @@ static RPCHelpMan getwalletinfo() {RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"}, {RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"}, {RPCResult::Type::BOOL, "blank", "Whether this wallet intentionally does not contain any keys, scripts, or descriptors"}, + {RPCResult::Type::NUM_TIME, "birthtime", /*optional=*/true, "The start time for blocks scanning. It could be modified by (re)importing any descriptor with an earlier timestamp."}, RESULT_LAST_PROCESSED_BLOCK, }}, }, @@ -132,6 +133,9 @@ static RPCHelpMan getwalletinfo() obj.pushKV("descriptors", pwallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); obj.pushKV("external_signer", pwallet->IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER)); obj.pushKV("blank", pwallet->IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET)); + if (int64_t birthtime = pwallet->GetBirthTime(); birthtime != UNKNOWN_TIME) { + obj.pushKV("birthtime", birthtime); + } AppendLastProcessedBlock(obj, *pwallet); return obj; @@ -165,7 +169,7 @@ static RPCHelpMan listwalletdir() UniValue wallets(UniValue::VARR); for (const auto& path : ListDatabases(GetWalletDir())) { UniValue wallet(UniValue::VOBJ); - wallet.pushKV("name", path.u8string()); + wallet.pushKV("name", path.utf8string()); wallets.push_back(wallet); } @@ -802,7 +806,7 @@ static RPCHelpMan migratewallet() if (res->solvables_wallet) { r.pushKV("solvables_name", res->solvables_wallet->GetName()); } - r.pushKV("backup_path", res->backup_path.u8string()); + r.pushKV("backup_path", res->backup_path.utf8string()); return r; }, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index 0b4800b848..70705667b0 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -710,7 +710,7 @@ void LegacyScriptPubKeyMan::UpdateTimeFirstKey(int64_t nCreateTime) // Cannot determine birthday information, so set the wallet birthday to // the beginning of time. nTimeFirstKey = 1; - } else if (!nTimeFirstKey || nCreateTime < nTimeFirstKey) { + } else if (nTimeFirstKey == UNKNOWN_TIME || nCreateTime < nTimeFirstKey) { nTimeFirstKey = nCreateTime; } diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index dccbf3ced6..449a75eb6b 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -51,6 +51,9 @@ public: virtual bool IsLocked() const = 0; }; +//! Constant representing an unknown spkm creation time +static constexpr int64_t UNKNOWN_TIME = std::numeric_limits<int64_t>::max(); + //! Default for -keypool static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; @@ -286,7 +289,8 @@ private: WatchOnlySet setWatchOnly GUARDED_BY(cs_KeyStore); WatchKeyMap mapWatchKeys GUARDED_BY(cs_KeyStore); - int64_t nTimeFirstKey GUARDED_BY(cs_KeyStore) = 0; + // By default, do not scan any block until keys/scripts are generated/imported + int64_t nTimeFirstKey GUARDED_BY(cs_KeyStore) = UNKNOWN_TIME; //! Number of pre-generated keys/scripts (part of the look-ahead process, used to detect payments) int64_t m_keypool_size GUARDED_BY(cs_KeyStore){DEFAULT_KEYPOOL_SIZE}; diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp index e97e658f38..b51cd6332f 100644 --- a/src/wallet/spend.cpp +++ b/src/wallet/spend.cpp @@ -1296,8 +1296,9 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( if (gArgs.GetBoolArg("-walletrejectlongchains", DEFAULT_WALLET_REJECT_LONG_CHAINS)) { // Lastly, ensure this tx will pass the mempool's chain limits - if (!wallet.chain().checkChainLimits(tx)) { - return util::Error{_("Transaction has too long of a mempool chain")}; + auto result = wallet.chain().checkChainLimits(tx); + if (!result) { + return util::Error{util::ErrorString(result)}; } } diff --git a/src/wallet/spend.h b/src/wallet/spend.h index 504c078b80..3bd37cfd0e 100644 --- a/src/wallet/spend.h +++ b/src/wallet/spend.h @@ -216,7 +216,7 @@ struct CreatedTransactionResult /** * Create a new transaction paying the recipients with a set of coins * selected by SelectCoins(); Also create the change output, when needed - * @note passing change_pos as -1 will result in setting a random position + * @note passing change_pos as std::nullopt will result in setting a random position */ util::Result<CreatedTransactionResult> CreateTransaction(CWallet& wallet, const std::vector<CRecipient>& vecSend, std::optional<unsigned int> change_pos, const CCoinControl& coin_control, bool sign = true); diff --git a/src/wallet/test/spend_tests.cpp b/src/wallet/test/spend_tests.cpp index b2d252b3f9..3509bc116f 100644 --- a/src/wallet/test/spend_tests.cpp +++ b/src/wallet/test/spend_tests.cpp @@ -33,7 +33,7 @@ BOOST_FIXTURE_TEST_CASE(SubtractFee, TestChain100Setup) coin_control.fOverrideFeeRate = true; // We need to use a change type with high cost of change so that the leftover amount will be dropped to fee instead of added as a change output coin_control.m_change_type = OutputType::LEGACY; - auto res = CreateTransaction(*wallet, {recipient}, std::nullopt, coin_control); + auto res = CreateTransaction(*wallet, {recipient}, /*change_pos=*/std::nullopt, coin_control); BOOST_CHECK(res); const auto& txr = *res; BOOST_CHECK_EQUAL(txr.tx->vout.size(), 1); @@ -97,12 +97,12 @@ BOOST_FIXTURE_TEST_CASE(wallet_duplicated_preset_inputs_test, TestChain100Setup) // so that the recipient's amount is no longer equal to the user's selected target of 299 BTC. // First case, use 'subtract_fee_from_outputs=true' - util::Result<CreatedTransactionResult> res_tx = CreateTransaction(*wallet, recipients, /*change_pos*/-1, coin_control); + util::Result<CreatedTransactionResult> res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control); BOOST_CHECK(!res_tx.has_value()); // Second case, don't use 'subtract_fee_from_outputs'. recipients[0].fSubtractFeeFromAmount = false; - res_tx = CreateTransaction(*wallet, recipients, /*change_pos*/-1, coin_control); + res_tx = CreateTransaction(*wallet, recipients, /*change_pos=*/std::nullopt, coin_control); BOOST_CHECK(!res_tx.has_value()); } diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 3d1cbe36a8..1c5ca1483f 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -558,7 +558,7 @@ public: CTransactionRef tx; CCoinControl dummy; { - auto res = CreateTransaction(*wallet, {recipient}, std::nullopt, dummy); + auto res = CreateTransaction(*wallet, {recipient}, /*change_pos=*/std::nullopt, dummy); BOOST_CHECK(res); tx = res->tx; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 892b9d51e4..507174413d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1080,6 +1080,9 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); wtx.nTimeSmart = ComputeTimeSmart(wtx, rescanning_old_block); AddToSpends(wtx, &batch); + + // Update birth time when tx time is older than it. + MaybeUpdateBirthTime(wtx.GetTxTime()); } if (!fInsertedNew) @@ -1199,6 +1202,10 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx } } } + + // Update birth time when tx time is older than it. + MaybeUpdateBirthTime(wtx.GetTxTime()); + return true; } @@ -1747,11 +1754,11 @@ bool CWallet::ImportScriptPubKeys(const std::string& label, const std::set<CScri return true; } -void CWallet::FirstKeyTimeChanged(const ScriptPubKeyMan* spkm, int64_t new_birth_time) +void CWallet::MaybeUpdateBirthTime(int64_t time) { int64_t birthtime = m_birth_time.load(); - if (new_birth_time < birthtime) { - m_birth_time = new_birth_time; + if (time < birthtime) { + m_birth_time = time; } } @@ -3089,7 +3096,7 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri int64_t time = spk_man->GetTimeFirstKey(); if (!time_first_key || time < *time_first_key) time_first_key = time; } - if (time_first_key) walletInstance->m_birth_time = *time_first_key; + if (time_first_key) walletInstance->MaybeUpdateBirthTime(*time_first_key); if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) { return nullptr; @@ -3482,10 +3489,12 @@ LegacyScriptPubKeyMan* CWallet::GetOrCreateLegacyScriptPubKeyMan() void CWallet::AddScriptPubKeyMan(const uint256& id, std::unique_ptr<ScriptPubKeyMan> spkm_man) { + // Add spkm_man to m_spk_managers before calling any method + // that might access it. const auto& spkm = m_spk_managers[id] = std::move(spkm_man); // Update birth time if needed - FirstKeyTimeChanged(spkm.get(), spkm->GetTimeFirstKey()); + MaybeUpdateBirthTime(spkm->GetTimeFirstKey()); } void CWallet::SetupLegacyScriptPubKeyMan() @@ -3518,7 +3527,7 @@ void CWallet::ConnectScriptPubKeyManNotifiers() for (const auto& spk_man : GetActiveScriptPubKeyMans()) { spk_man->NotifyWatchonlyChanged.connect(NotifyWatchonlyChanged); spk_man->NotifyCanGetAddressesChanged.connect(NotifyCanGetAddressesChanged); - spk_man->NotifyFirstKeyTimeChanged.connect(std::bind(&CWallet::FirstKeyTimeChanged, this, std::placeholders::_1, std::placeholders::_2)); + spk_man->NotifyFirstKeyTimeChanged.connect(std::bind(&CWallet::MaybeUpdateBirthTime, this, std::placeholders::_2)); } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 11b7964316..487921443f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -688,8 +688,8 @@ public: bool ImportPubKeys(const std::vector<CKeyID>& ordered_pubkeys, const std::map<CKeyID, CPubKey>& pubkey_map, const std::map<CKeyID, std::pair<CPubKey, KeyOriginInfo>>& key_origins, const bool add_keypool, const bool internal, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool ImportScriptPubKeys(const std::string& label, const std::set<CScript>& script_pub_keys, const bool have_solving_data, const bool apply_label, const int64_t timestamp) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); - /** Updates wallet birth time if 'new_birth_time' is below it */ - void FirstKeyTimeChanged(const ScriptPubKeyMan* spkm, int64_t new_birth_time); + /** Updates wallet birth time if 'time' is below it */ + void MaybeUpdateBirthTime(int64_t time); CFeeRate m_pay_tx_fee{DEFAULT_PAY_TX_FEE}; unsigned int m_confirm_target{DEFAULT_TX_CONFIRM_TARGET}; @@ -887,6 +887,9 @@ public: /* Returns true if the wallet can give out new addresses. This means it has keys in the keypool or can generate new keys */ bool CanGetAddresses(bool internal = false) const; + /* Returns the time of the first created key or, in case of an import, it could be the time of the first received transaction */ + int64_t GetBirthTime() const { return m_birth_time; } + /** * Blocks until the wallet state is up-to-date to /at least/ the current * chain at the time this function is entered |