From c84390b741ab7b61c9f702d8b447c8cadc1257c8 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 7 Jul 2022 19:30:30 -0400 Subject: [PATCH 01/11] test/mempool_persist: Test manual savemempool when -persistmempool=0 --- test/functional/mempool_persist.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index 58f4c913434..b6fa7fbd919 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -141,6 +141,16 @@ class MempoolPersistTest(BitcoinTestFramework): self.nodes[2].syncwithvalidationinterfacequeue() # Flush mempool to wallet assert_equal(node2_balance, wallet_watch.getbalance()) + mempooldat0 = os.path.join(self.nodes[0].datadir, self.chain, 'mempool.dat') + mempooldat1 = os.path.join(self.nodes[1].datadir, self.chain, 'mempool.dat') + + self.log.debug("Force -persistmempool=0 node1 to savemempool to disk via RPC") + assert not os.path.exists(mempooldat1) + result1 = self.nodes[1].savemempool() + assert os.path.isfile(mempooldat1) + assert_equal(result1['filename'], mempooldat1) + os.remove(mempooldat1) + self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") self.stop_nodes() self.start_node(0, extra_args=["-persistmempool=0"]) @@ -153,8 +163,6 @@ class MempoolPersistTest(BitcoinTestFramework): assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 7) - mempooldat0 = os.path.join(self.nodes[0].datadir, self.chain, 'mempool.dat') - mempooldat1 = os.path.join(self.nodes[1].datadir, self.chain, 'mempool.dat') self.log.debug("Remove the mempool.dat file. Verify that savemempool to disk via RPC re-creates it") os.remove(mempooldat0) result0 = self.nodes[0].savemempool() From bd4407817e523e3c5b347bc6be25ed007cb27034 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Mon, 16 May 2022 14:09:48 -0400 Subject: [PATCH 02/11] DumpMempool: Use std::chrono instead of weird int64_t arthmetics This makes it so that DumpMempool doesn't depend on MICRO anymore --- src/util/time.h | 1 + src/validation.cpp | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/util/time.h b/src/util/time.h index 9df69a953c6..fc49f23ce31 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -24,6 +24,7 @@ struct NodeClock : public std::chrono::system_clock { }; using NodeSeconds = std::chrono::time_point; +using SteadyClock = std::chrono::steady_clock; using SteadySeconds = std::chrono::time_point; using SteadyMilliseconds = std::chrono::time_point; using SteadyMicroseconds = std::chrono::time_point; diff --git a/src/validation.cpp b/src/validation.cpp index 4c694a2c21c..48535d32a91 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -47,12 +47,14 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -4726,7 +4728,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool skip_file_commit) { - int64_t start = GetTimeMicros(); + auto start = SteadyClock::now(); std::map mapDeltas; std::vector vinfo; @@ -4744,7 +4746,7 @@ bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool s unbroadcast_txids = pool.GetUnbroadcastTxs(); } - int64_t mid = GetTimeMicros(); + auto mid = SteadyClock::now(); try { FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat.new", "wb")}; @@ -4776,8 +4778,11 @@ bool DumpMempool(const CTxMemPool& pool, FopenFn mockable_fopen_function, bool s if (!RenameOver(gArgs.GetDataDirNet() / "mempool.dat.new", gArgs.GetDataDirNet() / "mempool.dat")) { throw std::runtime_error("Rename failed"); } - int64_t last = GetTimeMicros(); - LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", (mid-start)*MICRO, (last-mid)*MICRO); + auto last = SteadyClock::now(); + + LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", + Ticks(mid - start), + Ticks(last - mid)); } catch (const std::exception& e) { LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); return false; From 413f4bb52b72e082ad8716664ede48352b8e7e5a Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 12 Jul 2022 15:54:11 -0400 Subject: [PATCH 03/11] 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); From 813962da0b17b918941c6849996845e35d84a451 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Thu, 7 Jul 2022 16:32:52 -0400 Subject: [PATCH 04/11] scripted-diff: Rename m_is_loaded -> m_load_tried m_is_loaded/IsLoaded() doesn't actually indicate whether or not the mempool was successfully, loaded, but rather if a load has been attempted and did not result in a catastrophic ShutdownRequested. -BEGIN VERIFY SCRIPT- find_regex="\bm_is_loaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@m_load_tried@g" find_regex="\bIsLoaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@GetLoadTried@g" find_regex="\bSetIsLoaded\b" \ && git grep -l -E "$find_regex" \ | xargs sed -i -E "s@$find_regex@SetLoadTried@g" -END VERIFY SCRIPT- --- src/init.cpp | 2 +- src/rpc/mempool.cpp | 4 ++-- src/txmempool.cpp | 8 ++++---- src/txmempool.h | 6 +++--- src/validation.cpp | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index fc068cf25e6..62eff218cb9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -249,7 +249,7 @@ void Shutdown(NodeContext& node) node.addrman.reset(); node.netgroupman.reset(); - if (node.mempool && node.mempool->IsLoaded() && ShouldPersistMempool(*node.args)) { + if (node.mempool && node.mempool->GetLoadTried() && ShouldPersistMempool(*node.args)) { DumpMempool(*node.mempool, MempoolPath(*node.args)); } diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index bab358d50a8..7f4ff47941c 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -656,7 +656,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool) // Make sure this call is atomic in the pool. LOCK(pool.cs); UniValue ret(UniValue::VOBJ); - ret.pushKV("loaded", pool.IsLoaded()); + ret.pushKV("loaded", pool.GetLoadTried()); ret.pushKV("size", (int64_t)pool.size()); ret.pushKV("bytes", (int64_t)pool.GetTotalTxSize()); ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage()); @@ -720,7 +720,7 @@ static RPCHelpMan savemempool() const ArgsManager& args{EnsureAnyArgsman(request.context)}; const CTxMemPool& mempool = EnsureAnyMemPool(request.context); - if (!mempool.IsLoaded()) { + if (!mempool.GetLoadTried()) { throw JSONRPCError(RPC_MISC_ERROR, "The mempool was not loaded yet"); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index aeaa10034eb..00e30689461 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1210,14 +1210,14 @@ void CTxMemPool::GetTransactionAncestry(const uint256& txid, size_t& ancestors, } } -bool CTxMemPool::IsLoaded() const +bool CTxMemPool::GetLoadTried() const { LOCK(cs); - return m_is_loaded; + return m_load_tried; } -void CTxMemPool::SetIsLoaded(bool loaded) +void CTxMemPool::SetLoadTried(bool loaded) { LOCK(cs); - m_is_loaded = loaded; + m_load_tried = loaded; } diff --git a/src/txmempool.h b/src/txmempool.h index 6e37f59f2e9..1abf641ed2e 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -451,7 +451,7 @@ protected: void trackPackageRemoved(const CFeeRate& rate) EXCLUSIVE_LOCKS_REQUIRED(cs); - bool m_is_loaded GUARDED_BY(cs){false}; + bool m_load_tried GUARDED_BY(cs){false}; CFeeRate GetMinFee(size_t sizelimit) const; @@ -729,10 +729,10 @@ public: void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const; /** @returns true if the mempool is fully loaded */ - bool IsLoaded() const; + bool GetLoadTried() const; /** Sets the current loaded state */ - void SetIsLoaded(bool loaded); + void SetLoadTried(bool loaded); unsigned long size() const { diff --git a/src/validation.cpp b/src/validation.cpp index 4d02fbef534..5c7bfc510c2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3869,7 +3869,7 @@ void CChainState::LoadMempool(const ArgsManager& args) if (args.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { ::LoadMempool(*m_mempool, *this); } - m_mempool->SetIsLoaded(!ShutdownRequested()); + m_mempool->SetLoadTried(!ShutdownRequested()); } bool CChainState::LoadChainTip() From f9e8e5719f28d84f68f7d75e26c8e7fccac8e7d3 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Wed, 13 Jul 2022 21:24:03 -0400 Subject: [PATCH 05/11] mempool: Improve comments for [GS]etLoadTried Also change the param name for SetLoadTried to load_tried. --- src/txmempool.cpp | 4 ++-- src/txmempool.h | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/txmempool.cpp b/src/txmempool.cpp index 00e30689461..7eff6bdbe32 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1216,8 +1216,8 @@ bool CTxMemPool::GetLoadTried() const return m_load_tried; } -void CTxMemPool::SetLoadTried(bool loaded) +void CTxMemPool::SetLoadTried(bool load_tried) { LOCK(cs); - m_load_tried = loaded; + m_load_tried = load_tried; } diff --git a/src/txmempool.h b/src/txmempool.h index 1abf641ed2e..d7d308038c2 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -728,11 +728,17 @@ public: */ void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const; - /** @returns true if the mempool is fully loaded */ + /** + * @returns true if we've made an attempt to load the mempool regardless of + * whether the attempt was successful or not + */ bool GetLoadTried() const; - /** Sets the current loaded state */ - void SetLoadTried(bool loaded); + /** + * Set whether or not we've made an attempt to load the mempool (regardless + * of whether the attempt was successful or not) + */ + void SetLoadTried(bool load_tried); unsigned long size() const { From ae1e8e37567fa603a5977d7d05105c682dd3f7db Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Wed, 29 Jun 2022 13:28:45 -0400 Subject: [PATCH 06/11] mempool: Use NodeClock+friends for LoadMempool --- src/validation.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 5c7bfc510c2..1d24f14dd8b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4644,7 +4644,6 @@ static const uint64_t MEMPOOL_DUMP_VERSION = 1; bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function) { - int64_t nExpiryTimeout = std::chrono::seconds{pool.m_expiry}.count(); FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat", "rb")}; CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); if (file.IsNull()) { @@ -4657,7 +4656,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka int64_t failed = 0; int64_t already_there = 0; int64_t unbroadcast = 0; - int64_t nNow = GetTime(); + auto now = NodeClock::now(); try { uint64_t version; @@ -4680,7 +4679,7 @@ bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mocka if (amountdelta) { pool.PrioritiseTransaction(tx->GetHash(), amountdelta); } - if (nTime > nNow - nExpiryTimeout) { + if (nTime > TicksSinceEpoch(now - pool.m_expiry)) { LOCK(cs_main); const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { From b3267258b052557fc136b9a4dcb754afb9219470 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Wed, 13 Jul 2022 12:32:43 -0400 Subject: [PATCH 07/11] Move FopenFn to fsbridge namespace [META] In a future commit in this patchset, it will be used by more than just validation, and it needs to align with fopen anyway. --- src/fs.h | 2 ++ src/validation.cpp | 2 ++ src/validation.h | 6 ++---- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/fs.h b/src/fs.h index cc55793b951..e8b34319bbe 100644 --- a/src/fs.h +++ b/src/fs.h @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -199,6 +200,7 @@ bool create_directories(const std::filesystem::path& p, std::error_code& ec) = d /** Bridge operations to C stdio */ namespace fsbridge { + using FopenFn = std::function; FILE *fopen(const fs::path& p, const char *mode); /** diff --git a/src/validation.cpp b/src/validation.cpp index 1d24f14dd8b..b757fed055a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -64,6 +65,7 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; using kernel::ComputeUTXOStats; +using fsbridge::FopenFn; using node::BLOCKFILE_CHUNK_SIZE; using node::BlockManager; using node::BlockMap; diff --git a/src/validation.h b/src/validation.h index a1901cd782a..711fc746e20 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1014,13 +1014,11 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) return DeploymentEnabled(chainman.GetConsensus(), dep); } -using FopenFn = std::function; - /** Dump the mempool to disk. */ -bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, fsbridge::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); +bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); /** * Return the expected assumeutxo value for a given height, if one exists. From b857ac60d9a0433036519c26675378bbf56a1de1 Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 17 May 2022 14:45:18 -0400 Subject: [PATCH 08/11] test/fuzz: Invoke LoadMempool via CChainState Not only does this increase coverage, it is also more correct in that when ::LoadMempool is called with a mempool and chainstate, it calls AcceptToMemoryPool with just the chainstate. AcceptToMemoryPool will then act on the chainstate's mempool via CChainState::GetMempool, which may be different from the mempool originally passed to ::LoadMempool. (In this fuzz test's case, it definitely is different) Also, move DummyChainstate to its own file since it's now used by the validation_load_mempool fuzz test to replace CChainState's m_mempool. --- src/Makefile.test_fuzz.include | 1 + src/test/fuzz/mempool_utils.h | 19 +++++++++++++++++++ src/test/fuzz/tx_pool.cpp | 10 +--------- src/test/fuzz/validation_load_mempool.cpp | 6 +++++- src/validation.cpp | 4 ++-- src/validation.h | 2 +- 6 files changed, 29 insertions(+), 13 deletions(-) create mode 100644 src/test/fuzz/mempool_utils.h diff --git a/src/Makefile.test_fuzz.include b/src/Makefile.test_fuzz.include index 8922dda3ad4..b43816636f9 100644 --- a/src/Makefile.test_fuzz.include +++ b/src/Makefile.test_fuzz.include @@ -10,6 +10,7 @@ EXTRA_LIBRARIES += \ TEST_FUZZ_H = \ test/fuzz/fuzz.h \ test/fuzz/FuzzedDataProvider.h \ + test/fuzz/mempool_utils.h \ test/fuzz/util.h libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(MINIUPNPC_CPPFLAGS) $(NATPMP_CPPFLAGS) $(EVENT_CFLAGS) $(EVENT_PTHREADS_CFLAGS) diff --git a/src/test/fuzz/mempool_utils.h b/src/test/fuzz/mempool_utils.h new file mode 100644 index 00000000000..bfe12e30ba0 --- /dev/null +++ b/src/test/fuzz/mempool_utils.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_TEST_FUZZ_MEMPOOL_UTILS_H +#define BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H + +#include + +class DummyChainState final : public CChainState +{ +public: + void SetMempool(CTxMemPool* mempool) + { + m_mempool = mempool; + } +}; + +#endif // BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index 2d88ee295bb..63fbf0516a2 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -34,15 +35,6 @@ struct MockedTxPool : public CTxMemPool { } }; -class DummyChainState final : public CChainState -{ -public: - void SetMempool(CTxMemPool* mempool) - { - m_mempool = mempool; - } -}; - void initialize_tx_pool() { static const auto testing_setup = MakeNoLogFileContext(); diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index d6afc0e4e31..20947e36388 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -36,9 +37,12 @@ FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool) CTxMemPool pool{MemPoolOptionsForTest(g_setup->m_node)}; + auto& chainstate{static_cast(g_setup->m_node.chainman->ActiveChainstate())}; + chainstate.SetMempool(&pool); + auto fuzzed_fopen = [&](const fs::path&, const char*) { return fuzzed_file_provider.open(); }; - (void)LoadMempool(pool, g_setup->m_node.chainman->ActiveChainstate(), fuzzed_fopen); + (void)chainstate.LoadMempool(g_setup->m_args, fuzzed_fopen); (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true); } diff --git a/src/validation.cpp b/src/validation.cpp index b757fed055a..f5a678bf85f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3865,11 +3865,11 @@ void PruneBlockFilesManual(CChainState& active_chainstate, int nManualPruneHeigh } } -void CChainState::LoadMempool(const ArgsManager& args) +void CChainState::LoadMempool(const ArgsManager& args, FopenFn mockable_fopen_function) { if (!m_mempool) return; if (args.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - ::LoadMempool(*m_mempool, *this); + ::LoadMempool(*m_mempool, *this, mockable_fopen_function); } m_mempool->SetLoadTried(!ShutdownRequested()); } diff --git a/src/validation.h b/src/validation.h index 711fc746e20..85b7a59b5d2 100644 --- a/src/validation.h +++ b/src/validation.h @@ -679,7 +679,7 @@ public: void CheckBlockIndex(); /** Load the persisted mempool from disk */ - void LoadMempool(const ArgsManager& args); + void LoadMempool(const ArgsManager& args, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */ bool LoadChainTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main); From 06b88ffb8ae7f2b2a93a32908cd80e77fafd270c Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 12 Jul 2022 21:42:00 -0400 Subject: [PATCH 09/11] LoadMempool: Pass in load_path, stop using gArgs Also: 1. Have CChainState::LoadMempool and ::ThreadImport take in paths and pass it through untouched to LoadMempool. 2. Make LoadMempool exit early if the load_path is empty. 3. Adjust the call to ::ThreadImport in ::AppInitMain to correctly pass in an empty path if mempool persistence is disabled. --- src/init.cpp | 2 +- src/node/blockstorage.cpp | 4 ++-- src/node/blockstorage.h | 2 +- src/test/fuzz/validation_load_mempool.cpp | 2 +- src/validation.cpp | 12 ++++++------ src/validation.h | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 62eff218cb9..542b6f105f3 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1673,7 +1673,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) } chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &chainman, &args] { - ThreadImport(chainman, vImportFiles, args); + ThreadImport(chainman, vImportFiles, args, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); }); // Wait for genesis block to be processed diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index cadafcaa8db..103f4f0d7f8 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -823,7 +823,7 @@ struct CImportingNow { } }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const ArgsManager& args) +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const ArgsManager& args, const fs::path& mempool_path) { SetSyscallSandboxPolicy(SyscallSandboxPolicy::INITIALIZATION_LOAD_BLOCKS); ScheduleBatchPriority(); @@ -893,6 +893,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile return; } } // End scope of CImportingNow - chainman.ActiveChainstate().LoadMempool(args); + chainman.ActiveChainstate().LoadMempool(mempool_path); } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index e017f3f4279..9b76371aaec 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -211,7 +211,7 @@ bool ReadRawBlockFromDisk(std::vector& block, const FlatFilePos& pos, c bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex); -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const ArgsManager& args); +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const ArgsManager& args, const fs::path& mempool_path); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 20947e36388..357d8a9987c 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -43,6 +43,6 @@ FUZZ_TARGET_INIT(validation_load_mempool, initialize_validation_load_mempool) auto fuzzed_fopen = [&](const fs::path&, const char*) { return fuzzed_file_provider.open(); }; - (void)chainstate.LoadMempool(g_setup->m_args, fuzzed_fopen); + (void)chainstate.LoadMempool(MempoolPath(g_setup->m_args), fuzzed_fopen); (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true); } diff --git a/src/validation.cpp b/src/validation.cpp index f5a678bf85f..510071de22e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3865,12 +3865,10 @@ void PruneBlockFilesManual(CChainState& active_chainstate, int nManualPruneHeigh } } -void CChainState::LoadMempool(const ArgsManager& args, FopenFn mockable_fopen_function) +void CChainState::LoadMempool(const fs::path& load_path, FopenFn mockable_fopen_function) { if (!m_mempool) return; - if (args.GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - ::LoadMempool(*m_mempool, *this, mockable_fopen_function); - } + ::LoadMempool(*m_mempool, load_path, *this, mockable_fopen_function); m_mempool->SetLoadTried(!ShutdownRequested()); } @@ -4644,9 +4642,11 @@ bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) static const uint64_t MEMPOOL_DUMP_VERSION = 1; -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, FopenFn mockable_fopen_function) +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, FopenFn mockable_fopen_function) { - FILE* filestr{mockable_fopen_function(gArgs.GetDataDirNet() / "mempool.dat", "rb")}; + if (load_path.empty()) return false; + + FILE* filestr{mockable_fopen_function(load_path, "rb")}; CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); if (file.IsNull()) { LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); diff --git a/src/validation.h b/src/validation.h index 85b7a59b5d2..fad94af8e2c 100644 --- a/src/validation.h +++ b/src/validation.h @@ -679,7 +679,7 @@ public: void CheckBlockIndex(); /** Load the persisted mempool from disk */ - void LoadMempool(const ArgsManager& args, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); + void LoadMempool(const fs::path& load_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); /** Update the chain tip based on database information, i.e. CoinsTip()'s best block. */ bool LoadChainTip() EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1018,7 +1018,7 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); /** Load the mempool from disk. */ -bool LoadMempool(CTxMemPool& pool, CChainState& active_chainstate, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); /** * Return the expected assumeutxo value for a given height, if one exists. From aa306765419f7dbea12b12e15553039835ba0e4d Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Wed, 13 Jul 2022 13:42:15 -0400 Subject: [PATCH 10/11] Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel It is no longer used by anything inside libbitcoinkernel, move it to node/mempool_persist_args.h where it belongs. --- src/init.cpp | 1 + src/node/mempool_persist_args.h | 6 ++++++ src/validation.h | 2 -- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 542b6f105f3..c46205d5839 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -109,6 +109,7 @@ using node::CalculateCacheSizes; using node::ChainstateLoadVerifyError; using node::ChainstateLoadingError; using node::CleanupBlockRevFiles; +using node::DEFAULT_PERSIST_MEMPOOL; using node::DEFAULT_PRINTPRIORITY; using node::DEFAULT_STOPAFTERBLOCKIMPORT; using node::LoadChainstate; diff --git a/src/node/mempool_persist_args.h b/src/node/mempool_persist_args.h index f2bfd2b748e..f719ec62ab2 100644 --- a/src/node/mempool_persist_args.h +++ b/src/node/mempool_persist_args.h @@ -11,6 +11,12 @@ class ArgsManager; namespace node { +/** + * Default for -persistmempool, indicating whether the node should attempt to + * automatically load the mempool on start and save to disk on shutdown + */ +static constexpr bool DEFAULT_PERSIST_MEMPOOL{true}; + bool ShouldPersistMempool(const ArgsManager& argsman); fs::path MempoolPath(const ArgsManager& argsman); diff --git a/src/validation.h b/src/validation.h index fad94af8e2c..df4fc77980c 100644 --- a/src/validation.h +++ b/src/validation.h @@ -68,8 +68,6 @@ static const bool DEFAULT_CHECKPOINTS_ENABLED = true; static const bool DEFAULT_TXINDEX = false; static constexpr bool DEFAULT_COINSTATSINDEX{false}; static const char* const DEFAULT_BLOCKFILTERINDEX = "0"; -/** Default for -persistmempool */ -static const bool DEFAULT_PERSIST_MEMPOOL = true; /** Default for -stopatheight */ static const int DEFAULT_STOPATHEIGHT = 0; /** Block files containing a block-height within MIN_BLOCKS_TO_KEEP of ActiveChain().Tip() will not be pruned. */ From cb3e9a1e3f8d72daaa361fc45dd853775e754b9d Mon Sep 17 00:00:00 2001 From: Carl Dong Date: Tue, 12 Jul 2022 22:24:31 -0400 Subject: [PATCH 11/11] Move {Load,Dump}Mempool to kernel namespace Also: 1. Add the newly introduced kernel/mempool_persist.cpp to IWYU CI script 2. Add chrono mapping for iwyu --- ci/test/06_script_b.sh | 1 + contrib/devtools/iwyu/bitcoin.core.imp | 1 + src/Makefile.am | 3 + src/init.cpp | 3 + src/kernel/mempool_persist.cpp | 189 ++++++++++++++++++++++ src/kernel/mempool_persist.h | 28 ++++ src/rpc/mempool.cpp | 4 + src/test/fuzz/validation_load_mempool.cpp | 4 + src/validation.cpp | 156 +----------------- src/validation.h | 6 - test/lint/lint-circular-dependencies.py | 1 + 11 files changed, 238 insertions(+), 158 deletions(-) create mode 100644 src/kernel/mempool_persist.cpp create mode 100644 src/kernel/mempool_persist.h diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index e1032ba6bd6..77358f93d95 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -41,6 +41,7 @@ if [ "${RUN_TIDY}" = "true" ]; then CI_EXEC "python3 ${DIR_IWYU}/include-what-you-use/iwyu_tool.py"\ " src/compat"\ " src/init"\ + " src/kernel/mempool_persist.cpp"\ " src/policy/feerate.cpp"\ " src/policy/packages.cpp"\ " src/policy/settings.cpp"\ diff --git a/contrib/devtools/iwyu/bitcoin.core.imp b/contrib/devtools/iwyu/bitcoin.core.imp index ce7786f58c2..919ffab102d 100644 --- a/contrib/devtools/iwyu/bitcoin.core.imp +++ b/contrib/devtools/iwyu/bitcoin.core.imp @@ -3,4 +3,5 @@ { include: [ "", private, "", public ] }, { include: [ "", private, "", public ] }, { include: [ "", private, "", public ] }, + { include: [ "", private, "", public ] }, ] diff --git a/src/Makefile.am b/src/Makefile.am index 1ba76ac5e79..c2148936af4 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -177,6 +177,7 @@ BITCOIN_CORE_H = \ kernel/context.h \ kernel/mempool_limits.h \ kernel/mempool_options.h \ + kernel/mempool_persist.h \ key.h \ key_io.h \ logging.h \ @@ -368,6 +369,7 @@ libbitcoin_node_a_SOURCES = \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ + kernel/mempool_persist.cpp \ mapport.cpp \ mempool_args.cpp \ net.cpp \ @@ -884,6 +886,7 @@ libbitcoinkernel_la_SOURCES = \ kernel/checks.cpp \ kernel/coinstats.cpp \ kernel/context.cpp \ + kernel/mempool_persist.cpp \ key.cpp \ logging.cpp \ node/blockstorage.cpp \ diff --git a/src/init.cpp b/src/init.cpp index c46205d5839..6cc42c5319e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -104,6 +105,8 @@ #include #endif +using kernel::DumpMempool; + using node::CacheSizes; using node::CalculateCacheSizes; using node::ChainstateLoadVerifyError; diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp new file mode 100644 index 00000000000..1a1cf2bbdc1 --- /dev/null +++ b/src/kernel/mempool_persist.cpp @@ -0,0 +1,189 @@ +// 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 +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using fsbridge::FopenFn; + +namespace kernel { + +static const uint64_t MEMPOOL_DUMP_VERSION = 1; + +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, FopenFn mockable_fopen_function) +{ + if (load_path.empty()) return false; + + FILE* filestr{mockable_fopen_function(load_path, "rb")}; + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + if (file.IsNull()) { + LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); + return false; + } + + int64_t count = 0; + int64_t expired = 0; + int64_t failed = 0; + int64_t already_there = 0; + int64_t unbroadcast = 0; + auto now = NodeClock::now(); + + try { + uint64_t version; + file >> version; + if (version != MEMPOOL_DUMP_VERSION) { + return false; + } + uint64_t num; + file >> num; + while (num) { + --num; + CTransactionRef tx; + int64_t nTime; + int64_t nFeeDelta; + file >> tx; + file >> nTime; + file >> nFeeDelta; + + CAmount amountdelta = nFeeDelta; + if (amountdelta) { + pool.PrioritiseTransaction(tx->GetHash(), amountdelta); + } + if (nTime > TicksSinceEpoch(now - pool.m_expiry)) { + LOCK(cs_main); + const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); + if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { + ++count; + } else { + // mempool may contain the transaction already, e.g. from + // wallet(s) having loaded it while we were processing + // mempool transactions; consider these as valid, instead of + // failed, but mark them as 'already there' + if (pool.exists(GenTxid::Txid(tx->GetHash()))) { + ++already_there; + } else { + ++failed; + } + } + } else { + ++expired; + } + if (ShutdownRequested()) + return false; + } + std::map mapDeltas; + file >> mapDeltas; + + for (const auto& i : mapDeltas) { + pool.PrioritiseTransaction(i.first, i.second); + } + + std::set unbroadcast_txids; + file >> unbroadcast_txids; + unbroadcast = unbroadcast_txids.size(); + for (const auto& txid : unbroadcast_txids) { + // Ensure transactions were accepted to mempool then add to + // unbroadcast set. + if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid); + } + } catch (const std::exception& e) { + LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); + return false; + } + + LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there, %i waiting for initial broadcast\n", count, failed, expired, already_there, unbroadcast); + return true; +} + +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function, bool skip_file_commit) +{ + auto start = SteadyClock::now(); + + std::map mapDeltas; + std::vector vinfo; + std::set unbroadcast_txids; + + static Mutex dump_mutex; + LOCK(dump_mutex); + + { + LOCK(pool.cs); + for (const auto &i : pool.mapDeltas) { + mapDeltas[i.first] = i.second; + } + vinfo = pool.infoAll(); + unbroadcast_txids = pool.GetUnbroadcastTxs(); + } + + auto mid = SteadyClock::now(); + + try { + FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")}; + if (!filestr) { + return false; + } + + CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); + + uint64_t version = MEMPOOL_DUMP_VERSION; + file << version; + + file << (uint64_t)vinfo.size(); + for (const auto& i : vinfo) { + file << *(i.tx); + file << int64_t{count_seconds(i.m_time)}; + file << int64_t{i.nFeeDelta}; + mapDeltas.erase(i.tx->GetHash()); + } + + file << mapDeltas; + + LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); + file << unbroadcast_txids; + + if (!skip_file_commit && !FileCommit(file.Get())) + throw std::runtime_error("FileCommit failed"); + file.fclose(); + if (!RenameOver(dump_path + ".new", dump_path)) { + throw std::runtime_error("Rename failed"); + } + auto last = SteadyClock::now(); + + LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", + Ticks(mid - start), + Ticks(last - mid)); + } catch (const std::exception& e) { + LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); + return false; + } + return true; +} + +} // namespace kernel diff --git a/src/kernel/mempool_persist.h b/src/kernel/mempool_persist.h new file mode 100644 index 00000000000..9a15ec6dcaf --- /dev/null +++ b/src/kernel/mempool_persist.h @@ -0,0 +1,28 @@ +// 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_KERNEL_MEMPOOL_PERSIST_H +#define BITCOIN_KERNEL_MEMPOOL_PERSIST_H + +#include + +class CChainState; +class CTxMemPool; + +namespace kernel { + +/** Dump the mempool to disk. */ +bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, + fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen, + bool skip_file_commit = false); + +/** Load the mempool from disk. */ +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, + CChainState& active_chainstate, + fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); + +} // namespace kernel + + +#endif // BITCOIN_KERNEL_MEMPOOL_PERSIST_H diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 7f4ff47941c..3b53ec82e4b 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -5,6 +5,8 @@ #include +#include + #include #include #include @@ -19,6 +21,8 @@ #include #include +using kernel::DumpMempool; + using node::DEFAULT_MAX_RAW_TX_FEE_RATE; using node::MempoolPath; using node::ShouldPersistMempool; diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index 357d8a9987c..90c1a71d9f5 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include + #include #include #include @@ -17,6 +19,8 @@ #include #include +using kernel::DumpMempool; + using node::MempoolPath; namespace { diff --git a/src/validation.cpp b/src/validation.cpp index 510071de22e..6840753cd4a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5,6 +5,9 @@ #include +#include +#include + #include #include #include @@ -19,7 +22,6 @@ #include #include #include -#include #include #include #include @@ -64,6 +66,7 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; using kernel::ComputeUTXOStats; +using kernel::LoadMempool; using fsbridge::FopenFn; using node::BLOCKFILE_CHUNK_SIZE; @@ -4640,157 +4643,6 @@ bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) return ret; } -static const uint64_t MEMPOOL_DUMP_VERSION = 1; - -bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, FopenFn mockable_fopen_function) -{ - if (load_path.empty()) return false; - - FILE* filestr{mockable_fopen_function(load_path, "rb")}; - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - if (file.IsNull()) { - LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); - return false; - } - - int64_t count = 0; - int64_t expired = 0; - int64_t failed = 0; - int64_t already_there = 0; - int64_t unbroadcast = 0; - auto now = NodeClock::now(); - - try { - uint64_t version; - file >> version; - if (version != MEMPOOL_DUMP_VERSION) { - return false; - } - uint64_t num; - file >> num; - while (num) { - --num; - CTransactionRef tx; - int64_t nTime; - int64_t nFeeDelta; - file >> tx; - file >> nTime; - file >> nFeeDelta; - - CAmount amountdelta = nFeeDelta; - if (amountdelta) { - pool.PrioritiseTransaction(tx->GetHash(), amountdelta); - } - if (nTime > TicksSinceEpoch(now - pool.m_expiry)) { - LOCK(cs_main); - const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false); - if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) { - ++count; - } else { - // mempool may contain the transaction already, e.g. from - // wallet(s) having loaded it while we were processing - // mempool transactions; consider these as valid, instead of - // failed, but mark them as 'already there' - if (pool.exists(GenTxid::Txid(tx->GetHash()))) { - ++already_there; - } else { - ++failed; - } - } - } else { - ++expired; - } - if (ShutdownRequested()) - return false; - } - std::map mapDeltas; - file >> mapDeltas; - - for (const auto& i : mapDeltas) { - pool.PrioritiseTransaction(i.first, i.second); - } - - std::set unbroadcast_txids; - file >> unbroadcast_txids; - unbroadcast = unbroadcast_txids.size(); - for (const auto& txid : unbroadcast_txids) { - // Ensure transactions were accepted to mempool then add to - // unbroadcast set. - if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid); - } - } catch (const std::exception& e) { - LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); - return false; - } - - LogPrintf("Imported mempool transactions from disk: %i succeeded, %i failed, %i expired, %i already there, %i waiting for initial broadcast\n", count, failed, expired, already_there, unbroadcast); - return true; -} - -bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function, bool skip_file_commit) -{ - auto start = SteadyClock::now(); - - std::map mapDeltas; - std::vector vinfo; - std::set unbroadcast_txids; - - static Mutex dump_mutex; - LOCK(dump_mutex); - - { - LOCK(pool.cs); - for (const auto &i : pool.mapDeltas) { - mapDeltas[i.first] = i.second; - } - vinfo = pool.infoAll(); - unbroadcast_txids = pool.GetUnbroadcastTxs(); - } - - auto mid = SteadyClock::now(); - - try { - FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")}; - if (!filestr) { - return false; - } - - CAutoFile file(filestr, SER_DISK, CLIENT_VERSION); - - uint64_t version = MEMPOOL_DUMP_VERSION; - file << version; - - file << (uint64_t)vinfo.size(); - for (const auto& i : vinfo) { - file << *(i.tx); - file << int64_t{count_seconds(i.m_time)}; - file << int64_t{i.nFeeDelta}; - mapDeltas.erase(i.tx->GetHash()); - } - - file << mapDeltas; - - LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size()); - file << unbroadcast_txids; - - if (!skip_file_commit && !FileCommit(file.Get())) - throw std::runtime_error("FileCommit failed"); - file.fclose(); - if (!RenameOver(dump_path + ".new", dump_path)) { - throw std::runtime_error("Rename failed"); - } - auto last = SteadyClock::now(); - - LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n", - Ticks(mid - start), - Ticks(last - mid)); - } catch (const std::exception& e) { - LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); - return false; - } - return true; -} - //! Guess how far we are in the verification process at the given block index //! require cs_main if pindex has not been validated yet (because nChainTx might be unset) double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex *pindex) { diff --git a/src/validation.h b/src/validation.h index df4fc77980c..f89d42e310a 100644 --- a/src/validation.h +++ b/src/validation.h @@ -1012,12 +1012,6 @@ bool DeploymentEnabled(const ChainstateManager& chainman, DEP dep) return DeploymentEnabled(chainman.GetConsensus(), dep); } -/** Dump the mempool to disk. */ -bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen, bool skip_file_commit = false); - -/** Load the mempool from disk. */ -bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); - /** * Return the expected assumeutxo value for a given height, if one exists. * diff --git a/test/lint/lint-circular-dependencies.py b/test/lint/lint-circular-dependencies.py index 5d157eb4b1b..f44ed8f7c7e 100755 --- a/test/lint/lint-circular-dependencies.py +++ b/test/lint/lint-circular-dependencies.py @@ -22,6 +22,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES = ( "wallet/fees -> wallet/wallet -> wallet/fees", "wallet/wallet -> wallet/walletdb -> wallet/wallet", "kernel/coinstats -> validation -> kernel/coinstats", + "kernel/mempool_persist -> validation -> kernel/mempool_persist", ) CODE_DIR = "src"