validation: make CChainState::m_mempool optional

Since we now have multiple chainstate objects, only one of them is active at any given
time. An active chainstate has a mempool, but there's no point to others having one.

This change will simplify proposed assumeutxo semantics. See the discussion here:
https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905

Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
This commit is contained in:
James O'Beirne
2021-07-06 17:05:25 -04:00
parent 088b348dbe
commit 617661703a
7 changed files with 64 additions and 40 deletions

View File

@@ -1349,7 +1349,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
const int64_t load_block_index_start_time = GetTimeMillis();
try {
LOCK(cs_main);
chainman.InitializeChainstate(*Assert(node.mempool));
chainman.InitializeChainstate(Assert(node.mempool.get()));
chainman.m_total_coinstip_cache = nCoinCacheUsage;
chainman.m_total_coinsdb_cache = nCoinDBCache;

View File

@@ -180,7 +180,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
// instead of unit tests, but for now we need these here.
RegisterAllCoreRPCCommands(tableRPC);
m_node.chainman->InitializeChainstate(*m_node.mempool);
m_node.chainman->InitializeChainstate(m_node.mempool.get());
m_node.chainman->ActiveChainstate().InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
assert(!m_node.chainman->ActiveChainstate().CanFlushToDisk());

View File

@@ -35,7 +35,7 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches)
return outp;
};
CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(mempool));
CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool));
c1.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23));

View File

@@ -36,7 +36,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
// Create a legacy (IBD) chainstate.
//
CChainState& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(mempool));
CChainState& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(&mempool));
chainstates.push_back(&c1);
c1.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -66,7 +66,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager)
//
const uint256 snapshot_blockhash = GetRandHash();
CChainState& c2 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(
mempool, snapshot_blockhash));
&mempool, snapshot_blockhash));
chainstates.push_back(&c2);
BOOST_CHECK_EQUAL(manager.SnapshotBlockhash().value(), snapshot_blockhash);
@@ -129,7 +129,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
// Create a legacy (IBD) chainstate.
//
CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(mempool));
CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool));
chainstates.push_back(&c1);
c1.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);
@@ -147,7 +147,7 @@ BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches)
// Create a snapshot-based chainstate.
//
CChainState& c2 = WITH_LOCK(cs_main, return manager.InitializeChainstate(mempool, GetRandHash()));
CChainState& c2 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool, GetRandHash()));
chainstates.push_back(&c2);
c2.InitCoinsDB(
/* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false);

View File

@@ -20,7 +20,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate)
{
CTxMemPool mempool;
BlockManager blockman{};
CChainState chainstate{mempool, blockman};
CChainState chainstate{&mempool, blockman};
chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false);
WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10));
CTxMemPool tx_pool{};

View File

@@ -329,7 +329,8 @@ static bool IsCurrentForFeeEstimation(CChainState& active_chainstate) EXCLUSIVE_
return true;
}
/* Make mempool consistent after a reorg, by re-adding or recursively erasing
/**
* Make mempool consistent after a reorg, by re-adding or recursively erasing
* disconnected block transactions from the mempool, and also removing any
* other transactions from the mempool that are no longer valid given the new
* tip/height.
@@ -1208,7 +1209,7 @@ void CoinsViews::InitCache()
m_cacheview = std::make_unique<CCoinsViewCache>(&m_catcherview);
}
CChainState::CChainState(CTxMemPool& mempool, BlockManager& blockman, std::optional<uint256> from_snapshot_blockhash)
CChainState::CChainState(CTxMemPool* mempool, BlockManager& blockman, std::optional<uint256> from_snapshot_blockhash)
: m_mempool(mempool),
m_params(::Params()),
m_blockman(blockman),
@@ -2053,7 +2054,7 @@ bool CChainState::FlushStateToDisk(
bool fFlushForPrune = false;
bool fDoFullFlush = false;
CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(&m_mempool);
CoinsCacheSizeState cache_state = GetCoinsCacheSizeState(m_mempool);
LOCK(cs_LastBlockFile);
if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) {
// make sure we don't prune above the blockfilterindexes bestblocks
@@ -2205,11 +2206,13 @@ static void AppendWarning(bilingual_str& res, const bilingual_str& warn)
}
/** Check warning conditions and do some notifications on new chain tip set. */
static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const CChainParams& chainParams, CChainState& active_chainstate)
static void UpdateTip(CTxMemPool* mempool, const CBlockIndex* pindexNew, const CChainParams& chainParams, CChainState& active_chainstate)
EXCLUSIVE_LOCKS_REQUIRED(::cs_main)
{
// New best block
mempool.AddTransactionsUpdated(1);
if (mempool) {
mempool->AddTransactionsUpdated(1);
}
{
LOCK(g_best_block_mutex);
@@ -2254,7 +2257,7 @@ static void UpdateTip(CTxMemPool& mempool, const CBlockIndex* pindexNew, const C
bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_mempool.cs);
if (m_mempool) AssertLockHeld(m_mempool->cs);
CBlockIndex *pindexDelete = m_chain.Tip();
assert(pindexDelete);
@@ -2280,7 +2283,7 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr
return false;
}
if (disconnectpool) {
if (disconnectpool && m_mempool) {
// Save transactions to re-add to mempool at end of reorg
for (auto it = block.vtx.rbegin(); it != block.vtx.rend(); ++it) {
disconnectpool->addTransaction(*it);
@@ -2288,7 +2291,7 @@ bool CChainState::DisconnectTip(BlockValidationState& state, DisconnectedBlockTr
while (disconnectpool->DynamicMemoryUsage() > MAX_DISCONNECTED_TX_POOL_SIZE * 1000) {
// Drop the earliest entry, and remove its children from the mempool.
auto it = disconnectpool->queuedTx.get<insertion_order>().begin();
m_mempool.removeRecursive(**it, MemPoolRemovalReason::REORG);
m_mempool->removeRecursive(**it, MemPoolRemovalReason::REORG);
disconnectpool->removeEntry(it);
}
}
@@ -2357,7 +2360,7 @@ public:
bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_mempool.cs);
if (m_mempool) AssertLockHeld(m_mempool->cs);
assert(pindexNew->pprev == m_chain.Tip());
// Read block from disk.
@@ -2401,8 +2404,10 @@ bool CChainState::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew
int64_t nTime5 = GetTimeMicros(); nTimeChainState += nTime5 - nTime4;
LogPrint(BCLog::BENCH, " - Writing chainstate: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime5 - nTime4) * MILLI, nTimeChainState * MICRO, nTimeChainState * MILLI / nBlocksTotal);
// Remove conflicting transactions from the mempool.;
m_mempool.removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
disconnectpool.removeForBlock(blockConnecting.vtx);
if (m_mempool) {
m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
disconnectpool.removeForBlock(blockConnecting.vtx);
}
// Update m_chain & related variables.
m_chain.SetTip(pindexNew);
UpdateTip(m_mempool, pindexNew, m_params, *this);
@@ -2495,7 +2500,7 @@ void CChainState::PruneBlockIndexCandidates() {
bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace)
{
AssertLockHeld(cs_main);
AssertLockHeld(m_mempool.cs);
if (m_mempool) AssertLockHeld(m_mempool->cs);
const CBlockIndex* pindexOldTip = m_chain.Tip();
const CBlockIndex* pindexFork = m_chain.FindFork(pindexMostWork);
@@ -2507,7 +2512,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex
if (!DisconnectTip(state, &disconnectpool)) {
// This is likely a fatal error, but keep the mempool consistent,
// just in case. Only remove from the mempool in this case.
UpdateMempoolForReorg(*this, m_mempool, disconnectpool, false);
if (m_mempool) UpdateMempoolForReorg(*this, *m_mempool, disconnectpool, false);
// If we're unable to disconnect a block during normal operation,
// then that is a failure of our local system -- we should abort
@@ -2551,7 +2556,7 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex
// A system error occurred (disk space, database error, ...).
// Make the mempool consistent with the current tip, just in case
// any observers try to use it before shutdown.
UpdateMempoolForReorg(*this, m_mempool, disconnectpool, false);
if (m_mempool) UpdateMempoolForReorg(*this, *m_mempool, disconnectpool, false);
return false;
}
} else {
@@ -2565,12 +2570,12 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex
}
}
if (fBlocksDisconnected) {
if (fBlocksDisconnected && m_mempool) {
// If any blocks were disconnected, disconnectpool may be non empty. Add
// any disconnected transactions back to the mempool.
UpdateMempoolForReorg(*this, m_mempool, disconnectpool, true);
UpdateMempoolForReorg(*this, *m_mempool, disconnectpool, true);
}
m_mempool.check(*this);
if (m_mempool) m_mempool->check(*this);
CheckForkWarningConditions();
@@ -2642,7 +2647,8 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr
{
LOCK(cs_main);
LOCK(m_mempool.cs); // Lock transaction pool for at least as long as it takes for connectTrace to be consumed
// Lock transaction pool for at least as long as it takes for connectTrace to be consumed
LOCK(MempoolMutex());
CBlockIndex* starting_tip = m_chain.Tip();
bool blocks_connected = false;
do {
@@ -2792,7 +2798,9 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind
LimitValidationInterfaceQueue();
LOCK(cs_main);
LOCK(m_mempool.cs); // Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is called after DisconnectTip without unlocking in between
// Lock for as long as disconnectpool is in scope to make sure UpdateMempoolForReorg is
// called after DisconnectTip without unlocking in between
LOCK(MempoolMutex());
if (!m_chain.Contains(pindex)) break;
pindex_was_in_chain = true;
CBlockIndex *invalid_walk_tip = m_chain.Tip();
@@ -2806,7 +2814,9 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind
// transactions back to the mempool if disconnecting was successful,
// and we're not doing a very deep invalidation (in which case
// keeping the mempool up to date is probably futile anyway).
UpdateMempoolForReorg(*this, m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret);
if (m_mempool) {
UpdateMempoolForReorg(*this, *m_mempool, disconnectpool, /* fAddToMempool = */ (++disconnected <= 10) && ret);
}
if (!ret) return false;
assert(invalid_walk_tip->pprev == m_chain.Tip());
@@ -3817,10 +3827,11 @@ bool CChainState::LoadBlockIndexDB()
void CChainState::LoadMempool(const ArgsManager& args)
{
if (!m_mempool) return;
if (args.GetArg("-persistmempool", DEFAULT_PERSIST_MEMPOOL)) {
::LoadMempool(m_mempool, *this);
::LoadMempool(*m_mempool, *this);
}
m_mempool.SetIsLoaded(!ShutdownRequested());
m_mempool->SetIsLoaded(!ShutdownRequested());
}
bool CChainState::LoadChainTip()
@@ -4684,7 +4695,8 @@ std::vector<CChainState*> ChainstateManager::GetAll()
return out;
}
CChainState& ChainstateManager::InitializeChainstate(CTxMemPool& mempool, const std::optional<uint256>& snapshot_blockhash)
CChainState& ChainstateManager::InitializeChainstate(
CTxMemPool* mempool, const std::optional<uint256>& snapshot_blockhash)
{
bool is_snapshot = snapshot_blockhash.has_value();
std::unique_ptr<CChainState>& to_modify =
@@ -4763,7 +4775,7 @@ bool ChainstateManager::ActivateSnapshot(
}
auto snapshot_chainstate = WITH_LOCK(::cs_main, return std::make_unique<CChainState>(
this->ActiveChainstate().m_mempool, m_blockman, base_blockhash));
/* mempool */ nullptr, m_blockman, base_blockhash));
{
LOCK(::cs_main);
@@ -4879,7 +4891,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot(
}
const auto snapshot_cache_state = WITH_LOCK(::cs_main,
return snapshot_chainstate.GetCoinsCacheSizeState(&snapshot_chainstate.m_mempool));
return snapshot_chainstate.GetCoinsCacheSizeState(snapshot_chainstate.m_mempool));
if (snapshot_cache_state >=
CoinsCacheSizeState::CRITICAL) {

View File

@@ -587,8 +587,9 @@ protected:
*/
mutable std::atomic<bool> m_cached_finished_ibd{false};
//! mempool that is kept in sync with the chain
CTxMemPool& m_mempool;
//! Optional mempool that is kept in sync with the chain.
//! Only the active chainstate has a mempool.
CTxMemPool* m_mempool;
const CChainParams& m_params;
@@ -600,7 +601,10 @@ public:
//! CChainState instances.
BlockManager& m_blockman;
explicit CChainState(CTxMemPool& mempool, BlockManager& blockman, std::optional<uint256> from_snapshot_blockhash = std::nullopt);
explicit CChainState(
CTxMemPool* mempool,
BlockManager& blockman,
std::optional<uint256> from_snapshot_blockhash = std::nullopt);
/**
* Initialize the CoinsViews UTXO set database management data structures. The in-memory
@@ -729,7 +733,7 @@ public:
CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
// Apply the effects of a block disconnection on the UTXO set.
bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
bool DisconnectTip(BlockValidationState& state, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
// Manual block validity manipulation:
/** Mark a block as precious and reorganize.
@@ -784,8 +788,8 @@ public:
std::string ToString() EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
private:
bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
bool ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool.cs);
bool ActivateBestChainStep(BlockValidationState& state, CBlockIndex* pindexMostWork, const std::shared_ptr<const CBlock>& pblock, bool& fInvalidFound, ConnectTrace& connectTrace) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
bool ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, const std::shared_ptr<const CBlock>& pblock, ConnectTrace& connectTrace, DisconnectedBlockTransactions& disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, m_mempool->cs);
void InvalidBlockFound(CBlockIndex* pindex, const BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
CBlockIndex* FindMostWorkChain() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
@@ -798,6 +802,12 @@ private:
bool LoadBlockIndexDB() EXCLUSIVE_LOCKS_REQUIRED(cs_main);
//! Indirection necessary to make lock annotations work with an optional mempool.
RecursiveMutex* MempoolMutex() const LOCK_RETURNED(m_mempool->cs)
{
return m_mempool ? &m_mempool->cs : nullptr;
}
friend ChainstateManager;
};
@@ -907,7 +917,9 @@ public:
// constructor
//! @param[in] snapshot_blockhash If given, signify that this chainstate
//! is based on a snapshot.
CChainState& InitializeChainstate(CTxMemPool& mempool, const std::optional<uint256>& snapshot_blockhash = std::nullopt)
CChainState& InitializeChainstate(
CTxMemPool* mempool,
const std::optional<uint256>& snapshot_blockhash = std::nullopt)
EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
//! Get all chainstates currently being used.