From 4a1308410347c1c922d94c0f5d38e0ab308ceabb Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 17 May 2020 02:03:34 +0300 Subject: [PATCH 1/4] Drop dead code in DoInvalidateBlock --- src/llmq/quorums_chainlocks.cpp | 25 ++++++++----------------- src/llmq/quorums_chainlocks.h | 2 +- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 926036926f3d..9e3a47381d0b 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -518,7 +518,7 @@ void CChainLocksHandler::EnforceBestChainLock() } LogPrintf("CChainLocksHandler::%s -- CLSIG (%s) invalidates block %s\n", __func__, clsig.ToString(), jt->second->GetBlockHash().ToString()); - DoInvalidateBlock(jt->second, false); + DoInvalidateBlock(jt->second); } pindex = pindex->pprev; @@ -581,27 +581,18 @@ void CChainLocksHandler::HandleNewRecoveredSig(const llmq::CRecoveredSig& recove } // WARNING, do not hold cs while calling this method as we'll otherwise run into a deadlock -void CChainLocksHandler::DoInvalidateBlock(const CBlockIndex* pindex, bool activateBestChain) +void CChainLocksHandler::DoInvalidateBlock(const CBlockIndex* pindex) { - auto& params = Params(); - - { - LOCK(cs_main); + LOCK(cs_main); - // get the non-const pointer - CBlockIndex* pindex2 = mapBlockIndex[pindex->GetBlockHash()]; + auto& params = Params(); - CValidationState state; - if (!InvalidateBlock(state, params, pindex2)) { - LogPrintf("CChainLocksHandler::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state)); - // This should not have happened and we are in a state were it's not safe to continue anymore - assert(false); - } - } + // get the non-const pointer + CBlockIndex* pindex2 = mapBlockIndex[pindex->GetBlockHash()]; CValidationState state; - if (activateBestChain && !ActivateBestChain(state, params)) { - LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); + if (!InvalidateBlock(state, params, pindex2)) { + LogPrintf("CChainLocksHandler::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state)); // This should not have happened and we are in a state were it's not safe to continue anymore assert(false); } diff --git a/src/llmq/quorums_chainlocks.h b/src/llmq/quorums_chainlocks.h index b2296b94146f..8c210324252a 100644 --- a/src/llmq/quorums_chainlocks.h +++ b/src/llmq/quorums_chainlocks.h @@ -110,7 +110,7 @@ class CChainLocksHandler : public CRecoveredSigsListener bool InternalHasChainLock(int nHeight, const uint256& blockHash); bool InternalHasConflictingChainLock(int nHeight, const uint256& blockHash); - void DoInvalidateBlock(const CBlockIndex* pindex, bool activateBestChain); + void DoInvalidateBlock(const CBlockIndex* pindex); BlockTxs::mapped_type GetBlockTxs(const uint256& blockHash); From 1c9f6da50a5a72ea9cd96f7ea32e901ba4fadc57 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Sun, 17 May 2020 02:11:23 +0300 Subject: [PATCH 2/4] Let ActivateBestChain skip SyncWithValidationInterfaceQueue when called from IS or CL threads --- src/llmq/quorums_chainlocks.cpp | 10 +++++++--- src/llmq/quorums_instantsend.cpp | 2 +- src/validation.cpp | 10 +++++----- src/validation.h | 2 +- 4 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 9e3a47381d0b..8e63f342b2a2 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -534,9 +534,13 @@ void CChainLocksHandler::EnforceBestChainLock() activateNeeded = chainActive.Tip()->GetAncestor(currentBestChainLockBlockIndex->nHeight) != currentBestChainLockBlockIndex; } - CValidationState state; - if (activateNeeded && !ActivateBestChain(state, Params())) { - LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); + if (activateNeeded) { + CValidationState state; + if (!ActivateBestChain(state, Params(), std::shared_ptr(), false /* fSyncQueue */)) { + LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); + // This should not have happened and we are in a state were it's not safe to continue anymore + assert(false); + } } const CBlockIndex* pindexNotify = nullptr; diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index f245874f1091..3deee82de0ed 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1324,7 +1324,7 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const if (activateBestChain) { CValidationState state; - if (!ActivateBestChain(state, Params())) { + if (!ActivateBestChain(state, Params(), std::shared_ptr(), false /* fSyncQueue */)) { LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); // This should not have happened and we are in a state were it's not safe to continue anymore assert(false); diff --git a/src/validation.cpp b/src/validation.cpp index 2711359de2a3..6bb5cb5e0dd7 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -164,7 +164,7 @@ class CChainState { bool LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree); - bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock); + bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock, bool fSyncQueue = true); bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex); bool AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock); @@ -2935,7 +2935,7 @@ static void NotifyHeaderTip() { * or an activated best chain. pblock is either nullptr or a pointer to a block * that is already loaded (to avoid loading it again from disk). */ -bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { +bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock, bool fSyncQueue) { // Note that while we're often called here from ProcessNewBlock, this is // far from a guarantee. Things in the P2P/RPC will often end up calling // us in the middle of ProcessNewBlock - do not assume pblock is set @@ -2953,7 +2953,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& do { boost::this_thread::interruption_point(); - if (GetMainSignals().CallbacksPending() > 10) { + if (fSyncQueue && GetMainSignals().CallbacksPending() > 10) { // Block until the validation queue drains. This should largely // never happen in normal operation, however may happen during // reindex, causing memory blowup if we run too far ahead. @@ -3026,8 +3026,8 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& return true; } -bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { - return g_chainstate.ActivateBestChain(state, chainparams, std::move(pblock)); +bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock, bool fSyncQueue) { + return g_chainstate.ActivateBestChain(state, chainparams, std::move(pblock), fSyncQueue); } bool CChainState::PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex) diff --git a/src/validation.h b/src/validation.h index 5b1aa0adf1d9..a60bfcc667fa 100644 --- a/src/validation.h +++ b/src/validation.h @@ -279,7 +279,7 @@ bool IsInitialBlockDownload(); /** Retrieve a transaction (from memory pool, or from disk, if possible) */ bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = nullptr); /** Find the best known block, and make it the tip of the block chain */ -bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr()); +bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr(), bool fSyncQueue = true); double ConvertBitsToDouble(unsigned int nBits); CAmount GetBlockSubsidy(int nBits, int nHeight, const Consensus::Params& consensusParams, bool fSuperblockPartOnly = false); From e279acf24db75c49cf17a3780076f3b742907a7e Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 19 May 2020 05:18:27 +0300 Subject: [PATCH 3/4] Use CL's own scheduler instead of a global one --- src/init.cpp | 2 +- src/llmq/quorums_chainlocks.cpp | 10 ++++++++-- src/llmq/quorums_chainlocks.h | 5 ++++- src/llmq/quorums_init.cpp | 5 ++--- src/llmq/quorums_init.h | 3 +-- src/test/test_dash.cpp | 3 ++- 6 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index bf68900aaf93..678a5a29e56c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1832,7 +1832,7 @@ bool AppInitMain() deterministicMNManager.reset(); deterministicMNManager.reset(new CDeterministicMNManager(*evoDb)); - llmq::InitLLMQSystem(*evoDb, &scheduler, false, fReset || fReindexChainState); + llmq::InitLLMQSystem(*evoDb, false, fReset || fReindexChainState); if (fReset) { pblocktree->WriteReindexing(true); diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 8e63f342b2a2..63647f5b2c23 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -33,13 +33,17 @@ std::string CChainLockSig::ToString() const return strprintf("CChainLockSig(nHeight=%d, blockHash=%s)", nHeight, blockHash.ToString()); } -CChainLocksHandler::CChainLocksHandler(CScheduler* _scheduler) : - scheduler(_scheduler) +CChainLocksHandler::CChainLocksHandler() { + scheduler = new CScheduler(); + CScheduler::Function serviceLoop = boost::bind(&CScheduler::serviceQueue, scheduler); + scheduler_thread = new boost::thread(boost::bind(&TraceThread, "cl-scheduler", serviceLoop)); } CChainLocksHandler::~CChainLocksHandler() { + delete scheduler_thread; + delete scheduler; } void CChainLocksHandler::Start() @@ -56,6 +60,8 @@ void CChainLocksHandler::Start() void CChainLocksHandler::Stop() { quorumSigningManager->UnregisterRecoveredSigsListener(this); + scheduler_thread->interrupt(); + scheduler_thread->join(); } bool CChainLocksHandler::AlreadyHave(const CInv& inv) diff --git a/src/llmq/quorums_chainlocks.h b/src/llmq/quorums_chainlocks.h index 8c210324252a..033f8041f74e 100644 --- a/src/llmq/quorums_chainlocks.h +++ b/src/llmq/quorums_chainlocks.h @@ -14,6 +14,8 @@ #include #include +#include + class CBlockIndex; class CScheduler; @@ -52,6 +54,7 @@ class CChainLocksHandler : public CRecoveredSigsListener private: CScheduler* scheduler; + boost::thread* scheduler_thread; CCriticalSection cs; bool tryLockChainTipScheduled{false}; bool isSporkActive{false}; @@ -78,7 +81,7 @@ class CChainLocksHandler : public CRecoveredSigsListener int64_t lastCleanupTime{0}; public: - explicit CChainLocksHandler(CScheduler* _scheduler); + explicit CChainLocksHandler(); ~CChainLocksHandler(); void Start(); diff --git a/src/llmq/quorums_init.cpp b/src/llmq/quorums_init.cpp index 4d2bb4e80e39..23b8b5ebdc9d 100644 --- a/src/llmq/quorums_init.cpp +++ b/src/llmq/quorums_init.cpp @@ -15,7 +15,6 @@ #include #include -#include namespace llmq { @@ -24,7 +23,7 @@ CBLSWorker* blsWorker; CDBWrapper* llmqDb; -void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests, bool fWipe) +void InitLLMQSystem(CEvoDB& evoDb, bool unitTests, bool fWipe) { llmqDb = new CDBWrapper(unitTests ? "" : (GetDataDir() / "llmq"), 1 << 20, unitTests, fWipe); blsWorker = new CBLSWorker(); @@ -35,7 +34,7 @@ void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests, bool f quorumManager = new CQuorumManager(evoDb, *blsWorker, *quorumDKGSessionManager); quorumSigSharesManager = new CSigSharesManager(); quorumSigningManager = new CSigningManager(*llmqDb, unitTests); - chainLocksHandler = new CChainLocksHandler(scheduler); + chainLocksHandler = new CChainLocksHandler(); quorumInstantSendManager = new CInstantSendManager(*llmqDb); } diff --git a/src/llmq/quorums_init.h b/src/llmq/quorums_init.h index 38309b0a13bc..d6e16222635c 100644 --- a/src/llmq/quorums_init.h +++ b/src/llmq/quorums_init.h @@ -7,7 +7,6 @@ class CDBWrapper; class CEvoDB; -class CScheduler; namespace llmq { @@ -16,7 +15,7 @@ namespace llmq static const bool DEFAULT_WATCH_QUORUMS = false; // Init/destroy LLMQ globals -void InitLLMQSystem(CEvoDB& evoDb, CScheduler* scheduler, bool unitTests, bool fWipe = false); +void InitLLMQSystem(CEvoDB& evoDb, bool unitTests, bool fWipe = false); void DestroyLLMQSystem(); // Manage scheduled tasks, threads, listeners etc. diff --git a/src/test/test_dash.cpp b/src/test/test_dash.cpp index 55254afaad6d..434e86a2cfc8 100644 --- a/src/test/test_dash.cpp +++ b/src/test/test_dash.cpp @@ -97,7 +97,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha connman = g_connman.get(); pblocktree.reset(new CBlockTreeDB(1 << 20, true)); pcoinsdbview.reset(new CCoinsViewDB(1 << 23, true)); - llmq::InitLLMQSystem(*evoDb, nullptr, true); + llmq::InitLLMQSystem(*evoDb, true); pcoinsTip.reset(new CCoinsViewCache(pcoinsdbview.get())); if (!LoadGenesisBlock(chainparams)) { throw std::runtime_error("LoadGenesisBlock failed."); @@ -117,6 +117,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha TestingSetup::~TestingSetup() { llmq::InterruptLLMQSystem(); + llmq::StopLLMQSystem(); threadGroup.interrupt_all(); threadGroup.join_all(); GetMainSignals().FlushBackgroundCallbacks(); From 509bb943571f5af8d8f0b7dc7fa3079f55198f40 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 26 May 2020 13:17:33 +0300 Subject: [PATCH 4/4] Revert "Let ActivateBestChain skip SyncWithValidationInterfaceQueue when called from IS or CL threads" This reverts commit 1c9f6da50a5a72ea9cd96f7ea32e901ba4fadc57. --- src/llmq/quorums_chainlocks.cpp | 10 +++------- src/llmq/quorums_instantsend.cpp | 2 +- src/validation.cpp | 10 +++++----- src/validation.h | 2 +- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 63647f5b2c23..a0f5be06d385 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -540,13 +540,9 @@ void CChainLocksHandler::EnforceBestChainLock() activateNeeded = chainActive.Tip()->GetAncestor(currentBestChainLockBlockIndex->nHeight) != currentBestChainLockBlockIndex; } - if (activateNeeded) { - CValidationState state; - if (!ActivateBestChain(state, Params(), std::shared_ptr(), false /* fSyncQueue */)) { - LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); - // This should not have happened and we are in a state were it's not safe to continue anymore - assert(false); - } + CValidationState state; + if (activateNeeded && !ActivateBestChain(state, Params())) { + LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); } const CBlockIndex* pindexNotify = nullptr; diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 3deee82de0ed..f245874f1091 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -1324,7 +1324,7 @@ void CInstantSendManager::ResolveBlockConflicts(const uint256& islockHash, const if (activateBestChain) { CValidationState state; - if (!ActivateBestChain(state, Params(), std::shared_ptr(), false /* fSyncQueue */)) { + if (!ActivateBestChain(state, Params())) { LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); // This should not have happened and we are in a state were it's not safe to continue anymore assert(false); diff --git a/src/validation.cpp b/src/validation.cpp index 6bb5cb5e0dd7..2711359de2a3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -164,7 +164,7 @@ class CChainState { bool LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree); - bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock, bool fSyncQueue = true); + bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock); bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex); bool AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock); @@ -2935,7 +2935,7 @@ static void NotifyHeaderTip() { * or an activated best chain. pblock is either nullptr or a pointer to a block * that is already loaded (to avoid loading it again from disk). */ -bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock, bool fSyncQueue) { +bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { // Note that while we're often called here from ProcessNewBlock, this is // far from a guarantee. Things in the P2P/RPC will often end up calling // us in the middle of ProcessNewBlock - do not assume pblock is set @@ -2953,7 +2953,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& do { boost::this_thread::interruption_point(); - if (fSyncQueue && GetMainSignals().CallbacksPending() > 10) { + if (GetMainSignals().CallbacksPending() > 10) { // Block until the validation queue drains. This should largely // never happen in normal operation, however may happen during // reindex, causing memory blowup if we run too far ahead. @@ -3026,8 +3026,8 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& return true; } -bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock, bool fSyncQueue) { - return g_chainstate.ActivateBestChain(state, chainparams, std::move(pblock), fSyncQueue); +bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { + return g_chainstate.ActivateBestChain(state, chainparams, std::move(pblock)); } bool CChainState::PreciousBlock(CValidationState& state, const CChainParams& params, CBlockIndex *pindex) diff --git a/src/validation.h b/src/validation.h index a60bfcc667fa..5b1aa0adf1d9 100644 --- a/src/validation.h +++ b/src/validation.h @@ -279,7 +279,7 @@ bool IsInitialBlockDownload(); /** Retrieve a transaction (from memory pool, or from disk, if possible) */ bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = nullptr); /** Find the best known block, and make it the tip of the block chain */ -bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr(), bool fSyncQueue = true); +bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr()); double ConvertBitsToDouble(unsigned int nBits); CAmount GetBlockSubsidy(int nBits, int nHeight, const Consensus::Params& consensusParams, bool fSuperblockPartOnly = false);