Skip to content

Commit aa26747

Browse files
PastaPastaPastaPastaPastaPasta
committed
refactor: replace CQuorumCPtr with CQuorum references in quorum-related functions
1 parent aa19512 commit aa26747

File tree

4 files changed

+58
-58
lines changed

4 files changed

+58
-58
lines changed

‎src/llmq/signing.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ bool CSigningManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigShares
733733

734734
if (allowReSign) {
735735
// make us re-announce all known shares (other nodes might have run into a timeout)
736-
shareman.ForceReAnnouncement(quorum, llmqType, id, msgHash);
736+
shareman.ForceReAnnouncement(*quorum, llmqType, id, msgHash);
737737
}
738738
shareman.AsyncSign(std::move(quorum), id, msgHash);
739739

‎src/llmq/signing_shares.cpp‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -757,14 +757,14 @@ void CSigSharesManager::ProcessSigShare(PeerManager& peerman, const CSigShare& s
757757
}
758758

759759
if (canTryRecovery) {
760-
TryRecoverSig(peerman, quorum, sigShare.getId(), sigShare.getMsgHash());
760+
TryRecoverSig(peerman, *quorum, sigShare.getId(), sigShare.getMsgHash());
761761
}
762762
}
763763

764-
void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& quorum, const uint256& id,
764+
void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorum& quorum, const uint256& id,
765765
const uint256& msgHash)
766766
{
767-
if (sigman.HasRecoveredSigForId(quorum->params.type, id)) {
767+
if (sigman.HasRecoveredSigForId(quorum.params.type, id)) {
768768
return;
769769
}
770770

@@ -773,13 +773,13 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q
773773
{
774774
LOCK(cs);
775775

776-
auto signHash = SignHash(quorum->params.type, quorum->qc->quorumHash, id, msgHash).Get();
776+
auto signHash = SignHash(quorum.params.type, quorum.qc->quorumHash, id, msgHash).Get();
777777
const auto* sigSharesForSignHash = sigShares.GetAllForSignHash(signHash);
778778
if (sigSharesForSignHash == nullptr) {
779779
return;
780780
}
781781

782-
if (quorum->params.size == 1) {
782+
if (quorum.params.size == 1) {
783783
if (sigSharesForSignHash->empty()) {
784784
LogPrint(BCLog::LLMQ_SIGS, /* Continued */
785785
"CSigSharesManager::%s -- impossible to recover single-node signature - no shares yet. id=%s, "
@@ -792,22 +792,22 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q
792792
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recover single-node signature. id=%s, msgHash=%s\n",
793793
__func__, id.ToString(), msgHash.ToString());
794794

795-
auto rs = std::make_shared<CRecoveredSig>(quorum->params.type, quorum->qc->quorumHash, id, msgHash,
795+
auto rs = std::make_shared<CRecoveredSig>(quorum.params.type, quorum.qc->quorumHash, id, msgHash,
796796
recoveredSig);
797797
sigman.ProcessRecoveredSig(rs, peerman);
798798
return; // end of single-quorum processing
799799
}
800800

801-
sigSharesForRecovery.reserve((size_t) quorum->params.threshold);
802-
idsForRecovery.reserve((size_t) quorum->params.threshold);
803-
for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum->params.threshold); ++it) {
801+
sigSharesForRecovery.reserve((size_t) quorum.params.threshold);
802+
idsForRecovery.reserve((size_t) quorum.params.threshold);
803+
for (auto it = sigSharesForSignHash->begin(); it != sigSharesForSignHash->end() && sigSharesForRecovery.size() < size_t(quorum.params.threshold); ++it) {
804804
const auto& sigShare = it->second;
805805
sigSharesForRecovery.emplace_back(sigShare.sigShare.Get());
806-
idsForRecovery.emplace_back(quorum->members[sigShare.getQuorumMember()]->proTxHash);
806+
idsForRecovery.emplace_back(quorum.members[sigShare.getQuorumMember()]->proTxHash);
807807
}
808808

809809
// check if we can recover the final signature
810-
if (sigSharesForRecovery.size() < size_t(quorum->params.threshold)) {
810+
if (sigSharesForRecovery.size() < size_t(quorum.params.threshold)) {
811811
return;
812812
}
813813
}
@@ -824,14 +824,14 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q
824824
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- recovered signature. id=%s, msgHash=%s, time=%d\n", __func__,
825825
id.ToString(), msgHash.ToString(), t.count());
826826

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

829829
// There should actually be no need to verify the self-recovered signatures as it should always succeed. Let's
830830
// however still verify it from time to time, so that we have a chance to catch bugs. We do only this sporadic
831831
// verification because this is unbatched and thus slow verification that happens here.
832832
if (((recoveredSigsCounter++) % 100) == 0) {
833833
auto signHash = rs->buildSignHash();
834-
bool valid = recoveredSig.VerifyInsecure(quorum->qc->quorumPublicKey, signHash.Get());
834+
bool valid = recoveredSig.VerifyInsecure(quorum.qc->quorumPublicKey, signHash.Get());
835835
if (!valid) {
836836
// this should really not happen as we have verified all signature shares before
837837
LogPrintf("CSigSharesManager::%s -- own recovered signature is invalid. id=%s, msgHash=%s\n", __func__,
@@ -843,13 +843,13 @@ void CSigSharesManager::TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& q
843843
sigman.ProcessRecoveredSig(rs, peerman);
844844
}
845845

846-
CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorumCPtr& quorum, const uint256 &id, int attempt)
846+
CDeterministicMNCPtr CSigSharesManager::SelectMemberForRecovery(const CQuorum& quorum, const uint256 &id, int attempt)
847847
{
848-
assert(attempt < quorum->params.recoveryMembers);
848+
assert(attempt < quorum.params.recoveryMembers);
849849

850850
std::vector<std::pair<uint256, CDeterministicMNCPtr>> v;
851-
v.reserve(quorum->members.size());
852-
for (const auto& dmn : quorum->members) {
851+
v.reserve(quorum.members.size());
852+
for (const auto& dmn : quorum.members) {
853853
auto h = ::SerializeHash(std::make_pair(dmn->proTxHash, id));
854854
v.emplace_back(h, dmn);
855855
}
@@ -1033,7 +1033,7 @@ void CSigSharesManager::CollectSigSharesToSendConcentrated(std::unordered_map<No
10331033
int64_t waitTime = exp2(signedSession.attempt) * EXP_SEND_FOR_RECOVERY_TIMEOUT;
10341034
waitTime = std::min(MAX_SEND_FOR_RECOVERY_TIMEOUT, waitTime);
10351035
signedSession.nextAttemptTime = curTime + waitTime;
1036-
auto dmn = SelectMemberForRecovery(signedSession.quorum, signedSession.sigShare.getId(), signedSession.attempt);
1036+
auto dmn = SelectMemberForRecovery(*signedSession.quorum, signedSession.sigShare.getId(), signedSession.attempt);
10371037
signedSession.attempt++;
10381038

10391039
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, sending to %s, attempt=%d\n", __func__,
@@ -1519,7 +1519,7 @@ void CSigSharesManager::SignPendingSigShares(const CConnman& connman, PeerManage
15191519
WITH_LOCK(cs_pendingSigns, v.swap(pendingSigns));
15201520

15211521
for (const auto& [pQuorum, id, msgHash] : v) {
1522-
auto opt_sigShare = CreateSigShare(pQuorum, id, msgHash);
1522+
auto opt_sigShare = CreateSigShare(*pQuorum, id, msgHash);
15231523

15241524
if (opt_sigShare.has_value() && opt_sigShare->sigShare.Get().IsValid()) {
15251525
auto sigShare = *opt_sigShare;
@@ -1537,25 +1537,25 @@ void CSigSharesManager::SignPendingSigShares(const CConnman& connman, PeerManage
15371537
}
15381538
}
15391539

1540-
std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const
1540+
std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const
15411541
{
15421542
assert(m_mn_activeman);
15431543

15441544
cxxtimer::Timer t(true);
15451545
auto activeMasterNodeProTxHash = m_mn_activeman->GetProTxHash();
15461546

1547-
if (!quorum->IsValidMember(activeMasterNodeProTxHash)) {
1547+
if (!quorum.IsValidMember(activeMasterNodeProTxHash)) {
15481548
return std::nullopt;
15491549
}
15501550

1551-
if (quorum->params.size == 1) {
1552-
int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash);
1551+
if (quorum.params.size == 1) {
1552+
int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash);
15531553
if (memberIdx == -1) {
15541554
// this should really not happen (IsValidMember gave true)
15551555
return std::nullopt;
15561556
}
15571557

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

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

15801580
return sigShare;
15811581
}
1582-
const CBLSSecretKey& skShare = quorum->GetSkShare();
1582+
const CBLSSecretKey& skShare = quorum.GetSkShare();
15831583
if (!skShare.IsValid()) {
1584-
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have our skShare for quorum %s\n", __func__, quorum->qc->quorumHash.ToString());
1584+
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have our skShare for quorum %s\n", __func__, quorum.qc->quorumHash.ToString());
15851585
return std::nullopt;
15861586
}
15871587

1588-
int memberIdx = quorum->GetMemberIndex(activeMasterNodeProTxHash);
1588+
int memberIdx = quorum.GetMemberIndex(activeMasterNodeProTxHash);
15891589
if (memberIdx == -1) {
15901590
// this should really not happen (IsValidMember gave true)
15911591
return std::nullopt;
15921592
}
15931593

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

15971597
sigShare.sigShare.Set(skShare.Sign(signHash, bls::bls_legacy_scheme.load()), bls::bls_legacy_scheme.load());
@@ -1604,20 +1604,20 @@ std::optional<CSigShare> CSigSharesManager::CreateSigShare(const CQuorumCPtr& qu
16041604
sigShare.UpdateKey();
16051605

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

16091609
return sigShare;
16101610
}
16111611

16121612
// causes all known sigShares to be re-announced
1613-
void CSigSharesManager::ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
1613+
void CSigSharesManager::ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash)
16141614
{
16151615
if (IsAllMembersConnectedEnabled(llmqType, m_sporkman)) {
16161616
return;
16171617
}
16181618

16191619
LOCK(cs);
1620-
auto signHash = SignHash(llmqType, quorum->qc->quorumHash, id, msgHash).Get();
1620+
auto signHash = SignHash(llmqType, quorum.qc->quorumHash, id, msgHash).Get();
16211621
if (const auto *const sigs = sigShares.GetAllForSignHash(signHash)) {
16221622
for (const auto& [quorumMemberIndex, _] : *sigs) {
16231623
// 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
@@ -435,12 +435,12 @@ class CSigSharesManager : public CRecoveredSigsListener
435435

436436
void AsyncSign(CQuorumCPtr quorum, const uint256& id, const uint256& msgHash)
437437
EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingSigns);
438-
std::optional<CSigShare> CreateSigShare(const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash) const;
439-
void ForceReAnnouncement(const CQuorumCPtr& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
438+
std::optional<CSigShare> CreateSigShare(const CQuorum& quorum, const uint256& id, const uint256& msgHash) const;
439+
void ForceReAnnouncement(const CQuorum& quorum, Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash);
440440

441441
[[nodiscard]] MessageProcessingResult HandleNewRecoveredSig(const CRecoveredSig& recoveredSig) override;
442442

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

445445
private:
446446
// all of these return false when the currently processed message should be aborted (as each message actually contains multiple messages)
@@ -466,7 +466,7 @@ class CSigSharesManager : public CRecoveredSigsListener
466466

467467
void ProcessSigShare(PeerManager& peerman, const CSigShare& sigShare, const CConnman& connman,
468468
const CQuorumCPtr& quorum);
469-
void TryRecoverSig(PeerManager& peerman, const CQuorumCPtr& quorum, const uint256& id, const uint256& msgHash);
469+
void TryRecoverSig(PeerManager& peerman, const CQuorum& quorum, const uint256& id, const uint256& msgHash);
470470

471471
bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo);
472472
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
@@ -179,20 +179,20 @@ static RPCHelpMan quorum_list_extended()
179179
}
180180

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

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

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

203203
if (includeMembers) {
204204
UniValue membersArr(UniValue::VARR);
205-
for (size_t i = 0; i < quorum->members.size(); i++) {
206-
const auto& dmn = quorum->members[i];
205+
for (size_t i = 0; i < quorum.members.size(); i++) {
206+
const auto& dmn = quorum.members[i];
207207
UniValue mo(UniValue::VOBJ);
208208
mo.pushKV("proTxHash", dmn->proTxHash.ToString());
209209
mo.pushKV("service", dmn->pdmnState->netInfo->GetPrimary().ToStringAddrPort());
210210
mo.pushKV("addresses", GetNetInfoWithLegacyFields(*dmn->pdmnState, dmn->nType));
211211
mo.pushKV("pubKeyOperator", dmn->pdmnState->pubKeyOperator.ToString());
212-
mo.pushKV("valid", static_cast<bool>(quorum->qc->validMembers[i]));
213-
if (quorum->qc->validMembers[i]) {
214-
if (quorum->params.size == 1) {
212+
mo.pushKV("valid", static_cast<bool>(quorum.qc->validMembers[i]));
213+
if (quorum.qc->validMembers[i]) {
214+
if (quorum.params.size == 1) {
215215
mo.pushKV("pubKeyShare", dmn->pdmnState->pubKeyOperator.ToString());
216216
} else {
217-
CBLSPublicKey pubKey = quorum->GetPubKeyShare(i);
217+
CBLSPublicKey pubKey = quorum.GetPubKeyShare(i);
218218
if (pubKey.IsValid()) {
219219
mo.pushKV("pubKeyShare", pubKey.ToString());
220220
}
@@ -225,8 +225,8 @@ static UniValue BuildQuorumInfo(const llmq::CQuorumBlockProcessor& quorum_block_
225225

226226
ret.pushKV("members", membersArr);
227227
}
228-
ret.pushKV("quorumPublicKey", quorum->qc->quorumPublicKey.ToString());
229-
const CBLSSecretKey& skShare = quorum->GetSkShare();
228+
ret.pushKV("quorumPublicKey", quorum.qc->quorumPublicKey.ToString());
229+
const CBLSSecretKey& skShare = quorum.GetSkShare();
230230
if (includeSkShare && skShare.IsValid()) {
231231
ret.pushKV("secretKeyShare", skShare.ToString());
232232
}
@@ -265,7 +265,7 @@ static RPCHelpMan quorum_info()
265265
throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found");
266266
}
267267

268-
return BuildQuorumInfo(*llmq_ctx.quorum_block_processor, quorum, true, includeSkShare);
268+
return BuildQuorumInfo(*llmq_ctx.quorum_block_processor, *quorum, true, includeSkShare);
269269
},
270270
};
271271
}
@@ -423,7 +423,7 @@ static RPCHelpMan quorum_memberof()
423423
auto quorums = llmq_ctx.qman->ScanQuorums(llmq_params_opt->type, count);
424424
for (auto& quorum : quorums) {
425425
if (quorum->IsMember(dmn->proTxHash)) {
426-
auto json = BuildQuorumInfo(*llmq_ctx.quorum_block_processor, quorum, false, false);
426+
auto json = BuildQuorumInfo(*llmq_ctx.quorum_block_processor, *quorum, false, false);
427427
json.pushKV("isValidMember", quorum->IsValidMember(dmn->proTxHash));
428428
json.pushKV("memberIndex", quorum->GetMemberIndex(dmn->proTxHash));
429429
result.push_back(json);
@@ -473,7 +473,7 @@ static UniValue quorum_sign_helper(const JSONRPCRequest& request, Consensus::LLM
473473
throw JSONRPCError(RPC_INVALID_PARAMETER, "quorum not found");
474474
}
475475

476-
auto sigShare = llmq_ctx.shareman->CreateSigShare(pQuorum, id, msgHash);
476+
auto sigShare = llmq_ctx.shareman->CreateSigShare(*pQuorum, id, msgHash);
477477

478478
if (!sigShare.has_value() || !sigShare->sigShare.Get().IsValid()) {
479479
throw JSONRPCError(RPC_INVALID_PARAMETER, "failed to create sigShare");
@@ -742,7 +742,7 @@ static RPCHelpMan quorum_selectquorum()
742742

743743
UniValue recoveryMembers(UniValue::VARR);
744744
for (int i = 0; i < quorum->params.recoveryMembers; ++i) {
745-
auto dmn = llmq_ctx.shareman->SelectMemberForRecovery(quorum, id, i);
745+
auto dmn = llmq_ctx.shareman->SelectMemberForRecovery(*quorum, id, i);
746746
recoveryMembers.push_back(dmn->proTxHash.ToString());
747747
}
748748
ret.pushKV("recoveryMembers", recoveryMembers);

0 commit comments

Comments
 (0)