Skip to content

Commit 845f965

Browse files
Merge #7247: refactor: simplify architecture of quorum observer and quorum participant
863ee6b doc: clarify masternode late InitializeCurrentBlockTip comment (UdjinM6) dc8c6cf fix: replay startup tip into ActiveContext/ObserverContext (UdjinM6) 21fe75f fix: guard GetProTxHash() behind is_masternode check, add CQuorum safety assert (UdjinM6) 27129d9 fix: defer full InitializeCurrentBlockTip broadcast on masternodes (UdjinM6) 55d8025 refactor: safety belt to be sure that CQuorum is initialized properly and no empty qc member (Konstantin Akimov) 8bc5744 fix: review comments - missing forward declaration, protection for no valid members (Konstantin Akimov) e2df648 refactor: drop argument request_limit_exceeded in ProcessContribQGETDATA (Konstantin Akimov) b47de25 refactor: remove net related code ProcessContribQGETDATA and ProcessContribQDATA to NetQuorum (Konstantin Akimov) 1be8863 refactor: move out network code to new NetQuorum from CQuorumManager (Konstantin Akimov) c86e4f6 refactor: it's always either observer_ctx or active_ctx, not both created together (Konstantin Akimov) 6a2f291 refactor: remove duplicated code between QuorumRole's UpdatedBlockTip and InitializeQuorumConnections (Konstantin Akimov) 6c99cb0 fix: parameter 'params' shadows member inherited from type 'CDKGSession' [-Werror,-Wshadow-field] (Konstantin Akimov) 0f1174a refactor: remove multiple friends of CDKGSession (Konstantin Akimov) f4cbf92 refactor: inline QuorumParticipant to ActiveContext (Konstantin Akimov) 5ded711 refactor: rename src/llmq/observer/context to src/llmq/observer (Konstantin Akimov) 93f168a refactor: remove QuorumRoleBase by moving its functionality directly to QuorumRole (Konstantin Akimov) 9bb50db refactor: merge QuorumObserver and ObserverContext classes to the one entity to reduce amount of abstractions (Konstantin Akimov) 7978822 refactor: quorum members: remove QuorumObserverParent; participant is no more child of 'observer' (Konstantin Akimov) Pull request description: ## Issue being fixed or feature implemented It's a follow-up for #7066 and related PRs This PR is reducing mental concentration when reading quorum-related code by significant simplification amount of levels of abstractions and inheritance classes from each other. This PR is a direct dependency of #7234 (kernel / chainstate project) Performance is not expected to be improved by any noticeable margin. Compilation time is expected to be improved marginally due to fewer files (-2) to compile and less transitive includes over `src/llmq/quorumsman.h`. ## What was done? This changes has been spotted during #7234 and extracted to own PR. - Remove QuorumObserverParent; decouple QuorumParticipant from QuorumObserver inheritance — they become independent implementations of a common QuorumRole interface. - Merge QuorumObserver and ObserverContext into a single class, then remove the transitional QuorumRoleBase by folding it into QuorumRole. - Rename src/llmq/observer/context.{h,cpp} to src/llmq/observer.{h,cpp} — flatten the now-single-file subdirectory. - Inline QuorumParticipant into ActiveContext — it only applies to active masternode mode; src/active/quorums.{h,cpp} deleted. - Remove four friend declarations from CDKGSession (ActiveDKGSession, ActiveDKGSessionHandler, CDKGSessionHandler, CDKGSessionManager); affected members changed from private to protected. - Extract network code from CQuorumManager and ActiveContext into new NetQuorum (src/llmq/net_quorum.{h,cpp}) — handles QGETDATA/QDATA processing, quorum peer connections, data recovery, and periodic cleanup. ### Class hierarchy — before vs. after *by claude Before: QuorumObserverParent (interface, implemented by CQuorumManager) │ QuorumObserver (base class with shared state) ├── ObserverContext (observer mode, also CValidationInterface) └── QuorumParticipant (active mode, in src/active/quorums.h) After: QuorumRole (pure virtual interface in quorumsman.h) ├── ObserverContext (observer mode, in src/llmq/observer.h) └── ActiveContext (active mode, QuorumParticipant inlined) ## How Has This Been Tested? Run unit & functional tests ## Breaking Changes N/A ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: UdjinM6: utACK 863ee6b Tree-SHA512: 13a3d8e533bf811fd5d438e0c2f3007b633e990b725767907d5c4b8eb2da32d06f282f3f61319ce78a17e40ca41dac1b3c791039c5672321e1b43a7174712b2d
2 parents 67683f3 + 863ee6b commit 845f965

30 files changed

Lines changed: 1175 additions & 1262 deletions

src/Makefile.am

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ BITCOIN_CORE_H = \
155155
active/dkgsession.h \
156156
active/dkgsessionhandler.h \
157157
active/masternode.h \
158-
active/quorums.h \
159158
addrdb.h \
160159
addressindex.h \
161160
spentindex.h \
@@ -283,19 +282,19 @@ BITCOIN_CORE_H = \
283282
llmq/dkgsessionhandler.h \
284283
llmq/dkgsessionmgr.h \
285284
llmq/ehf_signals.h \
285+
llmq/observer.h \
286286
llmq/options.h \
287287
llmq/params.h \
288288
llmq/quorums.h \
289289
llmq/quorumsman.h \
290290
llmq/signhash.h \
291291
llmq/signing.h \
292+
llmq/net_quorum.h \
292293
llmq/net_signing.h \
293294
llmq/signing_shares.h \
294295
llmq/snapshot.h \
295296
llmq/types.h \
296297
llmq/utils.h \
297-
llmq/observer/context.h \
298-
llmq/observer/quorums.h \
299298
logging.h \
300299
logging/timer.h \
301300
mapport.h \
@@ -486,7 +485,6 @@ libbitcoin_node_a_SOURCES = \
486485
active/dkgsession.cpp \
487486
active/dkgsessionhandler.cpp \
488487
active/masternode.cpp \
489-
active/quorums.cpp \
490488
addrdb.cpp \
491489
addressindex.cpp \
492490
addrman.cpp \
@@ -555,7 +553,9 @@ libbitcoin_node_a_SOURCES = \
555553
llmq/dkgsessionhandler.cpp \
556554
llmq/dkgsessionmgr.cpp \
557555
llmq/ehf_signals.cpp \
556+
llmq/net_quorum.cpp \
558557
llmq/net_signing.cpp \
558+
llmq/observer.cpp \
559559
llmq/options.cpp \
560560
llmq/quorums.cpp \
561561
llmq/quorumsman.cpp \
@@ -564,8 +564,6 @@ libbitcoin_node_a_SOURCES = \
564564
llmq/signing_shares.cpp \
565565
llmq/snapshot.cpp \
566566
llmq/utils.cpp \
567-
llmq/observer/context.cpp \
568-
llmq/observer/quorums.cpp \
569567
mapport.cpp \
570568
masternode/meta.cpp \
571569
masternode/payments.cpp \

src/active/context.cpp

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,21 @@
66

77
#include <active/dkgsessionhandler.h>
88
#include <active/masternode.h>
9-
#include <active/quorums.h>
9+
#include <bls/bls_worker.h>
1010
#include <chainlock/handler.h>
1111
#include <chainlock/signing.h>
12+
#include <evo/deterministicmns.h>
1213
#include <governance/governance.h>
1314
#include <governance/signing.h>
1415
#include <instantsend/instantsend.h>
1516
#include <instantsend/signing.h>
16-
#include <llmq/context.h>
1717
#include <llmq/debug.h>
1818
#include <llmq/dkgsessionmgr.h>
1919
#include <llmq/ehf_signals.h>
20+
#include <llmq/quorums.h>
2021
#include <llmq/quorumsman.h>
2122
#include <llmq/signing_shares.h>
23+
#include <masternode/sync.h>
2224
#include <util/check.h>
2325
#include <validation.h>
2426
#include <validationinterface.h>
@@ -30,17 +32,16 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
3032
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
3133
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
3234
const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
33-
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
34-
bool quorums_recovery, bool quorums_watch) :
35-
m_qman{qman},
35+
const util::DbWrapperParams& db_params, bool quorums_watch) :
36+
llmq::QuorumRole{qman},
37+
m_bls_worker{bls_worker},
38+
m_quorums_watch{quorums_watch},
3639
nodeman{std::make_unique<CActiveMasternodeManager>(connman, dmnman, operator_sk)},
3740
dkgdbgman{std::make_unique<llmq::CDKGDebugManager>(dmnman, qsnapman, chainman)},
3841
qdkgsman{std::make_unique<llmq::CDKGSessionManager>(dmnman, qsnapman, chainman, sporkman, db_params, quorums_watch)},
3942
shareman{std::make_unique<llmq::CSigSharesManager>(connman, chainman, sigman, *nodeman, qman, sporkman)},
4043
gov_signer{std::make_unique<GovernanceSigner>(connman, dmnman, govman, *nodeman, chainman, mn_sync)},
4144
ehf_sighandler{std::make_unique<llmq::CEHFSignalsHandler>(chainman, sigman, *shareman, qman)},
42-
qman_handler{std::make_unique<llmq::QuorumParticipant>(bls_worker, connman, dmnman, qman, qsnapman, *nodeman, chainman,
43-
mn_sync, sporkman, sync_map, quorums_recovery, quorums_watch)},
4445
cl_signer{std::make_unique<chainlock::ChainLockSigner>(chainman.ActiveChainstate(), chainlocks, clhandler, isman,
4546
qman, sigman, *shareman, mn_sync)},
4647
is_signer{std::make_unique<instantsend::InstantSendSigner>(chainman.ActiveChainstate(), chainlocks, isman, sigman,
@@ -52,17 +53,16 @@ ActiveContext::ActiveContext(CBLSWorker& bls_worker, ChainstateManager& chainman
5253
qblockman, qsnapman, *nodeman, chainman, sporkman,
5354
llmq_params, quorums_watch, quorum_idx);
5455
});
55-
m_qman.ConnectManagers(qman_handler.get(), qdkgsman.get());
56+
m_qman.ConnectManagers(this, qdkgsman.get());
5657
}
5758

5859
ActiveContext::~ActiveContext()
5960
{
6061
m_qman.DisconnectManagers();
6162
}
6263

63-
void ActiveContext::Start(CConnman& connman, PeerManager& peerman, int16_t worker_count)
64+
void ActiveContext::Start(CConnman& connman, PeerManager& peerman)
6465
{
65-
qman_handler->Start(worker_count);
6666
qdkgsman->StartThreads(connman, peerman);
6767
cl_signer->Start();
6868
cl_signer->RegisterRecoveryInterface();
@@ -81,7 +81,6 @@ void ActiveContext::Stop()
8181
cl_signer->UnregisterRecoveryInterface();
8282
cl_signer->Stop();
8383
qdkgsman->StopThreads();
84-
qman_handler->Stop();
8584
}
8685

8786
CCoinJoinServer& ActiveContext::GetCJServer() const
@@ -99,9 +98,6 @@ void ActiveContext::SetCJServer(gsl::not_null<CCoinJoinServer*> cj_server)
9998
void ActiveContext::InitializeCurrentBlockTip(const CBlockIndex* tip, bool ibd)
10099
{
101100
UpdatedBlockTip(tip, nullptr, ibd);
102-
if (tip) {
103-
qman_handler->InitializeQuorumConnections(tip);
104-
}
105101
}
106102

107103
void ActiveContext::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload)
@@ -113,5 +109,26 @@ void ActiveContext::UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIn
113109
ehf_sighandler->UpdatedBlockTip(pindexNew);
114110
gov_signer->UpdatedBlockTip(pindexNew);
115111
qdkgsman->UpdatedBlockTip(pindexNew, fInitialDownload);
116-
qman_handler->UpdatedBlockTip(pindexNew, fInitialDownload);
112+
}
113+
114+
bool ActiveContext::IsMasternode() const
115+
{
116+
// We are only initialized if masternode mode is enabled
117+
return true;
118+
}
119+
120+
bool ActiveContext::IsWatching() const
121+
{
122+
// Watch-only mode can co-exist with masternode mode
123+
return m_quorums_watch;
124+
}
125+
126+
uint256 ActiveContext::GetProTxHash() const
127+
{
128+
return nodeman->GetProTxHash();
129+
}
130+
131+
bool ActiveContext::SetQuorumSecretKeyShare(llmq::CQuorum& quorum, Span<CBLSSecretKey> skContributions) const
132+
{
133+
return quorum.SetSecretKeyShare(m_bls_worker.AggregateSecretKeys(skContributions), nodeman->GetProTxHash());
117134
}

src/active/context.h

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,19 @@
55
#ifndef BITCOIN_ACTIVE_CONTEXT_H
66
#define BITCOIN_ACTIVE_CONTEXT_H
77

8-
#include <llmq/options.h>
8+
#include <llmq/quorumsman.h>
99

1010
#include <validationinterface.h>
1111

1212
#include <gsl/pointers.h>
13+
#include <span.h>
1314

1415
#include <memory>
1516

1617
class CActiveMasternodeManager;
17-
class CBLSSecretKey;
1818
class CBLSWorker;
19-
class ChainstateManager;
2019
class CCoinJoinServer;
2120
class CConnman;
22-
class CDeterministicMNManager;
2321
class CGovernanceManager;
2422
class CMasternodeMetaMan;
2523
class CMasternodeSync;
@@ -38,23 +36,19 @@ class InstantSendSigner;
3836
} // namespace instantsend
3937
namespace llmq {
4038
class CDKGDebugManager;
41-
class CDKGSessionManager;
4239
class CEHFSignalsHandler;
4340
class CInstantSendManager;
44-
class CQuorumBlockProcessor;
45-
class CQuorumManager;
46-
class CQuorumSnapshotManager;
4741
class CSigningManager;
4842
class CSigSharesManager;
49-
class QuorumParticipant;
5043
} // namespace llmq
5144
namespace util {
5245
struct DbWrapperParams;
5346
} // namespace util
5447

55-
struct ActiveContext final : public CValidationInterface {
48+
struct ActiveContext final : public llmq::QuorumRole, public CValidationInterface {
5649
private:
57-
llmq::CQuorumManager& m_qman;
50+
CBLSWorker& m_bls_worker;
51+
const bool m_quorums_watch{false};
5852

5953
public:
6054
ActiveContext() = delete;
@@ -67,19 +61,24 @@ struct ActiveContext final : public CValidationInterface {
6761
llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumManager& qman,
6862
llmq::CQuorumSnapshotManager& qsnapman, llmq::CSigningManager& sigman,
6963
const CMasternodeSync& mn_sync, const CBLSSecretKey& operator_sk,
70-
const llmq::QvvecSyncModeMap& sync_map, const util::DbWrapperParams& db_params,
71-
bool quorums_recovery, bool quorums_watch);
64+
const util::DbWrapperParams& db_params, bool quorums_watch);
7265
~ActiveContext();
7366

74-
void Start(CConnman& connman, PeerManager& peerman, int16_t worker_count);
67+
void Start(CConnman& connman, PeerManager& peerman);
7568
void Stop();
76-
void InitializeCurrentBlockTip(const CBlockIndex* tip, bool ibd);
7769

7870
CCoinJoinServer& GetCJServer() const;
7971
void SetCJServer(gsl::not_null<CCoinJoinServer*> cj_server);
8072

73+
// QuorumRole
74+
bool IsMasternode() const override;
75+
bool IsWatching() const override;
76+
uint256 GetProTxHash() const override;
77+
bool SetQuorumSecretKeyShare(llmq::CQuorum& quorum, Span<CBLSSecretKey> skContributions) const override;
78+
8179
protected:
8280
// CValidationInterface
81+
void InitializeCurrentBlockTip(const CBlockIndex* tip, bool ibd) override;
8382
void UpdatedBlockTip(const CBlockIndex* pindexNew, const CBlockIndex* pindexFork, bool fInitialDownload) override;
8483

8584
public:
@@ -95,7 +94,6 @@ struct ActiveContext final : public CValidationInterface {
9594
private:
9695
const std::unique_ptr<GovernanceSigner> gov_signer;
9796
const std::unique_ptr<llmq::CEHFSignalsHandler> ehf_sighandler;
98-
const std::unique_ptr<llmq::QuorumParticipant> qman_handler;
9997
const std::unique_ptr<chainlock::ChainLockSigner> cl_signer;
10098

10199
public:

src/active/dkgsession.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ ActiveDKGSession::ActiveDKGSession(CBLSWorker& bls_worker, CDeterministicMNManag
2525
CDKGSessionManager& qdkgsman, CMasternodeMetaMan& mn_metaman,
2626
CQuorumSnapshotManager& qsnapman, const CActiveMasternodeManager& mn_activeman,
2727
const ChainstateManager& chainman, const CSporkManager& sporkman,
28-
const CBlockIndex* base_block_index, const Consensus::LLMQParams& params) :
29-
CDKGSession(bls_worker, dmnman, dkgdbgman, qdkgsman, qsnapman, chainman, base_block_index, params),
28+
const CBlockIndex* base_block_index, const Consensus::LLMQParams& llmq_params) :
29+
CDKGSession(bls_worker, dmnman, dkgdbgman, qdkgsman, qsnapman, chainman, base_block_index, llmq_params),
3030
m_mn_metaman{mn_metaman},
3131
m_mn_activeman{mn_activeman},
3232
m_sporkman{sporkman},

src/active/dkgsessionhandler.cpp

Lines changed: 8 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,11 @@ bool ActiveDKGSessionHandler::InitNewQuorum(gsl::not_null<const CBlockIndex*> pQ
106106

107107
if (!curSession->Init(m_mn_activeman.GetProTxHash(), quorumIndex)) {
108108
LogPrintf("ActiveDKGSessionHandler::%s -- height[%d] quorum initialization failed for %s qi[%d]\n", __func__,
109-
pQuorumBaseBlockIndex->nHeight, curSession->params.name, quorumIndex);
109+
pQuorumBaseBlockIndex->nHeight, params.name, quorumIndex);
110110
return false;
111111
}
112112

113-
LogPrintf("ActiveDKGSessionHandler::%s -- height[%d] quorum initialization OK for %s qi[%d]\n", __func__, pQuorumBaseBlockIndex->nHeight, curSession->params.name, quorumIndex);
113+
LogPrintf("ActiveDKGSessionHandler::%s -- height[%d] quorum initialization OK for %s qi[%d]\n", __func__, pQuorumBaseBlockIndex->nHeight, params.name, quorumIndex);
114114
return true;
115115
}
116116

@@ -461,11 +461,11 @@ void ActiveDKGSessionHandler::HandleDKGRound(CConnman& connman, PeerManager& pee
461461

462462
const auto tip_mn_list = m_dmnman.GetListAtChainTip();
463463
utils::EnsureQuorumConnections(params, connman, m_sporkman, {m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex},
464-
tip_mn_list, curSession->myProTxHash, /*is_masternode=*/true, m_quorums_watch);
464+
tip_mn_list, curSession->ProTx(), /*is_masternode=*/true, m_quorums_watch);
465465
if (curSession->AreWeMember()) {
466466
utils::AddQuorumProbeConnections(params, connman, m_mn_metaman, m_sporkman,
467467
{m_dmnman, m_qsnapman, m_chainman, pQuorumBaseBlockIndex}, tip_mn_list,
468-
curSession->myProTxHash);
468+
curSession->ProTx());
469469
}
470470

471471
WaitForNextPhase(QuorumPhase::Initialized, QuorumPhase::Contribute, curQuorumHash);
@@ -527,58 +527,22 @@ void ActiveDKGSessionHandler::PhaseHandlerThread(CConnman& connman, PeerManager&
527527

528528
bool ActiveDKGSessionHandler::GetContribution(const uint256& hash, CDKGContribution& ret) const
529529
{
530-
if (!curSession) {
531-
return false;
532-
}
533-
LOCK(curSession->invCs);
534-
auto it = curSession->contributions.find(hash);
535-
if (it != curSession->contributions.end()) {
536-
ret = it->second;
537-
return true;
538-
}
539-
return false;
530+
return curSession && curSession->GetContribution(hash, ret);
540531
}
541532

542533
bool ActiveDKGSessionHandler::GetComplaint(const uint256& hash, CDKGComplaint& ret) const
543534
{
544-
if (!curSession) {
545-
return false;
546-
}
547-
LOCK(curSession->invCs);
548-
auto it = curSession->complaints.find(hash);
549-
if (it != curSession->complaints.end()) {
550-
ret = it->second;
551-
return true;
552-
}
553-
return false;
535+
return curSession && curSession->GetComplaint(hash, ret);
554536
}
555537

556538
bool ActiveDKGSessionHandler::GetJustification(const uint256& hash, CDKGJustification& ret) const
557539
{
558-
if (!curSession) {
559-
return false;
560-
}
561-
LOCK(curSession->invCs);
562-
auto it = curSession->justifications.find(hash);
563-
if (it != curSession->justifications.end()) {
564-
ret = it->second;
565-
return true;
566-
}
567-
return false;
540+
return curSession && curSession->GetJustification(hash, ret);
568541
}
569542

570543
bool ActiveDKGSessionHandler::GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const
571544
{
572-
if (!curSession) {
573-
return false;
574-
}
575-
LOCK(curSession->invCs);
576-
auto it = curSession->prematureCommitments.find(hash);
577-
if (it != curSession->prematureCommitments.end() && curSession->validCommitments.count(hash)) {
578-
ret = it->second;
579-
return true;
580-
}
581-
return false;
545+
return curSession && curSession->GetPrematureCommitment(hash, ret);
582546
}
583547

584548
QuorumPhase ActiveDKGSessionHandler::GetPhase() const

0 commit comments

Comments
 (0)