From bb8ae2c41941d0776968f75cec0de89247e952dc Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 1 Feb 2019 14:09:36 -0800 Subject: rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json And use it to fix a race condition in mempool_persist.py: https://travis-ci.org/Empact/bitcoin/jobs/487577243 Since e.g. getrawmempool returns errors based on this status, this enables users to test it for readiness. --- doc/REST-interface.md | 1 + src/rpc/blockchain.cpp | 2 ++ test/functional/mempool_persist.py | 17 +++++++++-------- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/doc/REST-interface.md b/doc/REST-interface.md index 02a665008b..c96871ab5f 100644 --- a/doc/REST-interface.md +++ b/doc/REST-interface.md @@ -101,6 +101,7 @@ $ curl localhost:18332/rest/getutxos/checkmempool/b2cdfd7b89def827ff8af7cd9bff76 Returns various information about the TX mempool. Only supports JSON as output format. +* loaded : (boolean) if the mempool is fully loaded * size : (numeric) the number of transactions in the TX mempool * bytes : (numeric) size of the TX mempool in bytes * usage : (numeric) total TX mempool memory usage diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 72fec2eaa5..cebb331f38 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1484,6 +1484,7 @@ static UniValue getchaintips(const JSONRPCRequest& request) UniValue MempoolInfoToJSON(const CTxMemPool& pool) { UniValue ret(UniValue::VOBJ); + ret.pushKV("loaded", g_is_mempool_loaded); ret.pushKV("size", (int64_t)pool.size()); ret.pushKV("bytes", (int64_t)pool.GetTotalTxSize()); ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage()); @@ -1504,6 +1505,7 @@ static UniValue getmempoolinfo(const JSONRPCRequest& request) {}, RPCResult{ "{\n" + " \"loaded\": true|false (boolean) True if the mempool is fully loaded\n" " \"size\": xxxxx, (numeric) Current tx count\n" " \"bytes\": xxxxx, (numeric) Sum of all virtual transaction sizes as defined in BIP 141. Differs from actual serialized size because witness data is discounted\n" " \"usage\": xxxxx, (numeric) Total memory usage for the mempool\n" diff --git a/test/functional/mempool_persist.py b/test/functional/mempool_persist.py index d74d4eaaf1..bb0169ee52 100755 --- a/test/functional/mempool_persist.py +++ b/test/functional/mempool_persist.py @@ -37,7 +37,6 @@ Test is as follows: """ from decimal import Decimal import os -import time from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_raises_rpc_error, wait_until @@ -83,9 +82,10 @@ class MempoolPersistTest(BitcoinTestFramework): self.start_node(1, extra_args=["-persistmempool=0"]) self.start_node(0) self.start_node(2) - # Give bitcoind a second to reload the mempool - wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5, timeout=1) - wait_until(lambda: len(self.nodes[2].getrawmempool()) == 5, timeout=1) + wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"], timeout=1) + wait_until(lambda: self.nodes[2].getmempoolinfo()["loaded"], timeout=1) + assert_equal(len(self.nodes[0].getrawmempool()), 5) + assert_equal(len(self.nodes[2].getrawmempool()), 5) # The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now: assert_equal(len(self.nodes[1].getrawmempool()), 0) @@ -100,14 +100,14 @@ class MempoolPersistTest(BitcoinTestFramework): self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.") self.stop_nodes() self.start_node(0, extra_args=["-persistmempool=0"]) - # Give bitcoind a second to reload the mempool - time.sleep(1) + wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"]) assert_equal(len(self.nodes[0].getrawmempool()), 0) self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.") self.stop_nodes() self.start_node(0) - wait_until(lambda: len(self.nodes[0].getrawmempool()) == 5) + wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"]) + assert_equal(len(self.nodes[0].getrawmempool()), 5) mempooldat0 = os.path.join(self.nodes[0].datadir, 'regtest', 'mempool.dat') mempooldat1 = os.path.join(self.nodes[1].datadir, 'regtest', 'mempool.dat') @@ -120,7 +120,8 @@ class MempoolPersistTest(BitcoinTestFramework): os.rename(mempooldat0, mempooldat1) self.stop_nodes() self.start_node(1, extra_args=[]) - wait_until(lambda: len(self.nodes[1].getrawmempool()) == 5) + wait_until(lambda: self.nodes[1].getmempoolinfo()["loaded"]) + assert_equal(len(self.nodes[1].getrawmempool()), 5) self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails") # to test the exception we are creating a tmp folder called mempool.dat.new -- cgit v1.2.3 From effe81f7503d2ca3c88cfdea687f9f997f353e0d Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Thu, 7 Mar 2019 09:54:44 -0500 Subject: Move g_is_mempool_loaded into CTxMemPool::m_is_loaded So the loaded state is explicitly mempool-specific. --- src/init.cpp | 8 ++++---- src/rpc/blockchain.cpp | 6 +++--- src/txmempool.cpp | 12 ++++++++++++ src/txmempool.h | 8 ++++++++ src/validation.cpp | 19 +++++++++---------- src/validation.h | 5 ++--- 6 files changed, 38 insertions(+), 20 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index ee9a451d0b..cf3be0556e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -235,8 +235,8 @@ void Shutdown(InitInterfaces& interfaces) g_banman.reset(); g_txindex.reset(); - if (g_is_mempool_loaded && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - DumpMempool(); + if (::mempool.IsLoaded() && gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { + DumpMempool(::mempool); } if (fFeeEstimatesInitialized) @@ -725,9 +725,9 @@ static void ThreadImport(std::vector vImportFiles) } } // End scope of CImportingNow if (gArgs.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) { - LoadMempool(); + LoadMempool(::mempool); } - g_is_mempool_loaded = !ShutdownRequested(); + ::mempool.SetIsLoaded(!ShutdownRequested()); } /** Sanity checks diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index cebb331f38..1c20feba12 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1484,7 +1484,7 @@ static UniValue getchaintips(const JSONRPCRequest& request) UniValue MempoolInfoToJSON(const CTxMemPool& pool) { UniValue ret(UniValue::VOBJ); - ret.pushKV("loaded", g_is_mempool_loaded); + ret.pushKV("loaded", pool.IsLoaded()); ret.pushKV("size", (int64_t)pool.size()); ret.pushKV("bytes", (int64_t)pool.GetTotalTxSize()); ret.pushKV("usage", (int64_t)pool.DynamicMemoryUsage()); @@ -2056,11 +2056,11 @@ static UniValue savemempool(const JSONRPCRequest& request) }.ToString()); } - if (!g_is_mempool_loaded) { + if (!::mempool.IsLoaded()) { throw JSONRPCError(RPC_MISC_ERROR, "The mempool was not loaded yet"); } - if (!DumpMempool()) { + if (!DumpMempool(::mempool)) { throw JSONRPCError(RPC_MISC_ERROR, "Unable to dump mempool to disk"); } diff --git a/src/txmempool.cpp b/src/txmempool.cpp index ca556bdc7b..8a414f5dec 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1090,4 +1090,16 @@ void CTxMemPool::GetTransactionAncestry(const uint256& txid, size_t& ancestors, } } +bool CTxMemPool::IsLoaded() const +{ + LOCK(cs); + return m_is_loaded; +} + +void CTxMemPool::SetIsLoaded(bool loaded) +{ + LOCK(cs); + m_is_loaded = loaded; +} + SaltedTxidHasher::SaltedTxidHasher() : k0(GetRand(std::numeric_limits::max())), k1(GetRand(std::numeric_limits::max())) {} diff --git a/src/txmempool.h b/src/txmempool.h index a8a0f7fa45..3ada47a28e 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -455,6 +455,8 @@ private: void trackPackageRemoved(const CFeeRate& rate) EXCLUSIVE_LOCKS_REQUIRED(cs); + bool m_is_loaded GUARDED_BY(cs){false}; + public: static const int ROLLING_FEE_HALFLIFE = 60 * 60 * 12; // public only for testing @@ -672,6 +674,12 @@ public: */ void GetTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) const; + /** @returns true if the mempool is fully loaded */ + bool IsLoaded() const; + + /** Sets the current loaded state */ + void SetIsLoaded(bool loaded); + unsigned long size() const { LOCK(cs); diff --git a/src/validation.cpp b/src/validation.cpp index 358992b74d..68dea5dc8d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -251,7 +251,6 @@ CAmount maxTxFee = DEFAULT_TRANSACTION_MAXFEE; CBlockPolicyEstimator feeEstimator; CTxMemPool mempool(&feeEstimator); -std::atomic_bool g_is_mempool_loaded{false}; /** Constant stuff for coinbase transactions we create: */ CScript COINBASE_FLAGS; @@ -4648,7 +4647,7 @@ int VersionBitsTipStateSinceHeight(const Consensus::Params& params, Consensus::D static const uint64_t MEMPOOL_DUMP_VERSION = 1; -bool LoadMempool() +bool LoadMempool(CTxMemPool& pool) { const CChainParams& chainparams = Params(); int64_t nExpiryTimeout = gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60; @@ -4683,12 +4682,12 @@ bool LoadMempool() CAmount amountdelta = nFeeDelta; if (amountdelta) { - mempool.PrioritiseTransaction(tx->GetHash(), amountdelta); + pool.PrioritiseTransaction(tx->GetHash(), amountdelta); } CValidationState state; if (nTime + nExpiryTimeout > nNow) { LOCK(cs_main); - AcceptToMemoryPoolWithTime(chainparams, mempool, state, tx, nullptr /* pfMissingInputs */, nTime, + AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, nullptr /* pfMissingInputs */, nTime, nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */, false /* test_accept */); if (state.IsValid()) { @@ -4698,7 +4697,7 @@ bool LoadMempool() // wallet(s) having loaded it while we were processing // mempool transactions; consider these as valid, instead of // failed, but mark them as 'already there' - if (mempool.exists(tx->GetHash())) { + if (pool.exists(tx->GetHash())) { ++already_there; } else { ++failed; @@ -4714,7 +4713,7 @@ bool LoadMempool() file >> mapDeltas; for (const auto& i : mapDeltas) { - mempool.PrioritiseTransaction(i.first, i.second); + pool.PrioritiseTransaction(i.first, i.second); } } catch (const std::exception& e) { LogPrintf("Failed to deserialize mempool data on disk: %s. Continuing anyway.\n", e.what()); @@ -4725,7 +4724,7 @@ bool LoadMempool() return true; } -bool DumpMempool() +bool DumpMempool(const CTxMemPool& pool) { int64_t start = GetTimeMicros(); @@ -4736,11 +4735,11 @@ bool DumpMempool() LOCK(dump_mutex); { - LOCK(mempool.cs); - for (const auto &i : mempool.mapDeltas) { + LOCK(pool.cs); + for (const auto &i : pool.mapDeltas) { mapDeltas[i.first] = i.second; } - vinfo = mempool.infoAll(); + vinfo = pool.infoAll(); } int64_t mid = GetTimeMicros(); diff --git a/src/validation.h b/src/validation.h index 4ef82ee2c7..0ff6c619b5 100644 --- a/src/validation.h +++ b/src/validation.h @@ -149,7 +149,6 @@ extern CScript COINBASE_FLAGS; extern CCriticalSection cs_main; extern CBlockPolicyEstimator feeEstimator; extern CTxMemPool mempool; -extern std::atomic_bool g_is_mempool_loaded; typedef std::unordered_map BlockMap; extern BlockMap& mapBlockIndex GUARDED_BY(cs_main); extern const std::string strMessageMagic; @@ -486,10 +485,10 @@ static const unsigned int REJECT_HIGHFEE = 0x100; CBlockFileInfo* GetBlockFileInfo(size_t n); /** Dump the mempool to disk. */ -bool DumpMempool(); +bool DumpMempool(const CTxMemPool& pool); /** Load the mempool from disk. */ -bool LoadMempool(); +bool LoadMempool(CTxMemPool& pool); //! Check whether the block associated with this index entry is pruned or not. inline bool IsBlockPruned(const CBlockIndex* pblockindex) -- cgit v1.2.3