Skip to content

Commit f55fd90

Browse files
refactor: update PreVerifyBatchedSigShares to return structured results
1 parent e5ce1f0 commit f55fd90

File tree

2 files changed

+31
-18
lines changed

2 files changed

+31
-18
lines changed

src/llmq/signing_shares.cpp

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,9 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const
410410
return true;
411411
}
412412

413-
if (bool ban{false}; !PreVerifyBatchedSigShares(*Assert(m_mn_activeman), qman, sessionInfo, batchedSigShares, ban)) {
414-
return !ban;
413+
auto verifyResult = PreVerifyBatchedSigShares(*Assert(m_mn_activeman), qman, sessionInfo, batchedSigShares);
414+
if (!verifyResult.IsSuccess()) {
415+
return !verifyResult.should_ban;
415416
}
416417

417418
std::vector<CSigShare> sigSharesToProcess;
@@ -510,46 +511,41 @@ void CSigSharesManager::ProcessMessageSigShare(NodeId fromId, PeerManager& peerm
510511
sigShare.GetSignHash().ToString(), sigShare.getId().ToString(), sigShare.getMsgHash().ToString(), sigShare.getQuorumMember(), fromId);
511512
}
512513

513-
bool CSigSharesManager::PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,
514-
const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan)
514+
PreVerifyBatchedResult CSigSharesManager::PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,
515+
const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares)
515516
{
516-
retBan = false;
517-
518517
if (!IsQuorumActive(session.llmqType, quorum_manager, session.quorum->qc->quorumHash)) {
519518
// quorum is too old
520-
return false;
519+
return {PreVerifyResult::QuorumTooOld, false};
521520
}
522521
if (!session.quorum->IsMember(mn_activeman.GetProTxHash())) {
523522
// we're not a member so we can't verify it (we actually shouldn't have received it)
524-
return false;
523+
return {PreVerifyResult::NotAMember, false};
525524
}
526525
if (!session.quorum->HasVerificationVector()) {
527526
// TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG
528527
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, no verification possible.\n", __func__,
529528
session.quorumHash.ToString());
530-
return false;
529+
return {PreVerifyResult::MissingVerificationVector, false};
531530
}
532531

533532
std::unordered_set<uint16_t> dupMembers;
534533

535534
for (const auto& [quorumMember, _] : batchedSigShares.sigShares) {
536535
if (!dupMembers.emplace(quorumMember).second) {
537-
retBan = true;
538-
return false;
536+
return {PreVerifyResult::DuplicateMember, true};
539537
}
540538

541539
if (quorumMember >= session.quorum->members.size()) {
542540
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember out of bounds\n", __func__);
543-
retBan = true;
544-
return false;
541+
return {PreVerifyResult::QuorumMemberOutOfBounds, true};
545542
}
546543
if (!session.quorum->qc->validMembers[quorumMember]) {
547544
LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- quorumMember not valid\n", __func__);
548-
retBan = true;
549-
return false;
545+
return {PreVerifyResult::QuorumMemberNotValid, true};
550546
}
551547
}
552-
return true;
548+
return {PreVerifyResult::Success, false};
553549
}
554550

555551
bool CSigSharesManager::CollectPendingSigSharesToVerify(

src/llmq/signing_shares.h

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,23 @@ class CSignedSession
358358
int attempt{0};
359359
};
360360

361+
enum class PreVerifyResult {
362+
Success,
363+
QuorumTooOld,
364+
NotAMember,
365+
MissingVerificationVector,
366+
DuplicateMember,
367+
QuorumMemberOutOfBounds,
368+
QuorumMemberNotValid
369+
};
370+
371+
struct PreVerifyBatchedResult {
372+
PreVerifyResult result;
373+
bool should_ban;
374+
375+
[[nodiscard]] bool IsSuccess() const { return result == PreVerifyResult::Success; }
376+
};
377+
361378
class CSigSharesManager : public CRecoveredSigsListener
362379
{
363380
private:
@@ -451,8 +468,8 @@ class CSigSharesManager : public CRecoveredSigsListener
451468
void ProcessMessageSigShare(NodeId fromId, PeerManager& peerman, const CSigShare& sigShare);
452469

453470
static bool VerifySigSharesInv(Consensus::LLMQType llmqType, const CSigSharesInv& inv);
454-
static bool PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,
455-
const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares, bool& retBan);
471+
static PreVerifyBatchedResult PreVerifyBatchedSigShares(const CActiveMasternodeManager& mn_activeman, const CQuorumManager& quorum_manager,
472+
const CSigSharesNodeState::SessionInfo& session, const CBatchedSigShares& batchedSigShares);
456473

457474
bool CollectPendingSigSharesToVerify(
458475
size_t maxUniqueSessions, std::unordered_map<NodeId, std::vector<CSigShare>>& retSigShares,

0 commit comments

Comments
 (0)