diff --git a/src/active/dkgsessionhandler.cpp b/src/active/dkgsessionhandler.cpp index 1d540275efda..2eec6e9b92bd 100644 --- a/src/active/dkgsessionhandler.cpp +++ b/src/active/dkgsessionhandler.cpp @@ -114,15 +114,31 @@ 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)); + const bool waiting_for_round_start{!curPhase.has_value()}; + bool logged_snapshot_wait{false}; while (true) { - if (m_chainman.IsSnapshotActiveAndUnvalidated()) { - LogPrint(BCLog::LLMQ_DKG, "ActiveDKGSessionHandler::%s -- %s qi[%d] - aborting because snapshot background validation is incomplete\n", __func__, params.name, quorumIndex); - throw AbortPhaseException(); - } if (stopRequested) { LogPrint(BCLog::LLMQ_DKG, "ActiveDKGSessionHandler::%s -- %s qi[%d] - aborting due to stop/shutdown requested\n", __func__, params.name, quorumIndex); throw AbortPhaseException(); } + if (m_chainman.IsSnapshotActiveAndUnvalidated()) { + if (!waiting_for_round_start) { + LogPrint(BCLog::LLMQ_DKG, + "ActiveDKGSessionHandler::%s -- %s qi[%d] - aborting because snapshot background validation " + "is incomplete\n", + __func__, params.name, quorumIndex); + throw AbortPhaseException(); + } + // No round can start until background validation completes, so wait instead of aborting + if (!logged_snapshot_wait) { + LogPrint(BCLog::LLMQ_DKG, + "ActiveDKGSessionHandler::%s -- %s qi[%d] - waiting for snapshot background validation\n", + __func__, params.name, quorumIndex); + logged_snapshot_wait = true; + } + UninterruptibleSleep(std::chrono::milliseconds{100}); + continue; + } auto [_phase, _quorumHash] = GetPhaseAndQuorumHash(); if (!expectedQuorumHash.IsNull() && _quorumHash != expectedQuorumHash) { LogPrint(BCLog::LLMQ_DKG, "ActiveDKGSessionHandler::%s -- %s qi[%d] - aborting due unexpected expectedQuorumHash change\n", __func__, params.name, quorumIndex); diff --git a/src/active/dkgsessionhandler.h b/src/active/dkgsessionhandler.h index 11764fe17bd1..acbcc36f29b6 100644 --- a/src/active/dkgsessionhandler.h +++ b/src/active/dkgsessionhandler.h @@ -107,7 +107,8 @@ class ActiveDKGSessionHandler final : public llmq::CDKGSessionHandler bool InitNewQuorum(gsl::not_null pQuorumBaseBlockIndex) EXCLUSIVE_LOCKS_REQUIRED(!cs_session); /** - * @param curPhase current QuorumPhase + * @param curPhase current QuorumPhase, or nullopt for the wait before a round starts, which waits out an + * unvalidated snapshot instead of aborting * @param nextPhase next QuorumPhase * @param expectedQuorumHash expected QuorumHash, defaults to null * @param shouldNotWait function that returns bool, defaults to function that returns false. If the function returns false, we will wait in the loop, if true, we don't wait diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 2fa994f1032e..ced9b4649c4e 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -21,13 +21,19 @@ #include #include #include +#include #include #include +#include +#include +#include #include #include #include #include +#include +#include #include #include #include @@ -35,6 +41,9 @@ #include +#include +#include +#include #include #include @@ -54,6 +63,59 @@ 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) + { + } + + //! Ends a wait that a failed check left running, so the test fails instead of hanging + ~TestDKGHandler() { handler.RequestStop(); } + + //! Runs a phase thread's wait on another thread + void StartWait(std::optional cur_phase, llmq::QuorumPhase next_phase) + { + wait = std::async(std::launch::async, + [this, cur_phase, next_phase] { handler.WaitForNextPhase(cur_phase, next_phase); }); + } + + //! Starts the wait a phase thread makes before a round, and returns whether that wait reached the + //! snapshot gate. A phase thread waits for Initialized; this one waits for Idle, the phase the + //! handler stays in, so it returns as soon as the gate lets it through. + bool StartRoundWait() + { + std::promise gated; + DebugLogHelper gate_log{"waiting for snapshot background validation", [&](const std::string* line) { + if (!line) return false; + gated.set_value(); + return true; + }}; + StartWait(std::nullopt, llmq::QuorumPhase::Idle); + return gated.get_future().wait_for(10s) == std::future_status::ready; + } + + std::future wait; +}; + } // namespace BOOST_FIXTURE_TEST_SUITE(validation_chainstatemanager_tests, ChainTestingSetup) @@ -129,6 +191,21 @@ 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. The handler stays Idle, so a wait from Idle has no phase + // change to abort on and only the snapshot check can end it. + dkg.StartWait(llmq::QuorumPhase::Idle, llmq::QuorumPhase::Initialized); + BOOST_REQUIRE(dkg.wait.wait_for(10s) == std::future_status::ready); + BOOST_CHECK_THROW(dkg.wait.get(), llmq::AbortPhaseException); + // A new round cannot start, so the phase thread waits instead of aborting in a loop + BOOST_REQUIRE(dkg.StartRoundWait()); + BOOST_CHECK(dkg.wait.wait_for(0s) == std::future_status::timeout); + // Shutdown still ends that wait + dkg.handler.RequestStop(); + BOOST_REQUIRE(dkg.wait.wait_for(10s) == std::future_status::ready); + BOOST_CHECK_THROW(dkg.wait.get(), llmq::AbortPhaseException); + } BOOST_CHECK_EQUAL(&c2, &manager.ActiveChainstate()); BOOST_CHECK(&c1 != &manager.ActiveChainstate()); auto all2 = manager.GetAll(); @@ -942,9 +1019,18 @@ 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()}; + BOOST_REQUIRE(dkg.StartRoundWait()); + BOOST_CHECK(dkg.wait.wait_for(0s) == 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 + BOOST_REQUIRE(dkg.wait.wait_for(10s) == std::future_status::ready); + BOOST_CHECK_NO_THROW(dkg.wait.get()); + } WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated())); BOOST_CHECK(chainman.IsSnapshotActive());