From d4798e908a0d2e479b3d3bc2ab5479184c19c96e Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 4 Nov 2026 05:59:46 +0000 Subject: [PATCH] fix: idle DKG phase threads while snapshot background validation is incomplete While the snapshot chainstate is active and unvalidated, every DKG phase handler thread spun with no delay: WaitForNextPhase(nullopt, Initialized), the first call of each round, threw AbortPhaseException on its first check, and PhaseHandlerThread caught it and restarted the round at once. Each iteration took cs_main, threw and caught an exception, called MarkAborted and, under -debug=llmq-dkg, logged four lines. No round can start in that state (InitNewQuorum refuses it), so the round-start wait now sleeps and re-checks instead of aborting. A round in progress is still abandoned, a stop request still ends the wait, and nodes not running on an unvalidated snapshot execute the same code as before. Co-Authored-By: Claude Opus 5.5 --- src/active/dkgsessionhandler.cpp | 12 ++++ .../validation_chainstatemanager_tests.cpp | 71 ++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/src/active/dkgsessionhandler.cpp b/src/active/dkgsessionhandler.cpp index c11fcd3d18cd..307e12e17cd9 100644 --- a/src/active/dkgsessionhandler.cpp +++ b/src/active/dkgsessionhandler.cpp @@ -16,6 +16,8 @@ #include #include +#include + namespace llmq { ActiveDKGSessionHandler::ActiveDKGSessionHandler( CBLSWorker& bls_worker, CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, @@ -113,8 +115,18 @@ void ActiveDKGSessionHandler::WaitForNextPhase(std::optional curPha { LogPrint(BCLog::LLMQ_DKG, "ActiveDKGSessionHandler::%s -- %s qi[%d] - starting, curPhase=%d, nextPhase=%d\n", __func__, params.name, quorumIndex, curPhase.has_value() ? std23::to_underlying(*curPhase) : -1, std23::to_underlying(nextPhase)); + bool logged_snapshot_wait{false}; while (true) { if (m_chainman.IsSnapshotActiveAndUnvalidated()) { + if (!curPhase.has_value() && !stopRequested) { + // No round can start before background validation completes, so there is nothing to abort + if (!std::exchange(logged_snapshot_wait, true)) { + LogPrint(BCLog::LLMQ_DKG, "%s -- %s qi[%d] - waiting for snapshot background validation\n", + __func__, params.name, quorumIndex); + } + UninterruptibleSleep(std::chrono::milliseconds{100}); + continue; + } LogPrint(BCLog::LLMQ_DKG, "ActiveDKGSessionHandler::%s -- %s qi[%d] - aborting because snapshot background validation is incomplete\n", __func__, params.name, quorumIndex); throw AbortPhaseException(); } diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 304386d600dc..3b5bbddca84e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -20,13 +20,19 @@ #include #include #include +#include #include #include +#include +#include +#include #include #include #include #include +#include +#include #include #include #include @@ -34,6 +40,9 @@ #include +#include +#include +#include #include #include @@ -53,6 +62,38 @@ void SeedSnapshotMarker(CEvoDB& evodb, const uint256& hash) BOOST_REQUIRE(evodb.CommitRootTransaction(EvoDbIdentity::SNAPSHOT)); } +CBLSSecretKey MakeOperatorKey() +{ + CBLSSecretKey sk; + sk.MakeNewKey(); + return sk; +} + +//! A DKG phase handler that no phase thread or block tip drives, so its phase stays Idle +struct TestDKGHandler { + CActiveMasternodeManager mn_activeman; + llmq::CDKGDebugManager dkgdbgman; + llmq::CDKGSessionManager qdkgsman; + llmq::ActiveDKGSessionHandler handler; + + TestDKGHandler(node::NodeContext& node, ChainstateManager& chainman, const fs::path& datadir) : + mn_activeman{*node.connman, *node.dmnman, MakeOperatorKey()}, + dkgdbgman{*node.dmnman, *node.llmq_ctx->qsnapman, chainman}, + qdkgsman{*node.dmnman, *node.llmq_ctx->qsnapman, chainman, *node.sporkman, + util::DbWrapperParams{.path = datadir, .memory = true, .wipe = true}}, + handler(*node.llmq_ctx->bls_worker, *node.dmnman, *node.mn_metaman, dkgdbgman, qdkgsman, + *node.llmq_ctx->quorum_block_processor, *node.llmq_ctx->qsnapman, mn_activeman, chainman, + *node.sporkman, Params().GetConsensus().llmqs.front(), /*quorums_watch=*/false, /*quorums_idx=*/0) + { + } + + //! The wait a phase thread makes at the start of every round + std::future StartRoundWait(llmq::QuorumPhase next_phase) + { + return std::async(std::launch::async, [this, next_phase] { handler.WaitForNextPhase(std::nullopt, next_phase); }); + } +}; + } // namespace BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, ChainTestingSetup) @@ -128,6 +169,19 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) BOOST_CHECK(WITH_LOCK(::cs_main, return !manager.IsSnapshotValidated())); BOOST_CHECK(manager.IsSnapshotActiveAndUnvalidated()); BOOST_CHECK(!llmq::CSigSharesManager::IsQuorumSigningAllowed(manager)); + { + TestDKGHandler dkg{m_node, manager, m_args.GetDataDirNet()}; + // A round in progress is abandoned + BOOST_CHECK_THROW(dkg.handler.WaitForNextPhase(llmq::QuorumPhase::Initialized, llmq::QuorumPhase::Contribute), + llmq::AbortPhaseException); + // A new round cannot start, so the phase thread waits instead of aborting in a loop + auto round_start = dkg.StartRoundWait(llmq::QuorumPhase::Initialized); + BOOST_CHECK(round_start.wait_for(200ms) == std::future_status::timeout); + // Shutdown still ends that wait + dkg.handler.RequestStop(); + BOOST_REQUIRE(round_start.wait_for(10s) == std::future_status::ready); + BOOST_CHECK_THROW(round_start.get(), llmq::AbortPhaseException); + } BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate()); BOOST_CHECK(&c1 != &manager.ActiveChainstate()); auto all2 = manager.GetAll(); @@ -934,9 +988,20 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()->GetBlockHash()); - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); - BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS); + { + TestDKGHandler dkg{m_node, chainman, m_args.GetDataDirNet()}; + auto round_start = dkg.StartRoundWait(llmq::QuorumPhase::Idle); + BOOST_CHECK(round_start.wait_for(200ms) == std::future_status::timeout); + + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS); + + // The DKG phase thread resumes once background validation completes + const bool resumed{round_start.wait_for(10s) == std::future_status::ready}; + BOOST_CHECK(resumed); + if (!resumed) dkg.handler.RequestStop(); // so a failure doesn't hang in the future's destructor + BOOST_CHECK_NO_THROW(round_start.get()); + } WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated())); BOOST_CHECK(chainman.IsSnapshotActive());