Skip to content

Commit 35a4f07

Browse files
PastaPastaPastaPastaPastaPasta
committed
refactor: replace CQuorumCPtr with CQuorum references in quorum-related functions
1 parent 382fd3c commit 35a4f07

File tree

3 files changed

+58
-58
lines changed

3 files changed

+58
-58
lines changed

‎src/llmq/signing_shares.cpp‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -768,13 +768,13 @@ void CSigSharesManager::ProcessSigShare(const CSigShare& sigShare, const CQuorum
768768
}
769769

770770
if (canTryRecovery) {
771-
TryRecoverSig(quorum, sigShare.getId(), sigShare.getMsgHash());
771+
TryRecoverSig(*quorum, sigShare.getId(), sigShare.getMsgHash());
772772
}
773773
}
774774

775-
void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash)
775+
void CSigSharesManager::TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash)
776776
{
777-
if (sigman.HasRecoveredSigForId(quorum->params.type, id)) {
777+
if (sigman.HasRecoveredSigForId(quorum.params.type, id)) {
778778
return;
779779
}
780780

@@ -783,13 +783,13 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256&
783783
{
784784
LOCK(cs);
785785

786-
auto signHash = SignHash(quorum->params.type, quorum->qc->quorumHash, id, msgHash).Get();
786+
auto signHash = SignHash(quorum.params.type, quorum.qc->quorumHash, id, msgHash).Get();
787787
const auto* sigSharesForSignHash = sigShares.GetAllForSignHash(signHash);
788788
if (sigSharesForSignHash == nullptr) {
789789
return;
790790
}
791791

792-
if (quorum->params.is_single_member()) {
792+
if (quorum.params.is_single_member()) {
793793
if (sigSharesForSignHash->empty()) {
794794
LogPrint(BCLog::LLMQ_SIGS, /* Continued */
795795
"CSigSharesManager::%s -- impossible to recover single-node signature - no shares yet. id=%s, "
@@ -802,22 +802,22 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256&
802802
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recover single-node signature. id=%s, msgHash=%s\n",
803803
__func__, id.ToString(), msgHash.ToString());
804804

805-
auto rs = std::make_shared<CRecoveredSig>(quorum->params.type, quorum->qc->quorumHash, id, msgHash,
805+
auto rs = std::make_shared<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash,
806806
recoveredSig);
807807
sigman.ProcessRecoveredSig(rs, m_peerman);
808808
return; // end of single-quorum processing
809809
}
810810

811-
sigSharesForRecovery.reserve((size_t) quorum->params.threshold);
812-
idsForRecovery.reserve((size_t) quorum->params.threshold);
813-
for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum->params.threshold); ++it) {
811+
sigSharesForRecovery.reserve((size_t) quorum.params.threshold);
812+
idsForRecovery.reserve((size_t) quorum.params.threshold);
813+
for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum.params.threshold); ++it) {
814814
const auto& sigShare = it->second;
815815
sigSharesForRecovery.emplace_back(sigShare.sigShare.Get());
816-
idsForRecovery.emplace_back(quorum->members[sigShare.getQuorumMember()]->proTxHash);
816+
idsForRecovery.emplace_back(quorum.members[sigShare.getQuorumMember()]->proTxHash);
817817
}
818818

819819
// check if we can recover the final signature
820-
if (sigSharesForRecovery.size() < size_t(quorum->params.threshold)) {
820+
if (sigSharesForRecovery.size() < size_t(quorum.params.threshold)) {
821821
return;
822822
}
823823
}
@@ -834,14 +834,14 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256&
834834
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recovered signature. id=%s, msgHash=%s, time=%d\n", __func__,
835835
id.ToString(), msgHash.ToString(), t.count());
836836

837-
auto rs = std::make_shared<CRecoveredSig>(quorum->params.type, quorum->qc->quorumHash, id, msgHash, recoveredSig);
837+
auto rs = std::make_shared<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash, recoveredSig);
838838

839839
// There should actually be no need to verify the self-recovered signatures as it should always succeed. Let's
840840
// however still verify it from time to time, so that we have a chance to catch bugs. We do only this sporadic
841841
// verification because this is unbatched and thus slow verification that happens here.
842842
if (((recoveredSigsCounter++) % 100) == 0) {
843843
auto signHash = rs->buildSignHash();
844-
bool valid = recoveredSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get());
844+
bool valid = recoveredSig.VerifyInsecure(quorum.qc->quorumPublicKey, signHash.Get());
845845
if (!valid) {
846846
// this should really not happen as we have verified all signature shares before
847847
LogPrintf("CSigSharesManager::%s -- own recovered signature is invalid. id=%s, msgHash=%s\n", __func__,
@@ -853,13 +853,13 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256&
853853
sigman.ProcessRecoveredSig(rs, m_peerman);
854854
}
855855

856-
CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256 &id, int attempt)
856+
CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt)
857857
{
858-
assert(attempt < quorum->params.recoveryMembers);
858+
assert(attempt < quorum.params.recoveryMembers);
859859

860860
std::vector<std::pair<uint256, CDeterministicMNCPtr>> v;
861-
v.reserve(quorum->members.size());
862-
for (const auto& dmn : quorum->members) {
861+
v.reserve(quorum.members.size());
862+
for (const auto& dmn : quorum.members) {
863863
auto h = ::SerializeHash(std::make_pair(dmn->proTxHash, id));
864864
v.emplace_back(h, dmn);
865865
}
@@ -941,7 +941,7 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning
941941

942942
if (allowReSign) {
943943
// make us re-announce all known shares (other nodes might have run into a timeout)
944-
ForceReAnnouncement(quorum, llmqType, id, msgHash);
944+
ForceReAnnouncement(*quorum, llmqType, id, msgHash);
945945
}
946946
AsyncSign(std::move(quorum), id, msgHash);
947947

@@ -1128,7 +1128,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map<No
11281128
int64_t waitTime = exp2(signedSession.attempt) * EXP_SEND_FOR_RECOVERY_TIMEOUT;
11291129
waitTime = std::min(MAX_SEND_FOR_RECOVERY_TIMEOUT, waitTime);
11301130
signedSession.nextAttemptTime = curTime + waitTime;
1131-
auto dmn = SelectMemberForRecovery(signedSession.quorum, signedSession.sigShare.getId(), signedSession.attempt);
1131+
auto dmn = SelectMemberForRecovery(*signedSession.quorum, signedSession.sigShare.getId(), signedSession.attempt);
11321132
signedSession.attempt++;
11331133

11341134
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, sending to %s, attempt=%d\n", __func__,
@@ -1611,7 +1611,7 @@ void CSigSharesManager::SignPendingSigShares()
16111611
WITH_LOCK(cs_pendingSigns, v.swap(pendingSigns));
16121612

16131613
for (const auto& [pQuorum, id, msgHash] : v) {
1614-
auto opt_sigShare = CreateSigShare(pQuorum, id, msgHash);
1614+
auto opt_sigShare = CreateSigShare(*pQuorum, id, msgHash);
16151615

16161616
if (opt_sigShare.has_value() && opt_sigShare->sigShare.Get().IsValid()) {
16171617
auto sigShare = *opt_sigShare;
@@ -1629,23 +1629,23 @@ void CSigSharesManager::SignPendingSigShares()
16291629
}
16301630
}
16311631

1632-
std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const
1632+
std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const
16331633
{
16341634
cxxtimer::Timer t(true);
16351635
auto activeMasterNodeProTxHash = m_mn_activeman.GetProTxHash();
16361636

1637-
if (!quorum->IsValidMember(activeMasterNodeProTxHash)) {
1637+
if (!quorum.IsValidMember(activeMasterNodeProTxHash)) {
16381638
return std::nullopt;
16391639
}
16401640

1641-
if (quorum->params.is_single_member()) {
1642-
int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash);
1641+
if (quorum.params.is_single_member()) {
1642+
int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash);
16431643
if (memberIdx == -1) {
16441644
// this should really not happen (IsValidMember gave true)
16451645
return std::nullopt;
16461646
}
16471647

1648-
CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
1648+
CSigShare sigShare(quorum.params.type, quorum.qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
16491649
uint256 signHash = sigShare.buildSignHash().Get();
16501650

16511651
// TODO: This one should be SIGN by QUORUM key, not by OPERATOR key
@@ -1665,23 +1665,23 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
16651665
"CSigSharesManager::%s -- created sigShare. signHash=%s, id=%s, msgHash=%s, llmqType=%d, quorum=%s, "
16661666
"time=%s\n",
16671667
__func__, signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(),
1668-
ToUnderlying(quorum->params.type), quorum->qc->quorumHash.ToString(), t.count());
1668+
ToUnderlying(quorum.params.type), quorum.qc->quorumHash.ToString(), t.count());
16691669

16701670
return sigShare;
16711671
}
1672-
const CBLSSecretKey& skShare = quorum->GetSkShare();
1672+
const CBLSSecretKey& skShare = quorum.GetSkShare();
16731673
if (!skShare.IsValid()) {
1674-
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have our skShare for quorum %s\n", __func__, quorum->qc->quorumHash.ToString());
1674+
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have our skShare for quorum %s\n", __func__, quorum.qc->quorumHash.ToString());
16751675
return std::nullopt;
16761676
}
16771677

1678-
int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash);
1678+
int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash);
16791679
if (memberIdx == -1) {
16801680
// this should really not happen (IsValidMember gave true)
16811681
return std::nullopt;
16821682
}
16831683

1684-
CSigShare sigShare(quorum->params.type, quorum->qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
1684+
CSigShare sigShare(quorum.params.type, quorum.qc->quorumHash, id, msgHash, uint16_t(memberIdx), {});
16851685
uint256 signHash = sigShare.buildSignHash().Get();
16861686

16871687
sigShare.sigShare.Set(skShare.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load());
@@ -1694,20 +1694,20 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
16941694
sigShare.UpdateKey();
16951695

16961696
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- created sigShare. signHash=%s, id=%s, msgHash=%s, llmqType=%d, quorum=%s, time=%s\n", __func__,
1697-
signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), ToUnderlying(quorum->params.type), quorum->qc->quorumHash.ToString(), t.count());
1697+
signHash.ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), ToUnderlying(quorum.params.type), quorum.qc->quorumHash.ToString(), t.count());
16981698

16991699
return sigShare;
17001700
}
17011701

17021702
// causes all known sigShares to be re-announced
1703-
void CSigSharesManager::ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
1703+
void CSigSharesManager::ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
17041704
{
17051705
if (IsAllMembersConnectedEnabled(llmqType, m_sporkman)) {
17061706
return;
17071707
}
17081708

17091709
LOCK(cs);
1710-
auto signHash = SignHash(llmqType, quorum->qc->quorumHash, id, msgHash).Get();
1710+
auto signHash = SignHash(llmqType, quorum.qc->quorumHash, id, msgHash).Get();
17111711
if (const auto *const sigs = sigShares.GetAllForSignHash(signHash)) {
17121712
for (const auto& [quorumMemberIndex, _] : *sigs) {
17131713
// re-announce every sigshare to every node

‎src/llmq/signing_shares.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -433,12 +433,12 @@ class CSigSharesManager : public CRecoveredSigsListener
433433

434434
void AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash)
435435
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
436-
std::optional<CSigShare> CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const;
437-
void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
436+
std::optional<CSigShare> CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const;
437+
void ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
438438

439439
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override;
440440

441-
static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256& id, int attempt);
441+
static CDeterministicMNCPtr SelectMemberForRecovery(const CQuorum& quorum, const uint256& id, int attempt);
442442

443443
bool AsyncSignIfMember(Consensus::LLMQType llmqType, CSigningManager& sigman, const uint256& id,
444444
const uint256& msgHash, const uint256& quorumHash = uint256(), bool allowReSign = false,
@@ -469,7 +469,7 @@ class CSigSharesManager : public CRecoveredSigsListener
469469
const std::unordered_map<std::pair<Consensus::LLMQType, uint256>, CQuorumCPtr, StaticSaltedHasher>& quorums);
470470

471471
void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum);
472-
void TryRecoverSig(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash);
472+
void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash);
473473

474474
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo);
475475
static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair<uint16_t, CBLSLazySignature>& in);

‎src/rpc/quorums.cpp‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -180,20 +180,20 @@ static RPCHelpMan quorum_list_extended()
180180
}
181181

182182
static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_processor,
183-
const llmq::CQuorumCPtr& quorum, bool includeMembers, bool includeSkShare)
183+
const llmq::CQuorum& quorum, bool includeMembers, bool includeSkShare)
184184
{
185185
UniValue ret(UniValue::VOBJ);
186186

187-
ret.pushKV("height", quorum->m_quorum_base_block_index->nHeight);
188-
ret.pushKV("type", std::string(quorum->params.name));
189-
ret.pushKV("quorumHash", quorum->qc->quorumHash.ToString());
190-
ret.pushKV("quorumIndex", quorum->qc->quorumIndex);
191-
ret.pushKV("minedBlock", quorum->minedBlockHash.ToString());
187+
ret.pushKV("height", quorum.m_quorum_base_block_index->nHeight);
188+
ret.pushKV("type", std::string(quorum.params.name));
189+
ret.pushKV("quorumHash", quorum.qc->quorumHash.ToString());
190+
ret.pushKV("quorumIndex", quorum.qc->quorumIndex);
191+
ret.pushKV("minedBlock", quorum.minedBlockHash.ToString());
192192

193-
if (quorum->params.useRotation) {
194-
auto previousActiveCommitment = quorum_block_processor.GetLastMinedCommitmentsByQuorumIndexUntilBlock(quorum->params.type, quorum->m_quorum_base_block_index, quorum->qc->quorumIndex, 0);
193+
if (quorum.params.useRotation) {
194+
auto previousActiveCommitment = quorum_block_processor.GetLastMinedCommitmentsByQuorumIndexUntilBlock(quorum.params.type, quorum.m_quorum_base_block_index, quorum.qc->quorumIndex, 0);
195195
if (previousActiveCommitment.has_value()) {
196-
int previousConsecutiveDKGFailures = (quorum->m_quorum_base_block_index->nHeight - previousActiveCommitment.value()->nHeight) / quorum->params.dkgInterval - 1;
196+
int previousConsecutiveDKGFailures = (quorum.m_quorum_base_block_index->nHeight - previousActiveCommitment.value()->nHeight) / quorum.params.dkgInterval - 1;
197197
ret.pushKV("previousConsecutiveDKGFailures", previousConsecutiveDKGFailures);
198198
}
199199
else {
@@ -203,19 +203,19 @@ static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_
203203

204204
if (includeMembers) {
205205
UniValue membersArr(UniValue::VARR);
206-
for (size_t i = 0; i < quorum->members.size(); i++) {
207-
const auto& dmn = quorum->members[i];
206+
for (size_t i = 0; i < quorum.members.size(); i++) {
207+
const auto& dmn = quorum.members[i];
208208
UniValue mo(UniValue::VOBJ);
209209
mo.pushKV("proTxHash", dmn->proTxHash.ToString());
210210
mo.pushKV("service", dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
211211
mo.pushKV("addresses", GetNetInfoWithLegacyFields(*dmn->pdmnState, dmn->nType));
212212
mo.pushKV("pubKeyOperator", dmn->pdmnState->pubKeyOperator.ToString());
213-
mo.pushKV("valid", static_cast<bool>(quorum->qc->validMembers[i]));
214-
if (quorum->qc->validMembers[i]) {
215-
if (quorum->params.is_single_member()) {
213+
mo.pushKV("valid", static_cast<bool>(quorum.qc->validMembers[i]));
214+
if (quorum.qc->validMembers[i]) {
215+
if (quorum.params.is_single_member()) {
216216
mo.pushKV("pubKeyShare", dmn->pdmnState->pubKeyOperator.ToString());
217217
} else {
218-
CBLSPublicKey pubKey = quorum->GetPubKeyShare(i);
218+
CBLSPublicKey pubKey = quorum.GetPubKeyShare(i);
219219
if (pubKey.IsValid()) {
220220
mo.pushKV("pubKeyShare", pubKey.ToString());
221221
}
@@ -226,8 +226,8 @@ static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_
226226

227227
ret.pushKV("members", membersArr);
228228
}
229-
ret.pushKV("quorumPublicKey", quorum->qc->quorumPublicKey.ToString());
230-
const CBLSSecretKey& skShare = quorum->GetSkShare();
229+
ret.pushKV("quorumPublicKey", quorum.qc->quorumPublicKey.ToString());
230+
const CBLSSecretKey& skShare = quorum.GetSkShare();
231231
if (includeSkShare && skShare.IsValid()) {
232232
ret.pushKV("secretKeyShare", skShare.ToString());
233233
}
@@ -271,7 +271,7 @@ static RPCHelpMan quorum_info()
271271
throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found");
272272
}
273273

274-
return BuildQuorumInfo(*llmq_ctx.quorum_block_processor, quorum, true, includeSkShare);
274+
return BuildQuorumInfo(*llmq_ctx.quorum_block_processor, *quorum, true, includeSkShare);
275275
},
276276
};
277277
}
@@ -467,7 +467,7 @@ static RPCHelpMan quorum_memberof()
467467
auto quorums = llmq_ctx.qman->ScanQuorums(llmq_params_opt->type, count);
468468
for (auto& quorum : quorums) {
469469
if (quorum->IsMember(dmn->proTxHash)) {
470-
auto json = BuildQuorumInfo(*llmq_ctx.quorum_block_processor, quorum, false, false);
470+
auto json = BuildQuorumInfo(*llmq_ctx.quorum_block_processor, *quorum, false, false);
471471
json.pushKV("isValidMember", quorum->IsValidMember(dmn->proTxHash));
472472
json.pushKV("memberIndex", quorum->GetMemberIndex(dmn->proTxHash));
473473
result.push_back(json);
@@ -521,7 +521,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM
521521
throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found");
522522
}
523523

524-
auto sigShare = CHECK_NONFATAL(node.active_ctx)->shareman->CreateSigShare(pQuorum, id, msgHash);
524+
auto sigShare = CHECK_NONFATAL(node.active_ctx)->shareman->CreateSigShare(*pQuorum, id, msgHash);
525525

526526
if (!sigShare.has_value() || !sigShare->sigShare.Get().IsValid()) {
527527
throw JSONRPCError(RPC_INVALID_PARAMETER, "failed to create sigShare");
@@ -815,7 +815,7 @@ static RPCHelpMan quorum_selectquorum()
815815

816816
UniValue recoveryMembers(UniValue::VARR);
817817
for (int i = 0; i < quorum->params.recoveryMembers; ++i) {
818-
auto dmn = llmq::CSigSharesManager::SelectMemberForRecovery(quorum, id, i);
818+
auto dmn = llmq::CSigSharesManager::SelectMemberForRecovery(*quorum, id, i);
819819
recoveryMembers.push_back(dmn->proTxHash.ToString());
820820
}
821821
ret.pushKV("recoveryMembers", recoveryMembers);

0 commit comments

Comments
 (0)