From 8e8b0d3d1a86272523a6311ff14895101ce59785 Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Sun, 13 Mar 2022 15:56:31 -0500 Subject: [PATCH] refactor: make QuorumPhase a enum class, remove QuorumPhase_None, use optional instead, adjust WaitForNextPhase (#4722) --- src/llmq/dkgsessionhandler.cpp | 48 +++++++++++++++++----------------- src/llmq/dkgsessionhandler.h | 27 +++++++++++-------- src/llmq/dkgsessionmgr.cpp | 8 +++--- 3 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 44765ebf12..cc671b5c1a 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -92,12 +92,12 @@ void CDKGSessionHandler::UpdatedBlockTip(const CBlockIndex* pindexNew) bool fNewPhase = (quorumStageInt % params.dkgPhaseBlocks) == 0; int phaseInt = quorumStageInt / params.dkgPhaseBlocks + 1; QuorumPhase oldPhase = phase; - if (fNewPhase && phaseInt >= QuorumPhase_Initialized && phaseInt <= QuorumPhase_Idle) { + if (fNewPhase && phaseInt >= int(QuorumPhase::Initialized) && phaseInt <= int(QuorumPhase::Idle)) { phase = static_cast(phaseInt); } LogPrint(BCLog::LLMQ_DKG, "CDKGSessionHandler::%s -- %s - currentHeight=%d, pQuorumBaseBlockIndex->nHeight=%d, oldPhase=%d, newPhase=%d\n", __func__, - params.name, currentHeight, pQuorumBaseBlockIndex->nHeight, oldPhase, phase); + params.name, currentHeight, pQuorumBaseBlockIndex->nHeight, int(oldPhase), int(phase)); } void CDKGSessionHandler::ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStream& vRecv) @@ -159,38 +159,38 @@ std::pair CDKGSessionHandler::GetPhaseAndQuorumHash() cons class AbortPhaseException : public std::exception { }; -void CDKGSessionHandler::WaitForNextPhase(QuorumPhase curPhase, +void CDKGSessionHandler::WaitForNextPhase(std::optional curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash, - const WhileWaitFunc& runWhileWaiting) const + const WhileWaitFunc& shouldNotWait) const { - LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - starting, curPhase=%d, nextPhase=%d\n", __func__, params.name, curPhase, nextPhase); + LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - starting, curPhase=%d, nextPhase=%d\n", __func__, params.name, curPhase.has_value() ? int(*curPhase) : -1, int(nextPhase)); while (true) { if (stopRequested) { LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - aborting due to stop/shutdown requested\n", __func__, params.name); throw AbortPhaseException(); } - auto p = GetPhaseAndQuorumHash(); - if (!expectedQuorumHash.IsNull() && p.second != expectedQuorumHash) { + auto [_phase, _quorumHash] = GetPhaseAndQuorumHash(); + if (!expectedQuorumHash.IsNull() && _quorumHash != expectedQuorumHash) { LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - aborting due unexpected expectedQuorumHash change\n", __func__, params.name); throw AbortPhaseException(); } - if (p.first == nextPhase) { + if (_phase == nextPhase) { break; } - if (curPhase != QuorumPhase_None && p.first != curPhase) { + if (curPhase.has_value() && _phase != curPhase) { LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - aborting due unexpected phase change\n", __func__, params.name); throw AbortPhaseException(); } - if (!runWhileWaiting()) { + if (!shouldNotWait()) { UninterruptibleSleep(std::chrono::milliseconds{100}); } } - LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - done, curPhase=%d, nextPhase=%d\n", __func__, params.name, curPhase, nextPhase); + LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - done, curPhase=%d, nextPhase=%d\n", __func__, params.name, curPhase.has_value() ? int(*curPhase) : -1, int(nextPhase)); - if (nextPhase == QuorumPhase_Initialized) { + if (nextPhase == QuorumPhase::Initialized) { quorumDKGDebugManager->ResetLocalSessionStatus(params.type); } else { quorumDKGDebugManager->UpdateLocalSessionStatus(params.type, [&](CDKGDebugSessionStatus& status) { @@ -256,7 +256,7 @@ void CDKGSessionHandler::SleepBeforePhase(QuorumPhase curPhase, heightTmp = heightStart = currentHeight; } - LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - starting sleep for %d ms, curPhase=%d\n", __func__, params.name, sleepTime, curPhase); + LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - starting sleep for %d ms, curPhase=%d\n", __func__, params.name, sleepTime, int(curPhase)); while (GetTimeMillis() < endTime) { if (stopRequested) { @@ -285,7 +285,7 @@ void CDKGSessionHandler::SleepBeforePhase(QuorumPhase curPhase, } } - LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - done, curPhase=%d\n", __func__, params.name, curPhase); + LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - done, curPhase=%d\n", __func__, params.name, int(curPhase)); } void CDKGSessionHandler::HandlePhase(QuorumPhase curPhase, @@ -295,13 +295,13 @@ void CDKGSessionHandler::HandlePhase(QuorumPhase curPhase, const StartPhaseFunc& startPhaseFunc, const WhileWaitFunc& runWhileWaiting) { - LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - starting, curPhase=%d, nextPhase=%d\n", __func__, params.name, curPhase, nextPhase); + LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - starting, curPhase=%d, nextPhase=%d\n", __func__, params.name, int(curPhase), int(nextPhase)); SleepBeforePhase(curPhase, expectedQuorumHash, randomSleepFactor, runWhileWaiting); startPhaseFunc(); WaitForNextPhase(curPhase, nextPhase, expectedQuorumHash, runWhileWaiting); - LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - done, curPhase=%d, nextPhase=%d\n", __func__, params.name, curPhase, nextPhase); + LogPrint(BCLog::LLMQ_DKG, "CDKGSessionManager::%s -- %s - done, curPhase=%d, nextPhase=%d\n", __func__, params.name, int(curPhase), int(nextPhase)); } // returns a set of NodeIds which sent invalid messages @@ -458,7 +458,7 @@ void CDKGSessionHandler::HandleDKGRound() { uint256 curQuorumHash; - WaitForNextPhase(QuorumPhase_None, QuorumPhase_Initialized, uint256(), []{return false;}); + WaitForNextPhase(std::nullopt, QuorumPhase::Initialized); { LOCK(cs); @@ -478,8 +478,8 @@ void CDKGSessionHandler::HandleDKGRound() } quorumDKGDebugManager->UpdateLocalSessionStatus(params.type, [&](CDKGDebugSessionStatus& status) { - bool changed = status.phase != (uint8_t) QuorumPhase_Initialized; - status.phase = (uint8_t) QuorumPhase_Initialized; + bool changed = status.phase != (uint8_t) QuorumPhase::Initialized; + status.phase = (uint8_t) QuorumPhase::Initialized; return changed; }); @@ -488,7 +488,7 @@ void CDKGSessionHandler::HandleDKGRound() CLLMQUtils::AddQuorumProbeConnections(params, pQuorumBaseBlockIndex, curSession->myProTxHash); } - WaitForNextPhase(QuorumPhase_Initialized, QuorumPhase_Contribute, curQuorumHash, []{return false;}); + WaitForNextPhase(QuorumPhase::Initialized, QuorumPhase::Contribute, curQuorumHash); // Contribute auto fContributeStart = [this]() { @@ -497,7 +497,7 @@ void CDKGSessionHandler::HandleDKGRound() auto fContributeWait = [this] { return ProcessPendingMessageBatch(*curSession, pendingContributions, 8); }; - HandlePhase(QuorumPhase_Contribute, QuorumPhase_Complain, curQuorumHash, 0.05, fContributeStart, fContributeWait); + HandlePhase(QuorumPhase::Contribute, QuorumPhase::Complain, curQuorumHash, 0.05, fContributeStart, fContributeWait); // Complain auto fComplainStart = [this]() { @@ -506,7 +506,7 @@ void CDKGSessionHandler::HandleDKGRound() auto fComplainWait = [this] { return ProcessPendingMessageBatch(*curSession, pendingComplaints, 8); }; - HandlePhase(QuorumPhase_Complain, QuorumPhase_Justify, curQuorumHash, 0.05, fComplainStart, fComplainWait); + HandlePhase(QuorumPhase::Complain, QuorumPhase::Justify, curQuorumHash, 0.05, fComplainStart, fComplainWait); // Justify auto fJustifyStart = [this]() { @@ -515,7 +515,7 @@ void CDKGSessionHandler::HandleDKGRound() auto fJustifyWait = [this] { return ProcessPendingMessageBatch(*curSession, pendingJustifications, 8); }; - HandlePhase(QuorumPhase_Justify, QuorumPhase_Commit, curQuorumHash, 0.05, fJustifyStart, fJustifyWait); + HandlePhase(QuorumPhase::Justify, QuorumPhase::Commit, curQuorumHash, 0.05, fJustifyStart, fJustifyWait); // Commit auto fCommitStart = [this]() { @@ -524,7 +524,7 @@ void CDKGSessionHandler::HandleDKGRound() auto fCommitWait = [this] { return ProcessPendingMessageBatch(*curSession, pendingPrematureCommitments, 8); }; - HandlePhase(QuorumPhase_Commit, QuorumPhase_Finalize, curQuorumHash, 0.1, fCommitStart, fCommitWait); + HandlePhase(QuorumPhase::Commit, QuorumPhase::Finalize, curQuorumHash, 0.1, fCommitStart, fCommitWait); auto finalCommitments = curSession->FinalizeCommitments(); for (const auto& fqc : finalCommitments) { diff --git a/src/llmq/dkgsessionhandler.h b/src/llmq/dkgsessionhandler.h index c8809c69eb..7a4fd8aa14 100644 --- a/src/llmq/dkgsessionhandler.h +++ b/src/llmq/dkgsessionhandler.h @@ -18,15 +18,14 @@ namespace llmq class CDKGSession; class CDKGSessionManager; -enum QuorumPhase { - QuorumPhase_None = -1, - QuorumPhase_Initialized = 1, - QuorumPhase_Contribute, - QuorumPhase_Complain, - QuorumPhase_Justify, - QuorumPhase_Commit, - QuorumPhase_Finalize, - QuorumPhase_Idle, +enum class QuorumPhase { + Initialized = 1, + Contribute, + Complain, + Justify, + Commit, + Finalize, + Idle, }; /** @@ -111,7 +110,7 @@ private: CBLSWorker& blsWorker; CDKGSessionManager& dkgManager; - QuorumPhase phase GUARDED_BY(cs) {QuorumPhase_Idle}; + QuorumPhase phase GUARDED_BY(cs) {QuorumPhase::Idle}; int currentHeight GUARDED_BY(cs) {-1}; uint256 quorumHash GUARDED_BY(cs); @@ -154,7 +153,13 @@ private: using StartPhaseFunc = std::function; using WhileWaitFunc = std::function; - void WaitForNextPhase(QuorumPhase curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash, const WhileWaitFunc& runWhileWaiting) const; + /** + * @param curPhase current QuorumPhase + * @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 + */ + void WaitForNextPhase(std::optional curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash=uint256(), const WhileWaitFunc& shouldNotWait=[]{return false;}) const; void WaitForNewQuorum(const uint256& oldQuorumHash) const; void SleepBeforePhase(QuorumPhase curPhase, const uint256& expectedQuorumHash, double randomSleepFactor, const WhileWaitFunc& runWhileWaiting) const; void HandlePhase(QuorumPhase curPhase, QuorumPhase nextPhase, const uint256& expectedQuorumHash, double randomSleepFactor, const StartPhaseFunc& startPhaseFunc, const WhileWaitFunc& runWhileWaiting); diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index c4922f6f8b..b428ef9651 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -213,7 +213,7 @@ bool CDKGSessionManager::GetContribution(const uint256& hash, CDKGContribution& for (const auto& p : dkgSessionHandlers) { auto& dkgType = p.second; LOCK(dkgType.cs); - if (dkgType.phase < QuorumPhase_Initialized || dkgType.phase > QuorumPhase_Contribute) { + if (dkgType.phase < QuorumPhase::Initialized || dkgType.phase > QuorumPhase::Contribute) { continue; } LOCK(dkgType.curSession->invCs); @@ -234,7 +234,7 @@ bool CDKGSessionManager::GetComplaint(const uint256& hash, CDKGComplaint& ret) c for (const auto& p : dkgSessionHandlers) { auto& dkgType = p.second; LOCK(dkgType.cs); - if (dkgType.phase < QuorumPhase_Contribute || dkgType.phase > QuorumPhase_Complain) { + if (dkgType.phase < QuorumPhase::Contribute || dkgType.phase > QuorumPhase::Complain) { continue; } LOCK(dkgType.curSession->invCs); @@ -255,7 +255,7 @@ bool CDKGSessionManager::GetJustification(const uint256& hash, CDKGJustification for (const auto& p : dkgSessionHandlers) { auto& dkgType = p.second; LOCK(dkgType.cs); - if (dkgType.phase < QuorumPhase_Complain || dkgType.phase > QuorumPhase_Justify) { + if (dkgType.phase < QuorumPhase::Complain || dkgType.phase > QuorumPhase::Justify) { continue; } LOCK(dkgType.curSession->invCs); @@ -276,7 +276,7 @@ bool CDKGSessionManager::GetPrematureCommitment(const uint256& hash, CDKGPrematu for (const auto& p : dkgSessionHandlers) { auto& dkgType = p.second; LOCK(dkgType.cs); - if (dkgType.phase < QuorumPhase_Justify || dkgType.phase > QuorumPhase_Commit) { + if (dkgType.phase < QuorumPhase::Justify || dkgType.phase > QuorumPhase::Commit) { continue; } LOCK(dkgType.curSession->invCs);