From 413f4bb52b72e082ad8716664ede48352b8e7e5a Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 12 Jul 2022 15:54:11 -0400 Subject: [PATCH] DumpMempool: Pass in dump_path, stop using gArgs Also introduce node::{ShouldPersistMempool,MempoolPath} helper functions in node/mempool_persist_args.{h,cpp} which are used by non-kernel DumpMempool callers to determine whether or not to automatically dump the mempool and where to dump it to. --- src/Makefile.am | 2 ++ src/init.cpp | 7 +++++-- src/node/mempool_persist_args.cpp | 23 +++++++++++++++++++++++ src/node/mempool_persist_args.h | 19 +++++++++++++++++++ src/rpc/mempool.cpp | 9 +++++++-- src/test/fuzz/validation_load_mempool.cpp | 5 ++++- src/validation.cpp | 6 +++--- src/validation.h | 2 +- 8 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 src/node/mempool_persist_args.cpp create mode 100644 src/node/mempool_persist_args.h diff --git a/src/Makefile.am b/src/Makefile.am index a9e9db0a7d4..1ba76ac5e79 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -198,6 +198,7 @@ BITCOIN_CORE_H = \ node/chainstate.h \ node/coin.h \ node/context.h \ + node/mempool_persist_args.h \ node/miner.h \ node/minisketchwrapper.h \ node/psbt.h \ @@ -380,6 +381,7 @@ libbitcoin_node_a_SOURCES = \ node/context.cpp \ node/eviction.cpp \ node/interfaces.cpp \ + node/mempool_persist_args.cpp \ node/miner.cpp \ node/minisketchwrapper.cpp \ node/psbt.cpp \ diff --git a/src/init.cpp b/src/init.cpp index eff37e1a83e..fc068cf25e6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include @@ -111,6 +112,8 @@ using node::CleanupBlockRevFiles; using node::DEFAULT_PRINTPRIORITY; using node::DEFAULT_STOPAFTERBLOCKIMPORT; using node::LoadChainstate; +using node::MempoolPath; +using node::ShouldPersistMempool; using node::NodeContext; using node::ThreadImport; using node::VerifyLoadedChainstate; @@ -246,8 +249,8 @@ void Shutdown(NodeContext& node) node.addrman.reset(); node.netgroupman.reset(); - if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - DumpMempool(*node.mempool); + if (node.mempool && node.mempool->IsLoaded() && ShouldPersistMempool(*node.args)) { + DumpMempool(*node.mempool, MempoolPath(*node.args)); } // Drop transactions we were still watching, and record fee estimations. diff --git a/src/node/mempool_persist_args.cpp b/src/node/mempool_persist_args.cpp new file mode 100644 index 00000000000..4e775869c64 --- /dev/null +++ b/src/node/mempool_persist_args.cpp @@ -0,0 +1,23 @@ +// Copyright (c) 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 + +#include +#include +#include + +namespace node { + +bool ShouldPersistMempool(const ArgsManager& argsman) +{ + return argsman.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL); +} + +fs::path MempoolPath(const ArgsManager& argsman) +{ + return argsman.GetDataDirNet() / "mempool.dat"; +} + +} // namespace node diff --git a/src/node/mempool_persist_args.h b/src/node/mempool_persist_args.h new file mode 100644 index 00000000000..f2bfd2b748e --- /dev/null +++ b/src/node/mempool_persist_args.h @@ -0,0 +1,19 @@ +// Copyright (c) 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. + +#ifndef BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H +#define BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H + +#include + +class ArgsManager; + +namespace node { + +bool ShouldPersistMempool(const ArgsManager& argsman); +fs::path MempoolPath(const ArgsManager& argsman); + +} // namespace node + +#endif // BITCOIN_NODE_MEMPOOL_PERSIST_ARGS_H diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 84d43e78188..bab358d50a8 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -19,6 +20,8 @@ #include using node::DEFAULT_MAX_RAW_TX_FEE_RATE; +using node::MempoolPath; +using node::ShouldPersistMempool; using node::NodeContext; static RPCHelpMan sendrawtransaction() @@ -721,12 +724,14 @@ static RPCHelpMan savemempool() throw JSONRPCError(RPC_MISC_ERROR, "The mempool was not loaded yet"); } - if (!DumpMempool(mempool)) { + const fs::path& dump_path = MempoolPath(args); + + if (!DumpMempool(mempool, dump_path)) { throw JSONRPCError(RPC_MISC_ERROR, "Unable to dump mempool to disk"); } UniValue ret(UniValue::VOBJ); - ret.pushKV("filename", fs::path((args.GetDataDirNet() / "mempool.dat")).u8string()); + ret.pushKV("filename", dump_path.u8string()); return ret; }, diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 9532610f8da..d6afc0e4e31 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -15,6 +16,8 @@ #include #include +using node::MempoolPath; + namespace { const TestingSetup* g_setup; } // namespace @@ -37,5 +40,5 @@ FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool) return fuzzed_file_provider.open(); }; (void)LoadMempool(pool, g_setup->m_node.chainman->ActiveChainstate(), fuzzed_fopen); - (void)DumpMempool(pool, fuzzed_fopen, true); + (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true); } diff --git a/src/validation.cpp b/src/validation.cpp index 48535d32a91..4d02fbef534 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4726,7 +4726,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka return true; } -bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool skip_file_commit) +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function, bool skip_file_commit) { auto start = SteadyClock::now(); @@ -4749,7 +4749,7 @@ bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool s auto mid = SteadyClock::now(); try { - FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat.new", "wb")}; + FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")}; if (!filestr) { return false; } @@ -4775,7 +4775,7 @@ bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool s if (!skip_file_commit && !FileCommit(file.Get())) throw std::runtime_error("FileCommit failed"); file.fclose(); - if (!RenameOver(gArgs.GetDataDirNet() / "mempool.dat.new", gArgs.GetDataDirNet() / "mempool.dat")) { + if (!RenameOver(dump_path + ".new", dump_path)) { throw std::runtime_error("Rename failed"); } auto last = SteadyClock::now(); diff --git a/src/validation.h b/src/validation.h index 0e27e117fa5..a1901cd782a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1017,7 +1017,7 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) using FopenFn = std::function; /** Dump the mempool to disk. */ -bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); /** Load the mempool from disk. */ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function = fsbridge::fopen);