From acdca4217354a9cffcb371f956b5ba7297fc9032 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 28 Jul 2023 11:48:07 +0100 Subject: [PATCH 1/9] Merge bitcoin/bitcoin#28168: refactor: Remove unused raw-pointer read helper from univalue fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 Remove unused raw-pointer read helper from univalue (MarcoFalke) Pull request description: The helpers are unused outside of tests and redundant with the existing `bool read(std::string_view raw);`. Fix both issues by removing them. Also, simplify the tests code by removing a `std::string` constructor where possible. ACKs for top commit: stickies-v: utACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 TheCharlatan: tACK fa940f41eaffa4b2a28c465a10a4c12d4b8976b8 Tree-SHA512: 60c154c1046f01551335af79bf820a6104844f63e89977271b4336b3cd06ac3bab1379e18b7bc61d12bef7446029e91c16541ddecf9e88bc8bc897fc1f6ee2c8 Co-authored-by: fanquake --- src/Makefile.test.include | 5 +++-- src/test/base58_tests.cpp | 4 ++-- src/test/blockfilter_tests.cpp | 4 +--- src/test/evo_trivialvalidation.cpp | 4 ++-- src/test/governance_validators_tests.cpp | 4 ++-- src/test/key_io_tests.cpp | 6 +++--- src/test/rpc_tests.cpp | 2 +- src/test/script_tests.cpp | 4 ++-- src/test/sighash_tests.cpp | 2 +- src/test/transaction_tests.cpp | 4 ++-- src/univalue/include/univalue.h | 8 ++++---- src/univalue/lib/univalue_read.cpp | 8 +++++--- src/univalue/test/unitester.cpp | 2 +- src/wallet/test/bip39_tests.cpp | 2 +- 14 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/Makefile.test.include b/src/Makefile.test.include index efc9a88fcfc3..a2194169c846 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -490,8 +490,9 @@ endif %.json.h: %.json @$(MKDIR_P) $(@D) $(AM_V_GEN) { \ + echo "#include " && \ echo "namespace json_tests{" && \ - echo "static unsigned const char $(*F)[] = {" && \ - $(HEXDUMP) -v -e '8/1 "0x%02x, "' -e '"\n"' $< | $(SED) -e 's/0x ,//g' && \ + echo "static const std::string $(*F){" && \ + $(HEXDUMP) -v -e '8/1 "char(0x%02x), "' -e '"\n"' $< | $(SED) -e 's/char(0x ),//g' && \ echo "};};"; \ } > "$@.new" && mv -f "$@.new" "$@" diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 9954e6ba2cbb..cb3fa43e8ba1 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -23,7 +23,7 @@ BOOST_AUTO_TEST_SUITE(base58_tests) // Goal: test low-level base58 encoding functionality BOOST_AUTO_TEST_CASE(base58_EncodeBase58) { - UniValue tests = read_json(std::string(json_tests::base58_encode_decode, json_tests::base58_encode_decode + sizeof(json_tests::base58_encode_decode))); + UniValue tests = read_json(json_tests::base58_encode_decode); for (unsigned int idx = 0; idx < tests.size(); idx++) { const UniValue& test = tests[idx]; std::string strTest = test.write(); @@ -43,7 +43,7 @@ BOOST_AUTO_TEST_CASE(base58_EncodeBase58) // Goal: test low-level base58 decoding functionality BOOST_AUTO_TEST_CASE(base58_DecodeBase58) { - UniValue tests = read_json(std::string(json_tests::base58_encode_decode, json_tests::base58_encode_decode + sizeof(json_tests::base58_encode_decode))); + UniValue tests = read_json(json_tests::base58_encode_decode); std::vector result; for (unsigned int idx = 0; idx < tests.size(); idx++) { diff --git a/src/test/blockfilter_tests.cpp b/src/test/blockfilter_tests.cpp index e8b3b41af062..ca36f0a18208 100644 --- a/src/test/blockfilter_tests.cpp +++ b/src/test/blockfilter_tests.cpp @@ -135,9 +135,7 @@ BOOST_AUTO_TEST_CASE(blockfilter_basic_test) BOOST_AUTO_TEST_CASE(blockfilters_json_test) { UniValue json; - std::string json_data(json_tests::blockfilters, - json_tests::blockfilters + sizeof(json_tests::blockfilters)); - if (!json.read(json_data) || !json.isArray()) { + if (!json.read(json_tests::blockfilters) || !json.isArray()) { BOOST_ERROR("Parse error."); return; } diff --git a/src/test/evo_trivialvalidation.cpp b/src/test/evo_trivialvalidation.cpp index 992760282b97..262d3f100331 100644 --- a/src/test/evo_trivialvalidation.cpp +++ b/src/test/evo_trivialvalidation.cpp @@ -120,7 +120,7 @@ void trivialvalidation_runner(ChainstateManager& chainman, const std::string& js BOOST_AUTO_TEST_CASE(trivialvalidation_valid) { - const std::string json(json_tests::trivially_valid, json_tests::trivially_valid + sizeof(json_tests::trivially_valid)); + const std::string& json = json_tests::trivially_valid; bls::bls_legacy_scheme.store(true); trivialvalidation_runner(*m_node.chainman, json); @@ -131,7 +131,7 @@ BOOST_AUTO_TEST_CASE(trivialvalidation_valid) BOOST_AUTO_TEST_CASE(trivialvalidation_invalid) { - const std::string json(json_tests::trivially_invalid, json_tests::trivially_invalid + sizeof(json_tests::trivially_invalid)); + const std::string& json = json_tests::trivially_invalid; bls::bls_legacy_scheme.store(true); trivialvalidation_runner(*m_node.chainman, json); diff --git a/src/test/governance_validators_tests.cpp b/src/test/governance_validators_tests.cpp index 3ee0a2126bac..0a2fc41f2365 100644 --- a/src/test/governance_validators_tests.cpp +++ b/src/test/governance_validators_tests.cpp @@ -37,7 +37,7 @@ static std::string CreateEncodedProposalObject(const UniValue& objJSON) BOOST_AUTO_TEST_CASE(valid_proposals_test) { // all proposals are valid but expired - UniValue tests = read_json(std::string(json_tests::proposals_valid, json_tests::proposals_valid + sizeof(json_tests::proposals_valid))); + UniValue tests = read_json(json_tests::proposals_valid); BOOST_CHECK_MESSAGE(tests.size(), "Empty `tests`"); for(size_t i = 0; i < tests.size(); ++i) { @@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE(invalid_proposals_test) { // all proposals are invalid regardless of being expired or not // (i.e. we don't even check for expiration here) - UniValue tests = read_json(std::string(json_tests::proposals_invalid, json_tests::proposals_invalid + sizeof(json_tests::proposals_invalid))); + UniValue tests = read_json(json_tests::proposals_invalid); BOOST_CHECK_MESSAGE(tests.size(), "Empty `tests`"); for(size_t i = 0; i < tests.size(); ++i) { diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp index ba4208ad1f28..2dfc2b0c099e 100644 --- a/src/test/key_io_tests.cpp +++ b/src/test/key_io_tests.cpp @@ -24,7 +24,7 @@ BOOST_FIXTURE_TEST_SUITE(key_io_tests, BasicTestingSetup) // Goal: check that parsed keys match test payload BOOST_AUTO_TEST_CASE(key_io_valid_parse) { - UniValue tests = read_json(std::string(json_tests::key_io_valid, json_tests::key_io_valid + sizeof(json_tests::key_io_valid))); + UniValue tests = read_json(json_tests::key_io_valid); CKey privkey; CTxDestination destination; SelectParams(ChainType::MAIN); @@ -85,7 +85,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_parse) // Goal: check that generated keys match test vectors BOOST_AUTO_TEST_CASE(key_io_valid_gen) { - UniValue tests = read_json(std::string(json_tests::key_io_valid, json_tests::key_io_valid + sizeof(json_tests::key_io_valid))); + UniValue tests = read_json(json_tests::key_io_valid); for (unsigned int idx = 0; idx < tests.size(); idx++) { const UniValue& test = tests[idx]; @@ -123,7 +123,7 @@ BOOST_AUTO_TEST_CASE(key_io_valid_gen) // Goal: check that base58 parsing code is robust against a variety of corrupted data BOOST_AUTO_TEST_CASE(key_io_invalid) { - UniValue tests = read_json(std::string(json_tests::key_io_invalid, json_tests::key_io_invalid + sizeof(json_tests::key_io_invalid))); // Negative testcases + UniValue tests = read_json(json_tests::key_io_invalid); // Negative testcases CKey privkey; CTxDestination destination; diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 19593b66d9e6..939d39ae9b89 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -19,7 +19,7 @@ static UniValue JSON(std::string_view json) { UniValue value; - BOOST_CHECK(value.read(json.data(), json.size())); + BOOST_CHECK(value.read(json)); return value; } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index 1afb5e3d053b..582107243bc3 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -847,7 +847,7 @@ BOOST_AUTO_TEST_CASE(script_build) std::set tests_set; { - UniValue json_tests = read_json(std::string(json_tests::script_tests, json_tests::script_tests + sizeof(json_tests::script_tests))); + UniValue json_tests = read_json(json_tests::script_tests); for (unsigned int idx = 0; idx < json_tests.size(); idx++) { const UniValue& tv = json_tests[idx]; @@ -884,7 +884,7 @@ BOOST_AUTO_TEST_CASE(script_json_test) // Inner arrays are [ "scriptSig", "scriptPubKey", "flags", "expected_scripterror" ] // ... where scriptSig and scriptPubKey are stringified // scripts. - UniValue tests = read_json(std::string(json_tests::script_tests, json_tests::script_tests + sizeof(json_tests::script_tests))); + UniValue tests = read_json(json_tests::script_tests); for (unsigned int idx = 0; idx < tests.size(); idx++) { const UniValue& test = tests[idx]; diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 4993ff2fcf89..761718c720fd 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -160,7 +160,7 @@ BOOST_AUTO_TEST_CASE(sighash_test) // Goal: check that SignatureHash generates correct hash BOOST_AUTO_TEST_CASE(sighash_from_data) { - UniValue tests = read_json(std::string(json_tests::sighash, json_tests::sighash + sizeof(json_tests::sighash))); + UniValue tests = read_json(json_tests::sighash); for (unsigned int idx = 0; idx < tests.size(); idx++) { const UniValue& test = tests[idx]; diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 17cc08a4fa95..4d271e681b35 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -154,7 +154,7 @@ BOOST_FIXTURE_TEST_SUITE(transaction_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(tx_valid) { // Read tests from test/data/tx_valid.json - UniValue tests = read_json(std::string(json_tests::tx_valid, json_tests::tx_valid + sizeof(json_tests::tx_valid))); + UniValue tests = read_json(json_tests::tx_valid); for (unsigned int idx = 0; idx < tests.size(); idx++) { const UniValue& test = tests[idx]; @@ -238,7 +238,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) BOOST_AUTO_TEST_CASE(tx_invalid) { // Read tests from test/data/tx_invalid.json - UniValue tests = read_json(std::string(json_tests::tx_invalid, json_tests::tx_invalid + sizeof(json_tests::tx_invalid))); + UniValue tests = read_json(json_tests::tx_invalid); for (unsigned int idx = 0; idx < tests.size(); idx++) { const UniValue& test = tests[idx]; diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h index 94f80f9c27b9..acbce25741b9 100644 --- a/src/univalue/include/univalue.h +++ b/src/univalue/include/univalue.h @@ -7,13 +7,15 @@ #define BITCOIN_UNIVALUE_INCLUDE_UNIVALUE_H #include +#include #include -#include #include #include #include #include +#include #include +#include #include class UniValue { @@ -94,9 +96,7 @@ class UniValue { std::string write(unsigned int prettyIndent = 0, unsigned int indentLevel = 0) const; - bool read(const char *raw, size_t len); - bool read(const char *raw) { return read(raw, strlen(raw)); } - bool read(std::string_view raw) { return read(raw.data(), raw.size()); } + bool read(std::string_view raw); private: UniValue::VType typ; diff --git a/src/univalue/lib/univalue_read.cpp b/src/univalue/lib/univalue_read.cpp index 2f2385383c14..40d465f43826 100644 --- a/src/univalue/lib/univalue_read.cpp +++ b/src/univalue/lib/univalue_read.cpp @@ -5,10 +5,11 @@ #include #include -#include #include +#include #include #include +#include #include /* @@ -259,7 +260,7 @@ enum expect_bits : unsigned { #define setExpect(bit) (expectMask |= EXP_##bit) #define clearExpect(bit) (expectMask &= ~EXP_##bit) -bool UniValue::read(const char *raw, size_t size) +bool UniValue::read(std::string_view str_in) { clear(); @@ -270,7 +271,8 @@ bool UniValue::read(const char *raw, size_t size) unsigned int consumed; enum jtokentype tok = JTOK_NONE; enum jtokentype last_tok = JTOK_NONE; - const char* end = raw + size; + const char* raw{str_in.data()}; + const char* end{raw + str_in.size()}; do { last_tok = tok; diff --git a/src/univalue/test/unitester.cpp b/src/univalue/test/unitester.cpp index 6344a5a0ab8a..d27e647e5cf3 100644 --- a/src/univalue/test/unitester.cpp +++ b/src/univalue/test/unitester.cpp @@ -153,7 +153,7 @@ void no_nul_test() { char buf[] = "___[1,2,3]___"; UniValue val; - assert(val.read(buf + 3, 7)); + assert(val.read({buf + 3, 7})); } int main (int argc, char *argv[]) diff --git a/src/wallet/test/bip39_tests.cpp b/src/wallet/test/bip39_tests.cpp index aebd805627f6..3787d807d74b 100644 --- a/src/wallet/test/bip39_tests.cpp +++ b/src/wallet/test/bip39_tests.cpp @@ -21,7 +21,7 @@ BOOST_FIXTURE_TEST_SUITE(bip39_tests, BasicTestingSetup) // https://github.com/trezor/python-mnemonic/blob/b502451a33a440783926e04428115e0bed87d01f/vectors.json BOOST_AUTO_TEST_CASE(bip39_vectors) { - UniValue tests = read_json(std::string(json_tests::bip39_vectors, json_tests::bip39_vectors + sizeof(json_tests::bip39_vectors))); + UniValue tests = read_json(json_tests::bip39_vectors); for (unsigned int i = 0; i < tests.size(); i++) { // printf("%d\n", i); From 5f7b298d07198730a31797053968e84b37072e4c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 15 Aug 2023 10:03:51 -0400 Subject: [PATCH 2/9] Merge bitcoin/bitcoin#27460: rpc: Add importmempool RPC fa776e61cd64a5ffd9a4be589ab8efeb5421861a Add importmempool RPC (MarcoFalke) fa20d734a29ba50cd19b78cb4fe39a2d826131b7 refactor: Add and use kernel::ImportMempoolOptions (MarcoFalke) fa8866990dba7817427977bfe834efdb17114d37 doc: Clarify the getmempoolinfo.loaded RPC field documentation (MarcoFalke) 6888886cecf6665da70b3dc3772b3c12ef06ad76 Remove Chainstate::LoadMempool (MarcoFalke) Pull request description: Currently it is possible to import a mempool by placing it in the datadir and starting the node. However this has many issues: * Users aren't expected to fiddle with the datadir, possibly corrupting it * An existing mempool file in the datadir may be overwritten * The node needs to be restarted * Importing an untrusted file this way is dangerous, because it can corrupt the mempool Fix all issues by adding a new RPC. ACKs for top commit: ajtowns: utACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a achow101: ACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a glozow: reACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a Tree-SHA512: fcb1a92d6460839283c546c47a2d930c363ac1013c4c50dc5215ddf9fe5e51921d23fe0abfae0a5a7631983cfc7e2fff3788b70f95937d0a989a203be4d67546 Co-authored-by: Andrew Chow --- src/init.cpp | 9 +++- src/kernel/mempool_persist.cpp | 31 ++++++----- src/kernel/mempool_persist.h | 12 +++-- src/node/blockstorage.cpp | 3 +- src/node/blockstorage.h | 2 +- src/rpc/client.cpp | 4 ++ src/rpc/mempool.cpp | 63 ++++++++++++++++++++++- src/test/fuzz/rpc.cpp | 1 + src/test/fuzz/validation_load_mempool.cpp | 7 ++- src/txmempool.h | 4 +- src/validation.cpp | 8 --- src/validation.h | 3 -- test/functional/mempool_persist.py | 53 ++++++++++++++++++- 13 files changed, 165 insertions(+), 35 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index 62d7cc259e6c..6b4c9c636c6b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -171,6 +171,7 @@ using kernel::CoinStatsHashType; using kernel::DEFAULT_STOPAFTERBLOCKIMPORT; using kernel::DumpMempool; +using kernel::LoadMempool; using kernel::ValidationCacheSizes; using node::ApplyArgsManOptions; @@ -2489,7 +2490,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) chainman.m_load_block = std::thread(&util::TraceThread, "loadblk", [=, &args, &chainman, &node] { // ThreadImport can switch fReindex from true to false, fetch its original state here to use later bool skip_evodb_repair_on_reindex = fReindex || fReindexChainState; - ThreadImport(chainman, vImportFiles, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}); + ThreadImport(chainman, vImportFiles); + + // Load mempool from disk + if (auto* pool{chainman.ActiveChainstate().GetMempool()}) { + LoadMempool(*pool, ShouldPersistMempool(args) ? MempoolPath(args) : fs::path{}, chainman.ActiveChainstate(), {}); + pool->SetLoadTried(!ShutdownRequested()); + } { const CBlockIndex* tip = WITH_LOCK(::cs_main, return chainman.ActiveTip()); diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index 50cd969fc613..2e9d26a87472 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include #include @@ -37,11 +36,11 @@ namespace kernel { static const uint64_t MEMPOOL_DUMP_VERSION = 1; -bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active_chainstate, FopenFn mockable_fopen_function) +bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active_chainstate, ImportMempoolOptions&& opts) { if (load_path.empty()) return false; - FILE* filestr{mockable_fopen_function(load_path, "rb")}; + FILE* filestr{opts.mockable_fopen_function(load_path, "rb")}; AutoFile file(filestr); if (file.IsNull()) { LogPrintf("Failed to open mempool file from disk. Continuing anyway.\n"); @@ -53,7 +52,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active int64_t failed = 0; int64_t already_there = 0; int64_t unbroadcast = 0; - auto now = NodeClock::now(); + const auto now{NodeClock::now()}; try { uint64_t version; @@ -82,8 +81,12 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active file >> nTime; file >> nFeeDelta; + if (opts.use_current_time) { + nTime = TicksSinceEpoch(now); + } + CAmount amountdelta = nFeeDelta; - if (amountdelta) { + if (amountdelta && opts.apply_fee_delta_priority) { pool.PrioritiseTransaction(tx->GetHash(), amountdelta); } if (nTime > TicksSinceEpoch(now - pool.m_expiry)) { @@ -111,17 +114,21 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active std::map mapDeltas; file >> mapDeltas; - for (const auto& i : mapDeltas) { - pool.PrioritiseTransaction(i.first, i.second); + if (opts.apply_fee_delta_priority) { + 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); + if (opts.apply_unbroadcast_set) { + 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()); diff --git a/src/kernel/mempool_persist.h b/src/kernel/mempool_persist.h index cb09119e4acf..e124a8eadf8f 100644 --- a/src/kernel/mempool_persist.h +++ b/src/kernel/mempool_persist.h @@ -12,15 +12,21 @@ class CTxMemPool; namespace kernel { -/** Dump the mempool to disk. */ +/** Dump the mempool to a file. */ 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. */ +struct ImportMempoolOptions { + fsbridge::FopenFn mockable_fopen_function{fsbridge::fopen}; + bool use_current_time{false}; + bool apply_fee_delta_priority{true}; + bool apply_unbroadcast_set{true}; +}; +/** Import the file and attempt to add its contents to the mempool. */ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active_chainstate, - fsbridge::FopenFn mockable_fopen_function = fsbridge::fopen); + ImportMempoolOptions&& opts); } // namespace kernel diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 5bb1e99fe34c..a944a753ca26 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -915,7 +915,7 @@ class ImportingNow } }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const fs::path& mempool_path) +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles) { ScheduleBatchPriority(); @@ -986,7 +986,6 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile return; } } // End scope of ImportingNow - chainman.ActiveChainstate().LoadMempool(mempool_path); g_indexes_ready_to_sync = true; } } // namespace node diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 8a06e7788393..f70b1b6731b4 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -267,7 +267,7 @@ class BlockManager void CleanupBlockRevFiles() const; }; -void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles, const fs::path& mempool_path); +void ThreadImport(ChainstateManager& chainman, std::vector vImportFiles); } // namespace node #endif // BITCOIN_NODE_BLOCKSTORAGE_H diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 39a5f85f0129..83a634c0bcba 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -260,6 +260,10 @@ static const CRPCConvertParam vRPCConvertParams[] = { "importaddress", 2, "rescan" }, { "importaddress", 3, "p2sh" }, { "importpubkey", 2, "rescan" }, + { "importmempool", 1, "options" }, + { "importmempool", 1, "apply_fee_delta_priority" }, + { "importmempool", 1, "use_current_time" }, + { "importmempool", 1, "apply_unbroadcast_set" }, { "importmulti", 0, "requests" }, { "importmulti", 1, "options" }, { "importmulti", 1, "rescan" }, diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index d0a471d58dad..29475d3314ab 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -701,7 +701,7 @@ static RPCHelpMan getmempoolinfo() RPCResult{ RPCResult::Type::OBJ, "", "", { - {RPCResult::Type::BOOL, "loaded", "True if the mempool is fully loaded"}, + {RPCResult::Type::BOOL, "loaded", "True if the initial load attempt of the persisted mempool finished"}, {RPCResult::Type::NUM, "size", "Current tx count"}, {RPCResult::Type::NUM, "bytes", "Sum of all transaction sizes"}, {RPCResult::Type::NUM, "usage", "Total memory usage for the mempool"}, @@ -727,6 +727,66 @@ static RPCHelpMan getmempoolinfo() }; } +static RPCHelpMan importmempool() +{ + return RPCHelpMan{ + "importmempool", + "Import a mempool.dat file and attempt to add its contents to the mempool.\n" + "Warning: Importing untrusted files is dangerous, especially if metadata from the file is taken over.", + { + {"filepath", RPCArg::Type::STR, RPCArg::Optional::NO, "The mempool file"}, + {"options", + RPCArg::Type::OBJ_NAMED_PARAMS, + RPCArg::Optional::OMITTED, + "", + { + {"use_current_time", RPCArg::Type::BOOL, RPCArg::Default{true}, + "Whether to use the current system time or use the entry time metadata from the mempool file.\n" + "Warning: Importing untrusted metadata may lead to unexpected issues and undesirable behavior."}, + {"apply_fee_delta_priority", RPCArg::Type::BOOL, RPCArg::Default{false}, + "Whether to apply the fee delta metadata from the mempool file.\n" + "It will be added to any existing fee deltas.\n" + "The fee delta can be set by the prioritisetransaction RPC.\n" + "Warning: Importing untrusted metadata may lead to unexpected issues and undesirable behavior.\n" + "Only set this bool if you understand what it does."}, + {"apply_unbroadcast_set", RPCArg::Type::BOOL, RPCArg::Default{false}, + "Whether to apply the unbroadcast set metadata from the mempool file.\n" + "Warning: Importing untrusted metadata may lead to unexpected issues and undesirable behavior."}, + }, + RPCArgOptions{.oneline_description = "\"options\""}}, + }, + RPCResult{RPCResult::Type::OBJ, "", "", std::vector{}}, + RPCExamples{HelpExampleCli("importmempool", "/path/to/mempool.dat") + HelpExampleRpc("importmempool", "/path/to/mempool.dat")}, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + const NodeContext& node{EnsureAnyNodeContext(request.context)}; + + CTxMemPool& mempool{EnsureMemPool(node)}; + Chainstate& chainstate = EnsureChainman(node).ActiveChainstate(); + + if (chainstate.IsInitialBlockDownload()) { + throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Can only import the mempool after the block download and sync is done."); + } + + const fs::path load_path{fs::u8path(request.params[0].get_str())}; + const UniValue& use_current_time{request.params[1]["use_current_time"]}; + const UniValue& apply_fee_delta{request.params[1]["apply_fee_delta_priority"]}; + const UniValue& apply_unbroadcast{request.params[1]["apply_unbroadcast_set"]}; + kernel::ImportMempoolOptions opts{ + .use_current_time = use_current_time.isNull() ? true : use_current_time.get_bool(), + .apply_fee_delta_priority = apply_fee_delta.isNull() ? false : apply_fee_delta.get_bool(), + .apply_unbroadcast_set = apply_unbroadcast.isNull() ? false : apply_unbroadcast.get_bool(), + }; + + if (!kernel::LoadMempool(mempool, load_path, chainstate, std::move(opts))) { + throw JSONRPCError(RPC_MISC_ERROR, "Unable to import mempool file, see debug.log for details."); + } + + UniValue ret{UniValue::VOBJ}; + return ret; + }, + }; +} + static RPCHelpMan savemempool() { return RPCHelpMan{"savemempool", @@ -912,6 +972,7 @@ void RegisterMempoolRPCCommands(CRPCTable& t) {"blockchain", &gettxspendingprevout}, {"blockchain", &getmempoolinfo}, {"blockchain", &getrawmempool}, + {"blockchain", &importmempool}, {"blockchain", &savemempool}, {"hidden", &submitpackage}, }; diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 775ce0e5d7dc..c8a8de622f1c 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -79,6 +79,7 @@ const std::vector RPC_COMMANDS_NOT_SAFE_FOR_FUZZING{ "generatetoaddress", // avoid prohibitively slow execution (when `num_blocks` is large) "generatetodescriptor", // avoid prohibitively slow execution (when `nblocks` is large) "gettxoutproof", // avoid prohibitively slow execution + "importmempool", // avoid reading from disk "importwallet", // avoid reading from disk "loadwallet", // avoid reading from disk "savemempool", // disabled as a precautionary measure: may take a file path argument in the future diff --git a/src/test/fuzz/validation_load_mempool.cpp b/src/test/fuzz/validation_load_mempool.cpp index c203dd4e390a..5d020b4d5938 100644 --- a/src/test/fuzz/validation_load_mempool.cpp +++ b/src/test/fuzz/validation_load_mempool.cpp @@ -20,6 +20,7 @@ #include using kernel::DumpMempool; +using kernel::LoadMempool; using node::MempoolPath; @@ -47,6 +48,10 @@ FUZZ_TARGET(validation_load_mempool, .init = initialize_validation_load_mempool) auto fuzzed_fopen = [&](const fs::path&, const char*) { return fuzzed_file_provider.open(); }; - (void)chainstate.LoadMempool(MempoolPath(g_setup->m_args), fuzzed_fopen); + (void)LoadMempool(pool, MempoolPath(g_setup->m_args), chainstate, + { + .mockable_fopen_function = fuzzed_fopen, + }); + pool.SetLoadTried(true); (void)DumpMempool(pool, MempoolPath(g_setup->m_args), fuzzed_fopen, true); } diff --git a/src/txmempool.h b/src/txmempool.h index c7b7ed72f6fd..9262d1bd07dd 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -699,13 +699,13 @@ class CTxMemPool void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants, size_t* ancestorsize = nullptr, CAmount* ancestorfees = nullptr) const; /** - * @returns true if we've made an attempt to load the mempool regardless of + * @returns true if an initial attempt to load the persisted mempool was made, regardless of * whether the attempt was successful or not */ bool GetLoadTried() const; /** - * Set whether or not we've made an attempt to load the mempool (regardless + * Set whether or not an initial attempt to load the persisted mempool was made (regardless * of whether the attempt was successful or not) */ void SetLoadTried(bool load_tried); diff --git a/src/validation.cpp b/src/validation.cpp index f63a3ad06684..2094d567d85f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -91,7 +91,6 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; using kernel::ComputeUTXOStats; -using kernel::LoadMempool; using kernel::Notifications; using fsbridge::FopenFn; @@ -5174,13 +5173,6 @@ void PruneBlockFilesManual(Chainstate& active_chainstate, int nManualPruneHeight } } -void Chainstate::LoadMempool(const fs::path& load_path, FopenFn mockable_fopen_function) -{ - if (!m_mempool) return; - ::LoadMempool(*m_mempool, load_path, *this, mockable_fopen_function); - m_mempool->SetLoadTried(!ShutdownRequested()); -} - bool Chainstate::LoadChainTip() { AssertLockHeld(cs_main); diff --git a/src/validation.h b/src/validation.h index a75f9695cb5d..ce18bc8664f5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -761,9 +761,6 @@ class Chainstate /** Find the last common block of this chain and a locator. */ const CBlockIndex* FindForkInGlobalIndex(const CBlockLocator& locator) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); - /** Load the persisted mempool from disk */ - 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); diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index dbb9c0258f4c..8a16bc1e3440 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -44,7 +44,7 @@ assert_equal, assert_greater_than_or_equal, assert_raises_rpc_error, ) -from test_framework.wallet import MiniWallet +from test_framework.wallet import MiniWallet, COIN class MempoolPersistTest(BitcoinTestFramework): @@ -157,6 +157,16 @@ def run_test(self): assert self.nodes[0].getmempoolinfo()["loaded"] assert_equal(len(self.nodes[0].getrawmempool()), 0) + self.log.debug("Import mempool at runtime to node0.") + assert_equal({}, self.nodes[0].importmempool(mempooldat0)) + assert_equal(len(self.nodes[0].getrawmempool()), 7) + fees = self.nodes[0].getmempoolentry(txid=last_txid)["fees"] + assert_equal(fees["base"], fees["modified"]) + assert_equal({}, self.nodes[0].importmempool(mempooldat0, {"apply_fee_delta_priority": True, "apply_unbroadcast_set": True})) + assert_equal(2, self.nodes[0].getmempoolinfo()["unbroadcastcount"]) + fees = self.nodes[0].getmempoolentry(txid=last_txid)["fees"] + assert_equal(fees["base"] + Decimal("0.00001000"), fees["modified"]) + self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.") self.stop_nodes() self.start_node(0) @@ -184,6 +194,7 @@ def run_test(self): assert_raises_rpc_error(-1, "Unable to dump mempool to disk", self.nodes[1].savemempool) os.rmdir(mempooldotnew1) + self.test_importmempool_union() self.test_persist_unbroadcast() def test_persist_unbroadcast(self): @@ -208,6 +219,46 @@ def test_persist_unbroadcast(self): # node0.mockscheduler(16 * 60) # 15 min + 1 for buffer # self.wait_until(lambda: len(conn.get_invs()) == 1) + def test_importmempool_union(self): + self.log.debug("Submit different transactions to node0 and node1's mempools") + self.start_node(0) + self.start_node(2) + tx_node0 = self.mini_wallet.send_self_transfer(from_node=self.nodes[0]) + tx_node1 = self.mini_wallet.send_self_transfer(from_node=self.nodes[1]) + tx_node01 = self.mini_wallet.create_self_transfer() + tx_node01_secret = self.mini_wallet.create_self_transfer() + self.nodes[0].prioritisetransaction(tx_node01["txid"], COIN) + self.nodes[0].prioritisetransaction(tx_node01_secret["txid"], 2 * COIN) + self.nodes[1].prioritisetransaction(tx_node01_secret["txid"], 3 * COIN) + self.nodes[0].sendrawtransaction(tx_node01["hex"]) + self.nodes[1].sendrawtransaction(tx_node01["hex"]) + assert tx_node0["txid"] in self.nodes[0].getrawmempool() + assert not tx_node0["txid"] in self.nodes[1].getrawmempool() + assert not tx_node1["txid"] in self.nodes[0].getrawmempool() + assert tx_node1["txid"] in self.nodes[1].getrawmempool() + assert tx_node01["txid"] in self.nodes[0].getrawmempool() + assert tx_node01["txid"] in self.nodes[1].getrawmempool() + assert not tx_node01_secret["txid"] in self.nodes[0].getrawmempool() + assert not tx_node01_secret["txid"] in self.nodes[1].getrawmempool() + + self.log.debug("Check that importmempool can add txns without replacing the entire mempool") + mempooldat0 = str(self.nodes[0].chain_path / "mempool.dat") + result0 = self.nodes[0].savemempool() + assert_equal(mempooldat0, result0["filename"]) + assert_equal({}, self.nodes[1].importmempool(mempooldat0, {"apply_fee_delta_priority": True})) + # All transactions should be in node1's mempool now. + assert tx_node0["txid"] in self.nodes[1].getrawmempool() + assert tx_node1["txid"] in self.nodes[1].getrawmempool() + assert not tx_node1["txid"] in self.nodes[0].getrawmempool() + # For transactions that already existed, priority should be changed + entry_node01 = self.nodes[1].getmempoolentry(tx_node01["txid"]) + assert_equal(entry_node01["fees"]["base"] + 1, entry_node01["fees"]["modified"]) + # Deltas for not-yet-submitted transactions should be applied as well (prioritisation is stackable). + self.nodes[1].sendrawtransaction(tx_node01_secret["hex"]) + entry_node01_secret = self.nodes[1].getmempoolentry(tx_node01_secret["txid"]) + assert_equal(entry_node01_secret["fees"]["base"] + 5, entry_node01_secret["fees"]["modified"]) + self.stop_nodes() + if __name__ == "__main__": MempoolPersistTest().main() From cf354cc4149815dfcf31d7de3250b2a6b3e7a7d9 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 18 Oct 2023 10:31:42 +0100 Subject: [PATCH 3/9] Merge bitcoin/bitcoin#28637: doc: add release note for #27460 (new `importmempool` RPC) 1b672eb7665cc032f2c285b1cad331dc92685265 doc: add release note for #27460 (new `importmempool` RPC) (Sebastian Falbesoner) Pull request description: This PR adds a missing release note for #27460. ACKs for top commit: glozow: ACK 1b672eb7665cc032f2c285b1cad331dc92685265 Tree-SHA512: 89deadbfd6779e6eb19801c9fe7459a9876b920d44e09df102774c1eb8b3c0716462613dc99d1711eda4bd959ea61595b33f4528424ac02cf1af6cb4e5f1f0e9 Co-authored-by: glozow --- doc/release-notes-27460.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 doc/release-notes-27460.md diff --git a/doc/release-notes-27460.md b/doc/release-notes-27460.md new file mode 100644 index 000000000000..bc29e2d3779e --- /dev/null +++ b/doc/release-notes-27460.md @@ -0,0 +1,7 @@ +- A new `importmempool` RPC has been added. It loads a valid `mempool.dat` file and attempts to + add its contents to the mempool. This can be useful to import mempool data from another node + without having to modify the datadir contents and without having to restart the node. (#8071) + - Warning: Importing untrusted files is dangerous, especially if metadata from the file is taken over. + - If you want to apply fee deltas, it is recommended to use the `getprioritisedtransactions` and + `prioritisetransaction` RPCs instead of the `apply_fee_delta_priority` option to avoid + double-prioritising any already-prioritised transactions in the mempool. From 7ec810dcd2a67d45c9c1b991b3c3953f4e193e7d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 18 Aug 2023 09:46:19 +0100 Subject: [PATCH 4/9] Merge bitcoin/bitcoin#28289: rpc: remove one more quote from non-string oneline description 239431444216850b63ecf01c3b5c5d6d24230d08 rpc: remove one more quote from non-string oneline description (Martin Zumsande) Pull request description: This fixes a silent conflict between https://github.com/bitcoin/bitcoin/pull/28123 (which removed all `\"options\"`) and https://github.com/bitcoin/bitcoin/pull/27460 (which added a new one). It should fix the current CI failures. ACKs for top commit: ajtowns: utACK 239431444216850b63ecf01c3b5c5d6d24230d08 MarcoFalke: lgtm ACK 239431444216850b63ecf01c3b5c5d6d24230d08 jonatack: ACK 239431444216850b63ecf01c3b5c5d6d24230d08 hebasto: ACK 239431444216850b63ecf01c3b5c5d6d24230d08 Tree-SHA512: feb0c2b936a77be45d9c65aa7d738277b2266b5153665fee3b1413045de521195dc7d5efa2fc8b37b22f16e9b8d0ee8de25bfd151a428666122b31f64056557a Co-authored-by: fanquake --- src/rpc/mempool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 29475d3314ab..3887a465b434 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -753,7 +753,7 @@ static RPCHelpMan importmempool() "Whether to apply the unbroadcast set metadata from the mempool file.\n" "Warning: Importing untrusted metadata may lead to unexpected issues and undesirable behavior."}, }, - RPCArgOptions{.oneline_description = "\"options\""}}, + RPCArgOptions{.oneline_description = "options"}}, }, RPCResult{RPCResult::Type::OBJ, "", "", std::vector{}}, RPCExamples{HelpExampleCli("importmempool", "/path/to/mempool.dat") + HelpExampleRpc("importmempool", "/path/to/mempool.dat")}, From 47e08b9051967cb86f96fb320f7e945b6b83b651 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Mon, 21 Aug 2023 10:41:36 +0100 Subject: [PATCH 5/9] Merge bitcoin/bitcoin#28218: refactor: Make IsInitialBlockDownload & NotifyHeaderTip not require a Chainstate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 94a98fbd1d14a4ea10629b917771d50f3191e944 assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ChainstateManager (Ryan Ofsky) Pull request description: This change makes `IsInitialBlockDownload` and `NotifyHeaderTip` functions no longer tied to individual `Chainstate` objects. It makes them work with the `ChainstateManager` object instead so code is simpler and it is no longer possible to call them incorrectly with an inactive `Chainstate`. This change also makes `m_cached_finished_ibd` caching easier to reason about, because now there is only one cached value instead of two (for background and snapshot chainstates) so the cached IBD state now no longer gets reset when a snapshot is loaded. There should be no change in behavior because these functions were always called on the active `ChainState` objects. These changes were discussed previously https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1246868905 and https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1237552792 as possible followups for that PR. ACKs for top commit: MarcoFalke: re-ACK 94a98fbd1d14a4ea10629b917771d50f3191e944 🐺 naumenkogs: ACK 94a98fbd1d14a4ea10629b917771d50f3191e944 dergoegge: reACK 94a98fbd1d14a4ea10629b917771d50f3191e944 Tree-SHA512: 374d6e5c9bbc7564c143f634bd709a4e8f4a42c8d77e7a8554c832acdcf60fa2a134f3ea10827db1a1e0191006496329c0ebf5c64f3ab868398c3722bb7ff56f Co-authored-by: fanquake --- src/bench/net_processing.cpp | 6 +- src/bitcoin-chainstate.cpp | 2 +- src/init.cpp | 4 +- src/net_processing.cpp | 14 ++--- src/node/interfaces.cpp | 7 ++- src/rpc/blockchain.cpp | 2 +- src/rpc/mempool.cpp | 5 +- src/rpc/mining.cpp | 2 +- src/test/fuzz/process_message.cpp | 4 +- src/test/fuzz/process_messages.cpp | 4 +- src/test/governance_inv_tests.cpp | 42 ++++++------- src/test/llmq_blockprocessor_tests.cpp | 2 +- src/test/llmq_chainlock_tests.cpp | 2 +- src/test/net_tests.cpp | 53 ++++++++-------- src/test/util/validation.cpp | 4 +- src/test/util/validation.h | 2 +- .../validation_chainstatemanager_tests.cpp | 12 +++- src/validation.cpp | 63 ++++++++++--------- src/validation.h | 23 +++---- 19 files changed, 129 insertions(+), 124 deletions(-) diff --git a/src/bench/net_processing.cpp b/src/bench/net_processing.cpp index cdb9b671d2be..ba78bf0f0f7d 100644 --- a/src/bench/net_processing.cpp +++ b/src/bench/net_processing.cpp @@ -15,8 +15,8 @@ using namespace std::literals; static void InventoryBatch(benchmark::Bench& bench, uint32_t count) { const auto setup = MakeNoLogFileContext(ChainType::REGTEST); - auto& chainstate = *static_cast(&setup->m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*setup->m_node.chainman); + chainman.JumpOutOfIbd(); auto& peerman = *setup->m_node.peerman; const auto& connman = *static_cast(setup->m_node.connman.get()); auto peer{MakeTestPeer(/*id=*/0)}; @@ -44,7 +44,7 @@ static void InventoryBatch(benchmark::Bench& bench, uint32_t count) }); peerman.FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index e8dc69795298..884cc872e318 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -182,7 +182,7 @@ int main(int argc, char* argv[]) << "\t" << "Reindexing: " << std::boolalpha << node::fReindex.load() << std::noboolalpha << std::endl << "\t" << "Snapshot Active: " << std::boolalpha << chainman.IsSnapshotActive() << std::noboolalpha << std::endl << "\t" << "Active Height: " << chainman.ActiveHeight() << std::endl - << "\t" << "Active IBD: " << std::boolalpha << chainman.ActiveChainstate().IsInitialBlockDownload() << std::noboolalpha << std::endl; + << "\t" << "Active IBD: " << std::boolalpha << chainman.IsInitialBlockDownload() << std::noboolalpha << std::endl; CBlockIndex* tip = chainman.ActiveTip(); if (tip) { std::cout << "\t" << tip->ToString() << std::endl; diff --git a/src/init.cpp b/src/init.cpp index 6b4c9c636c6b..fc6f22d7617e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2500,7 +2500,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) { const CBlockIndex* tip = WITH_LOCK(::cs_main, return chainman.ActiveTip()); - const bool ibd = chainman.ActiveChainstate().IsInitialBlockDownload(); + const bool ibd = chainman.IsInitialBlockDownload(); if (node.active_ctx) { // On masternodes, defer the full broadcast until after // nodeman->Init() so that GetProTxHash() is available @@ -2591,7 +2591,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // directly because the full broadcast (NetQuorum, // ActiveContext) depends on a valid proTxHash. const CBlockIndex* tip = WITH_LOCK(::cs_main, return chainman.ActiveTip()); - const bool ibd = chainman.ActiveChainstate().IsInitialBlockDownload(); + const bool ibd = chainman.IsInitialBlockDownload(); GetMainSignals().InitializeCurrentBlockTip(tip, ibd); } }); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 10ae4ebe2874..f001f6d2f795 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2400,7 +2400,7 @@ void PeerManagerImpl::BlockChecked(const CBlock& block, const BlockValidationSta // the tip yet so we have no way to check this directly here. Instead we // just check that there are currently no other blocks in flight. else if (state.IsValid() && - !m_chainman.ActiveChainstate().IsInitialBlockDownload() && + !m_chainman.IsInitialBlockDownload() && mapBlocksInFlight.count(hash) == mapBlocksInFlight.size()) { if (it != mapBlockSource.end()) { MaybeSetPeerAsAnnouncingHeaderAndIDs(it->second.first); @@ -3359,7 +3359,7 @@ void PeerManagerImpl::UpdatePeerStateForReceivedHeaders(CNode& pfrom, Peer& peer // If we're in IBD, we want outbound peers that will serve us a useful // chain. Disconnect peers that are on chains with insufficient work. - if (m_chainman.ActiveChainstate().IsInitialBlockDownload() && !may_have_more_headers) { + if (m_chainman.IsInitialBlockDownload() && !may_have_more_headers) { // If the peer has no more headers to give us, then we know we have // their tip. if (nodestate->pindexBestKnownBlock && nodestate->pindexBestKnownBlock->nChainWork < m_chainman.MinimumChainWork()) { @@ -4674,7 +4674,7 @@ void PeerManagerImpl::ProcessMessage( return; } bool allowWhileInIBD = allowWhileInIBDObjs.count(inv.type); - if (allowWhileInIBD || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) { + if (allowWhileInIBD || !m_chainman.IsInitialBlockDownload()) { AddObjectAnnouncement(pfrom, inv, current_time); } } @@ -4964,7 +4964,7 @@ void PeerManagerImpl::ProcessMessage( // Stop processing the transaction early if we are still in IBD since we don't // have enough information to validate it yet. Sending unsolicited transactions // is not considered a protocol violation, so don't punish the peer. - if (m_chainman.ActiveChainstate().IsInitialBlockDownload()) return; + if (m_chainman.IsInitialBlockDownload()) return; CTransactionRef ptx; CCoinJoinBroadcastTx dstx; @@ -5167,7 +5167,7 @@ void PeerManagerImpl::ProcessMessage( if (!m_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.hashPrevBlock)) { // Doesn't connect (or is genesis), instead of DoSing in AcceptBlockHeader, request deeper headers - if (!m_chainman.ActiveChainstate().IsInitialBlockDownload()) { + if (!m_chainman.IsInitialBlockDownload()) { std::string ret_val = UsesCompressedHeaders(*peer) ? NetMsgType::GETHEADERS2 : NetMsgType::GETHEADERS; MaybeSendGetHeaders(pfrom, ret_val, GetLocator(m_chainman.m_best_header), *peer); } @@ -6314,7 +6314,7 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros LOCK(peer.m_addr_send_times_mutex); // Periodically advertise our local address to the peer. - if (fListen && !m_chainman.ActiveChainstate().IsInitialBlockDownload() && + if (fListen && !m_chainman.IsInitialBlockDownload() && peer.m_next_local_addr_send < current_time) { // If we've sent before, clear the bloom filter for the peer, so that our // self-announcement will actually go out. @@ -6940,7 +6940,7 @@ bool PeerManagerImpl::SendMessages(CNode* pto) // // Message: getdata (blocks) // - if (CanServeBlocks(*peer) && pto->CanRelay() && ((sync_blocks_and_headers_from_peer && !IsLimitedPeer(*peer)) || !m_chainman.ActiveChainstate().IsInitialBlockDownload()) && state.vBlocksInFlight.size() < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { + if (CanServeBlocks(*peer) && pto->CanRelay() && ((sync_blocks_and_headers_from_peer && !IsLimitedPeer(*peer)) || !m_chainman.IsInitialBlockDownload()) && state.vBlocksInFlight.size() < MAX_BLOCKS_IN_TRANSIT_PER_PEER) { std::vector vToDownload; NodeId staller = -1; FindNextBlocksToDownload(*peer, MAX_BLOCKS_IN_TRANSIT_PER_PEER - state.vBlocksInFlight.size(), vToDownload, staller); diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index a12196d561e9..09d9b824642b 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1128,8 +1128,9 @@ class NodeImpl : public Node { return GuessVerificationProgress(chainman().GetParams().TxData(), WITH_LOCK(::cs_main, return chainman().ActiveChain().Tip())); } - bool isInitialBlockDownload() override { - return chainman().ActiveChainstate().IsInitialBlockDownload(); + bool isInitialBlockDownload() override + { + return chainman().IsInitialBlockDownload(); } bool isMasternode() override { @@ -1651,7 +1652,7 @@ class ChainImpl : public Chain bool isReadyToBroadcast() override { return !chainman().m_blockman.LoadingBlocks() && !isInitialBlockDownload(); } bool isInitialBlockDownload() override { - return chainman().ActiveChainstate().IsInitialBlockDownload(); + return chainman().IsInitialBlockDownload(); } bool shutdownRequested() override { return ShutdownRequested(); } void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9eeae575a609..57d7126f42fe 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1606,7 +1606,7 @@ RPCHelpMan getblockchaininfo() obj.pushKV("time", tip.GetBlockTime()); obj.pushKV("mediantime", tip.GetMedianTimePast()); obj.pushKV("verificationprogress", GuessVerificationProgress(chainman.GetParams().TxData(), &tip)); - obj.pushKV("initialblockdownload", active_chainstate.IsInitialBlockDownload()); + obj.pushKV("initialblockdownload", chainman.IsInitialBlockDownload()); obj.pushKV("chainwork", tip.nChainWork.GetHex()); obj.pushKV("size_on_disk", chainman.m_blockman.CalculateCurrentUsage()); obj.pushKV("pruned", chainman.m_blockman.IsPruneMode()); diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index 3887a465b434..4c07da282038 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -761,9 +761,10 @@ static RPCHelpMan importmempool() const NodeContext& node{EnsureAnyNodeContext(request.context)}; CTxMemPool& mempool{EnsureMemPool(node)}; - Chainstate& chainstate = EnsureChainman(node).ActiveChainstate(); + ChainstateManager& chainman = EnsureChainman(node); + Chainstate& chainstate = chainman.ActiveChainstate(); - if (chainstate.IsInitialBlockDownload()) { + if (chainman.IsInitialBlockDownload()) { throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Can only import the mempool after the block download and sync is done."); } diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index bfb0025b7774..fc6483620e8e 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -781,7 +781,7 @@ static RPCHelpMan getblocktemplate() throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!"); } - if (active_chainstate.IsInitialBlockDownload()) { + if (chainman.IsInitialBlockDownload()) { throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, PACKAGE_NAME " is in initial sync and waiting for blocks..."); } diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index f16d0535df2a..c1c0a0820c0a 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -53,9 +53,9 @@ FUZZ_TARGET(process_message, .init = initialize_process_message) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); ConnmanTestMsg& connman = *static_cast(g_setup->m_node.connman.get()); - TestChainState& chainstate = *static_cast(&g_setup->m_node.chainman->ActiveChainstate()); + auto& chainman = static_cast(*g_setup->m_node.chainman); SetMockTime(1610000000); // any time to successfully reset ibd - chainstate.ResetIbd(); + chainman.ResetIbd(); LOCK(NetEventsInterface::g_msgproc_mutex); diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index f0293ae80eb0..ac02d4240919 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -38,9 +38,9 @@ FUZZ_TARGET(process_messages, .init = initialize_process_messages) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); ConnmanTestMsg& connman = *static_cast(g_setup->m_node.connman.get()); - TestChainState& chainstate = *static_cast(&g_setup->m_node.chainman->ActiveChainstate()); + auto& chainman = static_cast(*g_setup->m_node.chainman); SetMockTime(1610000000); // any time to successfully reset ibd - chainstate.ResetIbd(); + chainman.ResetIbd(); LOCK(NetEventsInterface::g_msgproc_mutex); diff --git a/src/test/governance_inv_tests.cpp b/src/test/governance_inv_tests.cpp index f1b21d04775f..d1cc18d8f29b 100644 --- a/src/test/governance_inv_tests.cpp +++ b/src/test/governance_inv_tests.cpp @@ -334,9 +334,8 @@ BOOST_AUTO_TEST_CASE(governance_objects_require_peer_announcement_or_request) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); NetGovernance net_gov(m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, *m_node.netfulfilledman, *m_node.connman); @@ -385,16 +384,15 @@ BOOST_AUTO_TEST_CASE(governance_objects_require_peer_announcement_or_request) m_node.peerman->FinalizeNode(*announcing_peer); m_node.peerman->FinalizeNode(*second_announcing_peer); m_node.peerman->FinalizeNode(*unsolicited_peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); } BOOST_AUTO_TEST_CASE(governance_objects_wait_for_the_collateral_index) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); NetGovernance net_gov(m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, *m_node.netfulfilledman, *m_node.connman); @@ -474,16 +472,15 @@ BOOST_AUTO_TEST_CASE(governance_objects_wait_for_the_collateral_index) m_node.peerman->FinalizeNode(*flooding_peer); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); } BOOST_AUTO_TEST_CASE(governance_votes_require_peer_announcement_or_request) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); NetGovernance net_gov(m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, *m_node.netfulfilledman, *m_node.connman); @@ -527,7 +524,7 @@ BOOST_AUTO_TEST_CASE(governance_votes_require_peer_announcement_or_request) m_node.peerman->FinalizeNode(*announcing_peer); m_node.peerman->FinalizeNode(*second_announcing_peer); m_node.peerman->FinalizeNode(*unsolicited_peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); } // A message received while not blockchain-synced is dropped, but the per-peer authorization must @@ -536,9 +533,8 @@ BOOST_AUTO_TEST_CASE(governance_vote_authorization_survives_unsynced_drop) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); NetGovernance net_gov(m_node.peerman.get(), *m_node.govman, *m_node.mn_sync, *m_node.netfulfilledman, *m_node.connman); @@ -570,7 +566,7 @@ BOOST_AUTO_TEST_CASE(governance_vote_authorization_survives_unsynced_drop) BOOST_CHECK_EQUAL(MisbehaviorScore(*m_node.peerman, *peer), 20); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); } // A vote whose parent object is unknown must prove masternode authorship before it is cached. @@ -578,9 +574,8 @@ BOOST_AUTO_TEST_CASE(orphan_votes_require_a_valid_masternode_signature) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); // Penalties are applied only once fully synced. m_node.mn_sync->SwitchToNextAsset(); @@ -606,7 +601,7 @@ BOOST_AUTO_TEST_CASE(orphan_votes_require_a_valid_masternode_signature) BOOST_CHECK_EQUAL(MisbehaviorScore(*m_node.peerman, *peer), 20); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); } // The same unauthenticated vote must cost the sender the same whether or not its parent object @@ -615,9 +610,8 @@ BOOST_AUTO_TEST_CASE(invalid_vote_is_scored_alike_with_and_without_a_parent_obje { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); m_node.mn_sync->SwitchToNextAsset(); BOOST_REQUIRE(m_node.mn_sync->IsSynced()); @@ -647,7 +641,7 @@ BOOST_AUTO_TEST_CASE(invalid_vote_is_scored_alike_with_and_without_a_parent_obje m_node.peerman->FinalizeNode(*orphan_peer); m_node.peerman->FinalizeNode(*known_parent_peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); } // Every orphan parent hash is re-requested from every peer on each governance tick, and one diff --git a/src/test/llmq_blockprocessor_tests.cpp b/src/test/llmq_blockprocessor_tests.cpp index 46ccfc1f1f16..fc12bc0b810f 100644 --- a/src/test/llmq_blockprocessor_tests.cpp +++ b/src/test/llmq_blockprocessor_tests.cpp @@ -52,7 +52,7 @@ BOOST_FIXTURE_TEST_CASE(unrequested_qfcommit_is_dropped_and_scored, TestChain100 // INV announcements for non-spork objects are only tracked outside IBD; the 100 mined blocks // of this fixture already take us out of it. - BOOST_REQUIRE(!m_node.chainman->ActiveChainstate().IsInitialBlockDownload()); + BOOST_REQUIRE(!m_node.chainman->IsInitialBlockDownload()); // Every Dash-specific message is offered to CMNAuth first, which asserts a loaded metadata // manager. The fixture leaves it unloaded, so initialise an empty cache here. diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index 253dd677a53f..69e8ed10554e 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -490,7 +490,7 @@ BOOST_FIXTURE_TEST_CASE(unrequested_clsig_is_dropped_and_scored, TestChain100Set // INV announcements for non-spork objects are only tracked outside IBD; the 100 mined blocks // of this fixture already take us out of it. - BOOST_REQUIRE(!m_node.chainman->ActiveChainstate().IsInitialBlockDownload()); + BOOST_REQUIRE(!m_node.chainman->IsInitialBlockDownload()); // Every Dash-specific message is offered to CMNAuth first, which asserts a loaded metadata // manager. The fixture leaves it unloaded, so initialise an empty cache here. diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 91fa78c70d21..64d96d1238b0 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -80,8 +80,8 @@ BOOST_AUTO_TEST_CASE(cnode_listen_port) BOOST_AUTO_TEST_CASE(inventory_request_accounting) { LOCK(NetEventsInterface::g_msgproc_mutex); - auto& chainstate = *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; auto fallback{MakeTestPeer(/*id=*/1)}; auto& peerman = *m_node.peerman; @@ -133,7 +133,7 @@ BOOST_AUTO_TEST_CASE(inventory_request_accounting) BOOST_CHECK_EQUAL(peerman.GetRequestedObjectCount(peer->GetId()), objects.size()); peerman.FinalizeNode(*peer); peerman.FinalizeNode(*fallback); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -197,9 +197,8 @@ BOOST_AUTO_TEST_CASE(peer_requested_object_authorizes_and_erases_per_peer_state) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; m_node.peerman->InitializeNode(*peer, NODE_NETWORK); @@ -239,7 +238,7 @@ BOOST_AUTO_TEST_CASE(peer_requested_object_authorizes_and_erases_per_peer_state) BOOST_CHECK(WITH_LOCK(::cs_main, return m_node.peerman->PeerConsumeObjectRequest(peer->GetId(), unsolicited_inv))); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -251,9 +250,8 @@ BOOST_AUTO_TEST_CASE(peer_getdata_response_requires_an_inflight_request) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; m_node.peerman->InitializeNode(*peer, NODE_NETWORK); @@ -279,7 +277,7 @@ BOOST_AUTO_TEST_CASE(peer_getdata_response_requires_an_inflight_request) BOOST_CHECK_EQUAL(m_node.peerman->GetRequestedObjectCount(peer->GetId()), 0U); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -292,8 +290,8 @@ BOOST_AUTO_TEST_CASE(expired_getdata_response_is_late_not_unrequested) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; m_node.peerman->InitializeNode(*peer, NODE_NETWORK); @@ -325,7 +323,7 @@ BOOST_AUTO_TEST_CASE(expired_getdata_response_is_late_not_unrequested) GetDataResponse::UNREQUESTED); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -336,8 +334,8 @@ BOOST_AUTO_TEST_CASE(forgotten_getdata_response_is_late_not_unrequested) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; m_node.peerman->InitializeNode(*peer, NODE_NETWORK); @@ -357,7 +355,7 @@ BOOST_AUTO_TEST_CASE(forgotten_getdata_response_is_late_not_unrequested) BOOST_CHECK(ConsumeGetDataResponse(*m_node.peerman, *peer, inv) == GetDataResponse::UNREQUESTED); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -370,8 +368,8 @@ BOOST_AUTO_TEST_CASE(getdata_response_grace_does_not_cross_inv_types) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; m_node.peerman->InitializeNode(*peer, NODE_NETWORK); @@ -391,7 +389,7 @@ BOOST_AUTO_TEST_CASE(getdata_response_grace_does_not_cross_inv_types) GetDataResponse::UNREQUESTED); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -401,8 +399,8 @@ BOOST_AUTO_TEST_CASE(getdata_response_grace_expires) { LOCK(NetEventsInterface::g_msgproc_mutex); - TestChainState& chainstate = *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); auto peer{MakeTestPeer(/*id=*/0)}; m_node.peerman->InitializeNode(*peer, NODE_NETWORK); @@ -421,7 +419,7 @@ BOOST_AUTO_TEST_CASE(getdata_response_grace_expires) BOOST_CHECK(ConsumeGetDataResponse(*m_node.peerman, *peer, inv) == GetDataResponse::UNREQUESTED); m_node.peerman->FinalizeNode(*peer); - chainstate.ResetIbd(); + chainman.ResetIbd(); SetMockTime(0s); } @@ -1224,11 +1222,10 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) const int64_t time{0}; const CNetMsgMaker msg_maker{PROTOCOL_VERSION}; - // Force Chainstate::IsInitialBlockDownload() to return false. + // Force ChainstateManager::IsInitialBlockDownload() to return false. // Otherwise PushAddress() isn't called by PeerManager::ProcessMessage(). - TestChainState& chainstate = - *static_cast(&m_node.chainman->ActiveChainstate()); - chainstate.JumpOutOfIbd(); + auto& chainman = static_cast(*m_node.chainman); + chainman.JumpOutOfIbd(); m_node.peerman->InitializeNode(peer, NODE_NETWORK); @@ -1278,7 +1275,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message) BOOST_CHECK(sent); CaptureMessage = CaptureMessageOrig; - chainstate.ResetIbd(); + chainman.ResetIbd(); m_node.args->ForceSetArg("-capturemessages", "0"); m_node.args->ForceSetArg("-bind", ""); // PeerManager::ProcessMessage() calls AddTimeData() which changes the internal state diff --git a/src/test/util/validation.cpp b/src/test/util/validation.cpp index c0d7a532dcd6..2d5562ae66c1 100644 --- a/src/test/util/validation.cpp +++ b/src/test/util/validation.cpp @@ -9,13 +9,13 @@ #include #include -void TestChainState::ResetIbd() +void TestChainstateManager::ResetIbd() { m_cached_finished_ibd = false; assert(IsInitialBlockDownload()); } -void TestChainState::JumpOutOfIbd() +void TestChainstateManager::JumpOutOfIbd() { Assert(IsInitialBlockDownload()); m_cached_finished_ibd = true; diff --git a/src/test/util/validation.h b/src/test/util/validation.h index 7a511a2b79e0..64654f3fb613 100644 --- a/src/test/util/validation.h +++ b/src/test/util/validation.h @@ -9,7 +9,7 @@ class CValidationInterface; -struct TestChainState : public Chainstate { +struct TestChainstateManager : public ChainstateManager { /** Reset the ibd cache to its initial state */ void ResetIbd(); /** Toggle IsInitialBlockDownload from true to false */ diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 304386d600dc..31ad1ef3f15a 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -192,14 +193,21 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_rebalance_caches, TestChain100Setup) c2.InitCoinsDB( /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false); + // Reset IBD state so IsInitialBlockDownload() returns true and causes + // MaybeRebalancesCaches() to prioritize the snapshot chainstate, giving it + // more cache space than the snapshot chainstate. Calling ResetIbd() is + // necessary because m_cached_finished_ibd is already latched to true before + // the test starts due to the test setup. After ResetIbd() is called. + // IsInitialBlockDownload will return true because at this point the active + // chainstate has a null chain tip. + static_cast(manager).ResetIbd(); + { LOCK(::cs_main); c2.InitCoinsCache(1 << 23); manager.MaybeRebalanceCaches(); } - // Since both chainstates are considered to be in initial block download, - // the snapshot chainstate should take priority. BOOST_CHECK_CLOSE(c1.m_coinstip_cache_size_bytes, max_cache * 0.05, 1); BOOST_CHECK_CLOSE(c1.m_coinsdb_cache_size_bytes, max_cache * 0.05, 1); BOOST_CHECK_CLOSE(c2.m_coinstip_cache_size_bytes, max_cache * 0.95, 1); diff --git a/src/validation.cpp b/src/validation.cpp index 2094d567d85f..accd95d5bf61 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -413,8 +413,9 @@ static void LimitMempoolSize(CTxMemPool& pool, CCoinsViewCache& coins_cache) static bool IsCurrentForFeeEstimation(Chainstate& active_chainstate) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); - if (active_chainstate.IsInitialBlockDownload()) + if (active_chainstate.m_chainman.IsInitialBlockDownload()) { return false; + } if (active_chainstate.m_chain.Tip()->GetBlockTime() < count_seconds(GetTime() - MAX_FEE_ESTIMATION_TIP_AGE)) return false; if (active_chainstate.m_chain.Height() < active_chainstate.m_chainman.m_best_header->nHeight - 1) { @@ -2008,7 +2009,7 @@ void Chainstate::InitCoinsCache(size_t cache_size_bytes) // `const` so that `CValidationInterface` clients (which are given a `const Chainstate*`) // can call it. // -bool Chainstate::IsInitialBlockDownload() const +bool ChainstateManager::IsInitialBlockDownload() const { // Optimization: pre-test latch before taking the lock. if (m_cached_finished_ibd.load(std::memory_order_relaxed)) @@ -2017,15 +2018,17 @@ bool Chainstate::IsInitialBlockDownload() const LOCK(cs_main); if (m_cached_finished_ibd.load(std::memory_order_relaxed)) return false; - if (m_chainman.m_blockman.LoadingBlocks()) { + if (m_blockman.LoadingBlocks()) { return true; } - if (m_chain.Tip() == nullptr) + CChain& chain{ActiveChain()}; + if (chain.Tip() == nullptr) { return true; - if (m_chain.Tip()->nChainWork < m_chainman.MinimumChainWork()) { + } + if (chain.Tip()->nChainWork < MinimumChainWork()) { return true; } - if (m_chain.Tip()->Time() < NodeClock::now() - m_chainman.m_options.max_tip_age) { + if (chain.Tip()->Time() < NodeClock::now() - m_options.max_tip_age) { return true; } LogPrintf("Leaving InitialBlockDownload (latching to false)\n"); @@ -2039,7 +2042,7 @@ void Chainstate::CheckForkWarningConditions() // Before we get past initial download, we cannot reliably alert about forks // (we assume we don't get stuck on a fork before finishing our initial sync) - if (IsInitialBlockDownload()) { + if (m_chainman.IsInitialBlockDownload()) { return; } @@ -3003,7 +3006,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // DASH : CHECK TRANSACTIONS FOR INSTANTSEND - if (m_isman != nullptr && !IsInitialBlockDownload()) { + if (m_isman != nullptr && !m_chainman.IsInitialBlockDownload()) { // Require other nodes to comply, send them some data in case they are missing it. const bool has_chainlock = m_chainlocks.HasChainLock(pindex->nHeight, pindex->GetBlockHash()); for (const auto& tx : block.vtx) { @@ -3211,7 +3214,7 @@ bool Chainstate::FlushStateToDisk( } else { LOG_TIME_MILLIS_WITH_CATEGORY("find files to prune", BCLog::BENCHMARK); - m_blockman.FindFilesToPrune(setFilesToPrune, m_chainman.GetParams().PruneAfterHeight(), m_chain.Height(), last_prune, IsInitialBlockDownload()); + m_blockman.FindFilesToPrune(setFilesToPrune, m_chainman.GetParams().PruneAfterHeight(), m_chain.Height(), last_prune, m_chainman.IsInitialBlockDownload()); m_blockman.m_check_for_pruning = false; } if (!setFilesToPrune.empty()) { @@ -3409,8 +3412,7 @@ void Chainstate::UpdateTip(const CBlockIndex* pindexNew) } bilingual_str warning_messages; - if (!this->IsInitialBlockDownload()) - { + if (!m_chainman.IsInitialBlockDownload()) { const CBlockIndex* pindex = pindexNew; for (int bit = 0; bit < VERSIONBITS_NUM_BITS; bit++) { WarningBitsConditionChecker checker(m_chainman, bit); @@ -3882,24 +3884,25 @@ static SynchronizationState GetSynchronizationState(bool init) return SynchronizationState::INIT_DOWNLOAD; } -static bool NotifyHeaderTip(Chainstate& chainstate) LOCKS_EXCLUDED(cs_main) { +static bool NotifyHeaderTip(ChainstateManager& chainman) LOCKS_EXCLUDED(cs_main) +{ bool fNotify = false; bool fInitialBlockDownload = false; static CBlockIndex* pindexHeaderOld = nullptr; CBlockIndex* pindexHeader = nullptr; { LOCK(cs_main); - pindexHeader = chainstate.m_chainman.m_best_header; + pindexHeader = chainman.m_best_header; if (pindexHeader != pindexHeaderOld) { fNotify = true; - fInitialBlockDownload = chainstate.IsInitialBlockDownload(); + fInitialBlockDownload = chainman.IsInitialBlockDownload(); pindexHeaderOld = pindexHeader; } } // Send block tip changed notifications without cs_main if (fNotify) { - chainstate.m_chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload), pindexHeader->nHeight, pindexHeader->nTime, false); + chainman.GetNotifications().headerTip(GetSynchronizationState(fInitialBlockDownload), pindexHeader->nHeight, pindexHeader->nTime, false); GetMainSignals().NotifyHeaderTip(pindexHeader, fInitialBlockDownload); } return fNotify; @@ -4003,7 +4006,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< if (!blocks_connected) return true; const CBlockIndex* pindexFork = m_chain.FindFork(starting_tip); - bool fInitialDownload = IsInitialBlockDownload(); + bool fInitialDownload = m_chainman.IsInitialBlockDownload(); // Notify external listeners about the new tip. // Enqueue while holding cs_main to ensure that UpdatedBlockTip is called in the order in which blocks are connected @@ -4224,14 +4227,14 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde InvalidChainFound(to_mark_failed); if (this == &m_chainman.ActiveChainstate()) { - GetMainSignals().SynchronousUpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); - GetMainSignals().UpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); + GetMainSignals().SynchronousUpdatedBlockTip(m_chain.Tip(), nullptr, m_chainman.IsInitialBlockDownload()); + GetMainSignals().UpdatedBlockTip(m_chain.Tip(), nullptr, m_chainman.IsInitialBlockDownload()); } } // Only notify about a new block tip if the active chain was modified. if (pindex_was_in_chain) { - m_chainman.GetNotifications().blockTip(GetSynchronizationState(IsInitialBlockDownload()), *to_mark_failed->pprev); + m_chainman.GetNotifications().blockTip(GetSynchronizationState(m_chainman.IsInitialBlockDownload()), *to_mark_failed->pprev); } return true; } @@ -4329,13 +4332,13 @@ bool Chainstate::MarkConflictingBlock(BlockValidationState& state, CBlockIndex * ConflictingChainFound(pindex); if (this == &m_chainman.ActiveChainstate()) { - GetMainSignals().SynchronousUpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); - GetMainSignals().UpdatedBlockTip(m_chain.Tip(), nullptr, IsInitialBlockDownload()); + GetMainSignals().SynchronousUpdatedBlockTip(m_chain.Tip(), nullptr, m_chainman.IsInitialBlockDownload()); + GetMainSignals().UpdatedBlockTip(m_chain.Tip(), nullptr, m_chainman.IsInitialBlockDownload()); } // Only notify about a new block tip if the active chain was modified. if (pindex_was_in_chain) { - m_chainman.GetNotifications().blockTip(GetSynchronizationState(IsInitialBlockDownload()), *pindex->pprev); + m_chainman.GetNotifications().blockTip(GetSynchronizationState(m_chainman.IsInitialBlockDownload()), *pindex->pprev); } return true; } @@ -4905,7 +4908,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida const auto msg = strprintf( "Saw new header hash=%s height=%d", hash.ToString(), pindex->nHeight); - if (ActiveChainstate().IsInitialBlockDownload()) { + if (IsInitialBlockDownload()) { LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Debug, "%s\n", msg); } else { LogPrintf("%s\n", msg); @@ -4935,8 +4938,8 @@ bool ChainstateManager::ProcessNewBlockHeaders(const std::vector& } } } - if (NotifyHeaderTip(ActiveChainstate())) { - if (ActiveChainstate().IsInitialBlockDownload() && ppindex && *ppindex) { + if (NotifyHeaderTip(*this)) { + if (IsInitialBlockDownload() && ppindex && *ppindex) { const CBlockIndex& last_accepted{**ppindex}; const int64_t blocks_left{(GetTime() - last_accepted.GetBlockTime()) / GetConsensus().nPowTargetSpacing}; const double progress{100.0 * last_accepted.nHeight / (last_accepted.nHeight + blocks_left)}; @@ -5019,7 +5022,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& pblock, // Header is valid/has work, merkle tree is good...RELAY NOW // (but if it does not build on our best tip, let the SendMessages loop relay it) - if (!ActiveChainstate().IsInitialBlockDownload() && ActiveTip() == pindex->pprev) + if (!IsInitialBlockDownload() && ActiveTip() == pindex->pprev) GetMainSignals().NewPoWValidBlock(pindex, pblock); // Write block to history file @@ -5085,7 +5088,7 @@ bool ChainstateManager::ProcessNewBlock(const std::shared_ptr& blo } } - NotifyHeaderTip(ActiveChainstate()); + NotifyHeaderTip(*this); BlockValidationState state; // Only used to report errors, not invalidity - ignore it if (!ActiveChainstate().ActivateBestChain(state, block)) { @@ -5745,7 +5748,7 @@ void ChainstateManager::LoadExternalBlockFile( } } - NotifyHeaderTip(ActiveChainstate()); + NotifyHeaderTip(*this); if (!blocks_with_unknown_parent) continue; @@ -5772,7 +5775,7 @@ void ChainstateManager::LoadExternalBlockFile( } range.first++; blocks_with_unknown_parent->erase(it); - NotifyHeaderTip(ActiveChainstate()); + NotifyHeaderTip(*this); } } } catch (const std::exception& e) { @@ -6930,7 +6933,7 @@ void ChainstateManager::MaybeRebalanceCaches() // If both chainstates exist, determine who needs more cache based on IBD status. // // Note: shrink caches first so that we don't inadvertently overwhelm available memory. - if (m_snapshot_chainstate->IsInitialBlockDownload()) { + if (IsInitialBlockDownload()) { m_ibd_chainstate->ResizeCoinsCaches( m_total_coinstip_cache * 0.05, m_total_coinsdb_cache * 0.05); m_snapshot_chainstate->ResizeCoinsCaches( diff --git a/src/validation.h b/src/validation.h index ce18bc8664f5..80a4149e9166 100644 --- a/src/validation.h +++ b/src/validation.h @@ -493,14 +493,6 @@ class Chainstate */ Mutex m_chainstate_mutex; - /** - * Whether this chainstate is undergoing initial block download. - * - * Mutable because we need to be able to mark IsInitialBlockDownload() - * const, which latches this for caching purposes. - */ - mutable std::atomic m_cached_finished_ibd{false}; - //! Optional mempool that is kept in sync with the chain. //! Only the active chainstate has a mempool. CTxMemPool* m_mempool; @@ -755,9 +747,6 @@ class Chainstate void ClearBlockIndexCandidates() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - /** Check whether we are doing an initial block download (synchronizing from disk or network) */ - bool IsInitialBlockDownload() const; - /** Find the last common block of this chain and a locator. */ const CBlockIndex* FindForkInGlobalIndex(const CBlockLocator& locator) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -1008,6 +997,15 @@ class ChainstateManager //! chainstate to avoid duplicating block metadata. node::BlockManager m_blockman; + /** + * Whether initial block download has ended and IsInitialBlockDownload + * should return false from now on. + * + * Mutable because we need to be able to mark IsInitialBlockDownload() + * const, which latches this for caching purposes. + */ + mutable std::atomic m_cached_finished_ibd{false}; + /** * Every received block is assigned a unique and increasing identifier, so we * know which one to give priority in case of a fork. @@ -1140,6 +1138,9 @@ class ChainstateManager //! Whether active-state-dependent masternode duties must remain disabled. bool IsSnapshotActiveAndUnvalidated() const; + /** Check whether we are doing an initial block download (synchronizing from disk or network) */ + bool IsInitialBlockDownload() const; + /** * Import blocks from an external file * From 549000d578ccaa28b4d568afe396d3f519a3325c Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 22 Aug 2023 10:16:20 +0100 Subject: [PATCH 6/9] Merge bitcoin/bitcoin#28200: refactor: Remove unused includes from wallet.cpp fa6286891fa4164510e4fbf4bc214ce3033b2d1b Remove unused includes from wallet.cpp (MarcoFalke) fa8fdbe22932a4717d2bc4060269da9bff228728 Remove unused includes from blockfilter.h (MarcoFalke) fad8c36aa9011c3f7b1183f8380577e16a2167a6 move-only: Create src/kernel/mempool_removal_reason.h (MarcoFalke) fa5760880094c4e4238249f6d1837cd74383cc3a Remove unused includes from txmempool.h (MarcoFalke) Pull request description: This makes compilation of wallet.cpp use a few % less memory and time, locally. Created in the context of https://github.com/bitcoin/bitcoin/issues/28109, but I don't think it is enough to actually fix this problem. ACKs for top commit: hebasto: ACK fa6286891fa4164510e4fbf4bc214ce3033b2d1b, I have reviewed the code and it looks OK. Tree-SHA512: 06f1120af2a8ef3368dbd9ae747acda88ace2507bd261bcc10341d476a0b3d71c8485377ea6c108b47df3e4c13b7f75a15f486bafa6a8466303168dde16ebbc8 Co-authored-by: fanquake --- src/Makefile.am | 3 ++ src/blockfilter.cpp | 2 ++ src/blockfilter.h | 13 +++++--- src/index/blockfilterindex.cpp | 1 + src/index/blockfilterindex.h | 2 ++ src/kernel/mempool_removal_reason.cpp | 21 +++++++++++++ src/kernel/mempool_removal_reason.h | 24 +++++++++++++++ src/node/miner.h | 2 ++ src/test/blockfilter_tests.cpp | 2 ++ src/test/util/blockfilter.cpp | 2 ++ src/txmempool.cpp | 12 -------- src/txmempool.h | 44 +++++++++------------------ src/wallet/test/util.cpp | 1 + src/wallet/test/wallet_tests.cpp | 1 + src/wallet/wallet.cpp | 40 ++++++++++++++++++++++-- src/wallet/wallet.h | 42 ++++++++++++++++++++----- 16 files changed, 156 insertions(+), 56 deletions(-) create mode 100644 src/kernel/mempool_removal_reason.cpp create mode 100644 src/kernel/mempool_removal_reason.h diff --git a/src/Makefile.am b/src/Makefile.am index e6684ec9c2ae..c728766fb564 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -311,6 +311,7 @@ BITCOIN_CORE_H = \ kernel/mempool_limits.h \ kernel/mempool_options.h \ kernel/mempool_persist.h \ + kernel/mempool_removal_reason.h \ kernel/notifications_interface.h \ kernel/validation_cache_sizes.h \ key.h \ @@ -618,6 +619,7 @@ libbitcoin_node_a_SOURCES = \ kernel/cs_main.cpp \ kernel/disconnected_transactions.cpp \ kernel/mempool_persist.cpp \ + kernel/mempool_removal_reason.cpp \ llmq/blockprocessor.cpp \ llmq/commitment.cpp \ llmq/context.cpp \ @@ -1342,6 +1344,7 @@ libdashkernel_la_SOURCES = \ kernel/cs_main.cpp \ kernel/disconnected_transactions.cpp \ kernel/mempool_persist.cpp \ + kernel/mempool_removal_reason.cpp \ key.cpp \ key_io.cpp \ llmq/blockprocessor.cpp \ diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index 496ec24fd2a5..f256d81f39ee 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -9,9 +9,11 @@ #include #include #include +#include #include #include