Skip to content

Commit 04afe98

Browse files
knstglozow
andcommitted
Merge bitcoin#25487: [kernel 3b/n] Decouple {Dump,Load}Mempool from ArgsManager
cb3e9a1 Move {Load,Dump}Mempool to kernel namespace (Carl Dong) aa30676 Move DEFAULT_PERSIST_MEMPOOL out of libbitcoinkernel (Carl Dong) 06b88ff LoadMempool: Pass in load_path, stop using gArgs (Carl Dong) b857ac6 test/fuzz: Invoke LoadMempool via CChainState (Carl Dong) b326725 Move FopenFn to fsbridge namespace (Carl Dong) ae1e8e3 mempool: Use NodeClock+friends for LoadMempool (Carl Dong) f9e8e57 mempool: Improve comments for [GS]etLoadTried (Carl Dong) 813962d scripted-diff: Rename m_is_loaded -> m_load_tried (Carl Dong) 413f4bb DumpMempool: Pass in dump_path, stop using gArgs (Carl Dong) bd44078 DumpMempool: Use std::chrono instead of weird int64_t arthmetics (Carl Dong) c84390b test/mempool_persist: Test manual savemempool when -persistmempool=0 (Carl Dong) Pull request description: This is part of the `libbitcoinkernel` project: bitcoin#24303, https://github.com/bitcoin/bitcoin/projects/18 ----- This PR moves `{Dump,Load}Mempool` into its own `kernel/mempool_persist` module and introduces `ArgsManager` `node::` helpers in `node/mempool_persist_args`to remove the scattered calls to `GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)`. More context can be gleaned from the commit messages. ----- One thing I was reflecting on as I wrote this was that in the long run, I think we should probably invert the validation <-> mempool relationship. Instead of mempool not depending on validation, it might make more sense to have validation not depend on mempool. Not super urgent since `libbitcoinkernel` will include both validation and mempool, but perhaps something for the future. ACKs for top commit: glozow: re ACK cb3e9a1 via `git range-diff 7ae032e...cb3e9a1` MarcoFalke: ACK cb3e9a1 🔒 ryanofsky: Code review ACK cb3e9a1 Tree-SHA512: 979d7237c3abb5a1dd9b5ad3dbf3b954f906a6d8320ed7b923557f41a4472deccae3e8a6bca0018c8e7a3c4a93afecc502acd1e26756f2054f157f1c0edd939d Co-authored-by: glozow <gloriajzhao@gmail.com>
1 parent 79ff48a commit 04afe98

22 files changed

Lines changed: 384 additions & 206 deletions

ci/dash/lint-tidy.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ iwyu_tool.py \
8787
"src/compat" \
8888
"src/dbwrapper.cpp" \
8989
"src/init" \
90+
"src/kernel/mempool_persist.cpp" \
9091
"src/node/chainstate.cpp" \
9192
"src/node/minisketchwrapper.cpp" \
9293
"src/policy/feerate.cpp" \
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
# Nothing for now.
22
[
3+
{ include: [ "<bits/chrono.h>", private, "<chrono>", public ] },
34
]

src/Makefile.am

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ BITCOIN_CORE_H = \
286286
kernel/context.h \
287287
kernel/mempool_limits.h \
288288
kernel/mempool_options.h \
289+
kernel/mempool_persist.h \
289290
key.h \
290291
key_io.h \
291292
limitedmap.h \
@@ -339,6 +340,7 @@ BITCOIN_CORE_H = \
339340
node/context.h \
340341
node/eviction.h \
341342
node/interface_ui.h \
343+
node/mempool_persist_args.h \
342344
node/miner.h \
343345
node/minisketchwrapper.h \
344346
node/psbt.h \
@@ -563,6 +565,7 @@ libbitcoin_node_a_SOURCES = \
563565
kernel/checks.cpp \
564566
kernel/coinstats.cpp \
565567
kernel/context.cpp \
568+
kernel/mempool_persist.cpp \
566569
llmq/blockprocessor.cpp \
567570
llmq/commitment.cpp \
568571
llmq/context.cpp \
@@ -601,6 +604,7 @@ libbitcoin_node_a_SOURCES = \
601604
node/eviction.cpp \
602605
node/interface_ui.cpp \
603606
node/interfaces.cpp \
607+
node/mempool_persist_args.cpp \
604608
node/miner.cpp \
605609
node/minisketchwrapper.cpp \
606610
node/psbt.cpp \
@@ -1279,6 +1283,7 @@ libdashkernel_la_SOURCES = \
12791283
kernel/checks.cpp \
12801284
kernel/coinstats.cpp \
12811285
kernel/context.cpp \
1286+
kernel/mempool_persist.cpp \
12821287
key.cpp \
12831288
key_io.cpp \
12841289
llmq/blockprocessor.cpp \

src/Makefile.test_fuzz.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ TEST_FUZZ_H = \
1212
test/fuzz/FuzzedDataProvider.h \
1313
test/fuzz/util.h \
1414
test/util/mining.h \
15+
test/fuzz/mempool_utils.h \
1516
test/fuzz/util/net.h
1617

1718
libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS)

src/fs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include <cstdio>
1111
#include <filesystem> // IWYU pragma: export
12+
#include <functional>
1213
#include <iomanip>
1314
#include <ios>
1415
#include <ostream>
@@ -204,6 +205,7 @@ bool create_directories(const std::filesystem::path& p, std::error_code& ec) = d
204205

205206
/** Bridge operations to C stdio */
206207
namespace fsbridge {
208+
using FopenFn = std::function<FILE*(const fs::path&, const char*)>;
207209
FILE *fopen(const fs::path& p, const char *mode);
208210

209211
/**

src/init.cpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <init.h>
1212

1313
#include <kernel/checks.h>
14+
#include <kernel/mempool_persist.h>
1415

1516
#include <addrman.h>
1617
#include <banman.h>
@@ -51,6 +52,7 @@
5152
#include <node/chainstate.h>
5253
#include <node/context.h>
5354
#include <node/interface_ui.h>
55+
#include <node/mempool_persist_args.h>
5456
#include <node/miner.h>
5557
#include <node/sync_manager.h>
5658
#include <node/txreconciliation.h>
@@ -150,15 +152,19 @@
150152
#endif
151153

152154
using kernel::CoinStatsHashType;
155+
using kernel::DumpMempool;
153156

154157
using node::CacheSizes;
155158
using node::CalculateCacheSizes;
156159
using node::ChainstateLoadingError;
157160
using node::ChainstateLoadVerifyError;
158161
using node::DashChainstateSetupClose;
162+
using node::DEFAULT_PERSIST_MEMPOOL;
159163
using node::DEFAULT_PRINTPRIORITY;
160164
using node::DEFAULT_STOPAFTERBLOCKIMPORT;
161165
using node::LoadChainstate;
166+
using node::MempoolPath;
167+
using node::ShouldPersistMempool;
162168
using node::NodeContext;
163169
using node::ThreadImport;
164170
using node::VerifyLoadedChainstate;
@@ -338,8 +344,8 @@ void PrepareShutdown(NodeContext& node)
338344
node.netgroupman.reset();
339345
::g_stats_client.reset();
340346

341-
if (node.mempool && node.mempool->IsLoaded() && node.args->GetBoolArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
342-
DumpMempool(*node.mempool);
347+
if (node.mempool && node.mempool->GetLoadTried() && ShouldPersistMempool(*node.args)) {
348+
DumpMempool(*node.mempool, MempoolPath(*node.args));
343349
}
344350

345351
// Drop transactions we were still watching, and record fee estimations.
@@ -2478,7 +2484,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
24782484
chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &args, &chainman, &node] {
24792485
// ThreadImport can switch fReindex from true to false, fetch its original state here to use later
24802486
bool skip_evodb_repair_on_reindex = fReindex || fReindexChainState;
2481-
ThreadImport(chainman, vImportFiles, args);
2487+
ThreadImport(chainman, vImportFiles, args, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{});
24822488

24832489
{
24842490
const CBlockIndex* tip = WITH_LOCK(::cs_main, return chainman.ActiveTip());

src/kernel/mempool_persist.cpp

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <kernel/mempool_persist.h>
6+
7+
#include <clientversion.h>
8+
#include <consensus/amount.h>
9+
#include <fs.h>
10+
#include <logging.h>
11+
#include <primitives/transaction.h>
12+
#include <serialize.h>
13+
#include <shutdown.h>
14+
#include <streams.h>
15+
#include <sync.h>
16+
#include <txmempool.h>
17+
#include <uint256.h>
18+
#include <util/system.h>
19+
#include <util/time.h>
20+
#include <validation.h>
21+
22+
#include <chrono>
23+
#include <cstdint>
24+
#include <cstdio>
25+
#include <exception>
26+
#include <functional>
27+
#include <map>
28+
#include <memory>
29+
#include <set>
30+
#include <stdexcept>
31+
#include <utility>
32+
#include <vector>
33+
34+
using fsbridge::FopenFn;
35+
36+
namespace kernel {
37+
38+
static const uint64_t MEMPOOL_DUMP_VERSION = 1;
39+
40+
bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, CChainState& active_chainstate, FopenFn mockable_fopen_function)
41+
{
42+
if (load_path.empty()) return false;
43+
44+
FILE* filestr{mockable_fopen_function(load_path, "rb")};
45+
AutoFile file(filestr);
46+
if (file.IsNull()) {
47+
LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n");
48+
return false;
49+
}
50+
51+
int64_t count = 0;
52+
int64_t expired = 0;
53+
int64_t failed = 0;
54+
int64_t already_there = 0;
55+
int64_t unbroadcast = 0;
56+
auto now = NodeClock::now();
57+
58+
try {
59+
uint64_t version;
60+
file >> version;
61+
if (version != MEMPOOL_DUMP_VERSION) {
62+
return false;
63+
}
64+
uint64_t total_txns_to_load;
65+
file >> total_txns_to_load;
66+
uint64_t txns_tried = 0;
67+
LogInfo("Loading %u mempool transactions from disk...\n", total_txns_to_load);
68+
int next_tenth_to_report = 0;
69+
while (txns_tried < total_txns_to_load) {
70+
const int percentage_done(100.0 * txns_tried / total_txns_to_load);
71+
if (next_tenth_to_report < percentage_done / 10) {
72+
LogInfo("Progress loading mempool transactions from disk: %d%% (tried %u, %u remaining)\n",
73+
percentage_done, txns_tried, total_txns_to_load - txns_tried);
74+
next_tenth_to_report = percentage_done / 10;
75+
}
76+
++txns_tried;
77+
78+
CTransactionRef tx;
79+
int64_t nTime;
80+
int64_t nFeeDelta;
81+
file >> tx;
82+
file >> nTime;
83+
file >> nFeeDelta;
84+
85+
CAmount amountdelta = nFeeDelta;
86+
if (amountdelta) {
87+
pool.PrioritiseTransaction(tx->GetHash(), amountdelta);
88+
}
89+
if (nTime > TicksSinceEpoch<std::chrono::seconds>(now - pool.m_expiry)) {
90+
LOCK(cs_main);
91+
const auto& accepted = AcceptToMemoryPool(active_chainstate, tx, nTime, /*bypass_limits=*/false, /*test_accept=*/false);
92+
if (accepted.m_result_type == MempoolAcceptResult::ResultType::VALID) {
93+
++count;
94+
} else {
95+
// mempool may contain the transaction already, e.g. from
96+
// wallet(s) having loaded it while we were processing
97+
// mempool transactions; consider these as valid, instead of
98+
// failed, but mark them as 'already there'
99+
if (pool.exists(tx->GetHash())) {
100+
++already_there;
101+
} else {
102+
++failed;
103+
}
104+
}
105+
} else {
106+
++expired;
107+
}
108+
if (ShutdownRequested())
109+
return false;
110+
}
111+
std::map<uint256, CAmount> mapDeltas;
112+
file >> mapDeltas;
113+
114+
for (const auto& i : mapDeltas) {
115+
pool.PrioritiseTransaction(i.first, i.second);
116+
}
117+
118+
std::set<uint256> unbroadcast_txids;
119+
file >> unbroadcast_txids;
120+
unbroadcast = unbroadcast_txids.size();
121+
for (const auto& txid : unbroadcast_txids) {
122+
// Ensure transactions were accepted to mempool then add to
123+
// unbroadcast set.
124+
if (pool.get(txid) != nullptr) pool.AddUnbroadcastTx(txid);
125+
}
126+
} catch (const std::exception& e) {
127+
LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what());
128+
return false;
129+
}
130+
131+
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);
132+
return true;
133+
}
134+
135+
bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mockable_fopen_function, bool skip_file_commit)
136+
{
137+
auto start = SteadyClock::now();
138+
139+
std::map<uint256, CAmount> mapDeltas;
140+
std::vector<TxMempoolInfo> vinfo;
141+
std::set<uint256> unbroadcast_txids;
142+
143+
static Mutex dump_mutex;
144+
LOCK(dump_mutex);
145+
146+
{
147+
LOCK(pool.cs);
148+
for (const auto &i : pool.mapDeltas) {
149+
mapDeltas[i.first] = i.second;
150+
}
151+
vinfo = pool.infoAll();
152+
unbroadcast_txids = pool.GetUnbroadcastTxs();
153+
}
154+
155+
auto mid = SteadyClock::now();
156+
157+
try {
158+
FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")};
159+
if (!filestr) {
160+
return false;
161+
}
162+
163+
AutoFile file(filestr);
164+
165+
uint64_t version = MEMPOOL_DUMP_VERSION;
166+
file << version;
167+
168+
file << (uint64_t)vinfo.size();
169+
for (const auto& i : vinfo) {
170+
file << *(i.tx);
171+
file << int64_t{count_seconds(i.m_time)};
172+
file << int64_t{i.nFeeDelta};
173+
mapDeltas.erase(i.tx->GetHash());
174+
}
175+
176+
file << mapDeltas;
177+
178+
LogPrintf("Writing %d unbroadcast transactions to disk.\n", unbroadcast_txids.size());
179+
file << unbroadcast_txids;
180+
181+
if (!skip_file_commit && !FileCommit(file.Get()))
182+
throw std::runtime_error("FileCommit failed");
183+
file.fclose();
184+
if (!RenameOver(dump_path + ".new", dump_path)) {
185+
throw std::runtime_error("Rename failed");
186+
}
187+
auto last = SteadyClock::now();
188+
189+
LogPrintf("Dumped mempool: %gs to copy, %gs to dump\n",
190+
Ticks<SecondsDouble>(mid - start),
191+
Ticks<SecondsDouble>(last - mid));
192+
} catch (const std::exception& e) {
193+
LogPrintf("Failed to dump mempool: %s. Continuing anyway.\n", e.what());
194+
return false;
195+
}
196+
return true;
197+
}
198+
199+
} // namespace kernel

src/kernel/mempool_persist.h

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_KERNEL_MEMPOOL_PERSIST_H
6+
#define BITCOIN_KERNEL_MEMPOOL_PERSIST_H
7+
8+
#include <fs.h>
9+
10+
class CChainState;
11+
class CTxMemPool;
12+
13+
namespace kernel {
14+
15+
/** Dump the mempool to disk. */
16+
bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path,
17+
fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen,
18+
bool skip_file_commit = false);
19+
20+
/** Load the mempool from disk. */
21+
bool LoadMempool(CTxMemPool& pool, const fs::path& load_path,
22+
CChainState& active_chainstate,
23+
fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen);
24+
25+
} // namespace kernel
26+
27+
28+
#endif // BITCOIN_KERNEL_MEMPOOL_PERSIST_H

src/node/blockstorage.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -836,7 +836,7 @@ struct CImportingNow {
836836
}
837837
};
838838

839-
void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args)
839+
void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args, const fs::path& mempool_path)
840840
{
841841
ScheduleBatchPriority();
842842

@@ -908,7 +908,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile
908908
return;
909909
}
910910
} // End scope of CImportingNow
911-
912-
chainman.ActiveChainstate().LoadMempool(args);
911+
chainman.ActiveChainstate().LoadMempool(mempool_path);
913912
}
914913
} // namespace node

src/node/blockstorage.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ bool ReadBlockFromDisk(CBlock& block, const CBlockIndex* pindex, const Consensus
227227

228228
bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex);
229229

230-
void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args);
230+
void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFiles, const ArgsManager& args, const fs::path& mempool_path);
231231
} // namespace node
232232

233233
#endif // BITCOIN_NODE_BLOCKSTORAGE_H

0 commit comments

Comments
 (0)