Skip to content

Commit d232e36

Browse files
committed
Merge bitcoin/bitcoin#28207: mempool: Persist with XOR
fa6b053 mempool: persist with XOR (MarcoFalke) Pull request description: Currently the `mempool.dat` file stores data received from remote peers as-is. This may be problematic when a program other than Bitcoin Core tries to interpret them by accident. For example, an anti-virus program or other program may scan the file and move it into quarantine, or delete it, or corrupt it. While the local wallet is expected to re-submit any pending transactions, unrelated transactions may be missing from the mempool after a restart. This may cause fee estimates to be off, or may cause block relay to be slower. Fix this, similar to bitcoin/bitcoin#6650, by rolling a random XOR pattern over the dat file when writing or reading it. Obviously this can only protect against programs that accidentally and unintentionally are trying to mess with the dat file. Any program that intentionally wants to mess with the dat file can still trivially do so. ACKs for top commit: achow101: re-ACK fa6b053 glozow: reACK fa6b053 ismaelsadeeq: ACK fa6b053 Tree-SHA512: ded2ce3d81bc944b828263534e3178a1e45a914fe8e024f4a14c6561a73e301820944ecc75dd704b3d4221a7a3a5c0597ccab79546250c1197609ee981fe324e
2 parents 6342348 + fa6b053 commit d232e36

File tree

9 files changed

+49
-15
lines changed

9 files changed

+49
-15
lines changed

doc/release-notes-28207.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
mempool.dat compatibility
2+
========================
3+
4+
The `mempool.dat` file created by -persistmempool or the savemempool RPC will
5+
be written in a new format, which can not be read by previous software
6+
releases. To allow for a downgrade, a temporary setting `-persistmempoolv1` has
7+
been added to fall back to the legacy format.

src/init.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,11 @@ void SetupServerArgs(ArgsManager& argsman)
458458
argsman.AddArg("-par=<n>", strprintf("Set the number of script verification threads (%u to %d, 0 = auto, <0 = leave that many cores free, default: %d)",
459459
-GetNumCores(), MAX_SCRIPTCHECK_THREADS, DEFAULT_SCRIPTCHECK_THREADS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
460460
argsman.AddArg("-persistmempool", strprintf("Whether to save the mempool on shutdown and load on restart (default: %u)", DEFAULT_PERSIST_MEMPOOL), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
461+
argsman.AddArg("-persistmempoolv1",
462+
strprintf("Whether a mempool.dat file created by -persistmempool or the savemempool RPC will be written in the legacy format "
463+
"(version 1) or the current format (version 2). This temporary option will be removed in the future. (default: %u)",
464+
DEFAULT_PERSIST_V1_DAT),
465+
ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
461466
argsman.AddArg("-pid=<file>", strprintf("Specify pid file. Relative paths will be prefixed by a net-specific datadir location. (default: %s)", BITCOIN_PID_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
462467
argsman.AddArg("-prune=<n>", strprintf("Reduce storage requirements by enabling pruning (deleting) of old blocks. This allows the pruneblockchain RPC to be called to delete specific blocks and enables automatic pruning of old blocks if a target size in MiB is provided. This mode is incompatible with -txindex. "
463468
"Warning: Reverting this setting requires re-downloading the entire blockchain. "

src/kernel/mempool_options.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ static constexpr unsigned int DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB{5};
2323
static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336};
2424
/** Default for -mempoolfullrbf, if the transaction replaceability signaling is ignored */
2525
static constexpr bool DEFAULT_MEMPOOL_FULL_RBF{false};
26+
/** Whether to fall back to legacy V1 serialization when writing mempool.dat */
27+
static constexpr bool DEFAULT_PERSIST_V1_DAT{false};
2628
/** Default for -acceptnonstdtxn */
2729
static constexpr bool DEFAULT_ACCEPT_NON_STD_TXN{false};
2830

@@ -56,6 +58,7 @@ struct MemPoolOptions {
5658
bool permit_bare_multisig{DEFAULT_PERMIT_BAREMULTISIG};
5759
bool require_standard{true};
5860
bool full_rbf{DEFAULT_MEMPOOL_FULL_RBF};
61+
bool persist_v1_dat{DEFAULT_PERSIST_V1_DAT};
5962
MemPoolLimits limits{};
6063
};
6164
} // namespace kernel

src/kernel/mempool_persist.cpp

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <consensus/amount.h>
99
#include <logging.h>
1010
#include <primitives/transaction.h>
11+
#include <random.h>
1112
#include <serialize.h>
1213
#include <streams.h>
1314
#include <sync.h>
@@ -34,14 +35,14 @@ using fsbridge::FopenFn;
3435

3536
namespace kernel {
3637

37-
static const uint64_t MEMPOOL_DUMP_VERSION = 1;
38+
static const uint64_t MEMPOOL_DUMP_VERSION_NO_XOR_KEY{1};
39+
static const uint64_t MEMPOOL_DUMP_VERSION{2};
3840

3941
bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active_chainstate, ImportMempoolOptions&& opts)
4042
{
4143
if (load_path.empty()) return false;
4244

43-
FILE* filestr{opts.mockable_fopen_function(load_path, "rb")};
44-
CAutoFile file{filestr, CLIENT_VERSION};
45+
CAutoFile file{opts.mockable_fopen_function(load_path, "rb"), CLIENT_VERSION};
4546
if (file.IsNull()) {
4647
LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n");
4748
return false;
@@ -57,9 +58,15 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active
5758
try {
5859
uint64_t version;
5960
file >> version;
60-
if (version != MEMPOOL_DUMP_VERSION) {
61+
std::vector<std::byte> xor_key;
62+
if (version == MEMPOOL_DUMP_VERSION_NO_XOR_KEY) {
63+
// Leave XOR-key empty
64+
} else if (version == MEMPOOL_DUMP_VERSION) {
65+
file >> xor_key;
66+
} else {
6167
return false;
6268
}
69+
file.SetXor(xor_key);
6370
uint64_t num;
6471
file >> num;
6572
while (num) {
@@ -151,17 +158,22 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock
151158

152159
auto mid = SteadyClock::now();
153160

154-
try {
155-
FILE* filestr{mockable_fopen_function(dump_path + ".new", "wb")};
156-
if (!filestr) {
157-
return false;
158-
}
159-
160-
CAutoFile file{filestr, CLIENT_VERSION};
161+
CAutoFile file{mockable_fopen_function(dump_path + ".new", "wb"), CLIENT_VERSION};
162+
if (file.IsNull()) {
163+
return false;
164+
}
161165

162-
uint64_t version = MEMPOOL_DUMP_VERSION;
166+
try {
167+
const uint64_t version{pool.m_persist_v1_dat ? MEMPOOL_DUMP_VERSION_NO_XOR_KEY : MEMPOOL_DUMP_VERSION};
163168
file << version;
164169

170+
std::vector<std::byte> xor_key(8);
171+
if (!pool.m_persist_v1_dat) {
172+
FastRandomContext{}.fillrand(xor_key);
173+
file << xor_key;
174+
}
175+
file.SetXor(xor_key);
176+
165177
file << (uint64_t)vinfo.size();
166178
for (const auto& i : vinfo) {
167179
file << *(i.tx);

src/node/mempool_args.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP
9393

9494
mempool_opts.full_rbf = argsman.GetBoolArg("-mempoolfullrbf", mempool_opts.full_rbf);
9595

96+
mempool_opts.persist_v1_dat = argsman.GetBoolArg("-persistmempoolv1", mempool_opts.persist_v1_dat);
97+
9698
ApplyArgsManOptions(argsman, mempool_opts.limits);
9799

98100
return {};

src/streams.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ class AutoFile
476476
{
477477
protected:
478478
std::FILE* m_file;
479-
const std::vector<std::byte> m_xor;
479+
std::vector<std::byte> m_xor;
480480

481481
public:
482482
explicit AutoFile(std::FILE* file, std::vector<std::byte> data_xor={}) : m_file{file}, m_xor{std::move(data_xor)} {}
@@ -516,6 +516,9 @@ class AutoFile
516516
*/
517517
bool IsNull() const { return m_file == nullptr; }
518518

519+
/** Continue with a different XOR key */
520+
void SetXor(std::vector<std::byte> data_xor) { m_xor = data_xor; }
521+
519522
/** Implementation detail, only used internally. */
520523
std::size_t detail_fread(Span<std::byte> dst);
521524

src/txmempool.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,7 @@ CTxMemPool::CTxMemPool(const Options& opts)
412412
m_max_datacarrier_bytes{opts.max_datacarrier_bytes},
413413
m_require_standard{opts.require_standard},
414414
m_full_rbf{opts.full_rbf},
415+
m_persist_v1_dat{opts.persist_v1_dat},
415416
m_limits{opts.limits}
416417
{
417418
}

src/txmempool.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@ class CTxMemPool
446446
const std::optional<unsigned> m_max_datacarrier_bytes;
447447
const bool m_require_standard;
448448
const bool m_full_rbf;
449+
const bool m_persist_v1_dat;
449450

450451
const Limits m_limits;
451452

test/functional/mempool_compatibility.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ def skip_test_if_missing_module(self):
2828

2929
def setup_network(self):
3030
self.add_nodes(self.num_nodes, versions=[
31-
200100, # Last release with previous mempool format
31+
200100, # Last release without unbroadcast serialization and without XOR
3232
None,
3333
])
3434
self.start_nodes()
@@ -59,7 +59,7 @@ def run_test(self):
5959
old_node_mempool.rename(new_node_mempool)
6060

6161
self.log.info("Start new node and verify mempool contains the tx")
62-
self.start_node(1)
62+
self.start_node(1, extra_args=["-persistmempoolv1=1"])
6363
assert old_tx_hash in new_node.getrawmempool()
6464

6565
self.log.info("Add unbroadcasted tx to mempool on new node and shutdown")

0 commit comments

Comments
 (0)