From 379168c8fe6f932e4743a8be713ba0b0ba3488cd Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Nov 2025 18:58:24 -0600 Subject: [PATCH 01/10] refac: Replace return types with std::optional for recovered signature methods Updated methods in the CRecoveredSigsDb and CSigningManager classes to return std::optional types instead of using output parameters. This change simplifies error handling and improves code readability. Adjusted related logic in net_processing, signing, and RPC layers to accommodate the new return types. --- src/instantsend/signing.cpp | 7 ++--- src/llmq/signing.cpp | 61 +++++++++++++++++++++---------------- src/llmq/signing.h | 15 ++++----- src/llmq/signing_shares.cpp | 13 ++++---- src/net_processing.cpp | 5 ++- src/rpc/quorums.cpp | 5 +-- 6 files changed, 56 insertions(+), 50 deletions(-) diff --git a/src/instantsend/signing.cpp b/src/instantsend/signing.cpp index 789c0135f9685..f2cda67ec22e0 100644 --- a/src/instantsend/signing.cpp +++ b/src/instantsend/signing.cpp @@ -299,11 +299,10 @@ bool InstantSendSigner::TrySignInputLocks(const CTransaction& tx, bool fRetroact auto id = GenInputLockRequestId(in.prevout); ids.emplace_back(id); - uint256 otherTxHash; - if (m_sigman.GetVoteForId(params.llmqTypeDIP0024InstantSend, id, otherTxHash)) { - if (otherTxHash != tx.GetHash()) { + if (auto otherTxHashOpt = m_sigman.GetVoteForId(params.llmqTypeDIP0024InstantSend, id)) { + if (*otherTxHashOpt != tx.GetHash()) { LogPrintf("%s -- txid=%s: input %s is conflicting with previous vote for tx %s\n", __func__, - tx.GetHash().ToString(), in.prevout.ToStringShort(), otherTxHash.ToString()); + tx.GetHash().ToString(), in.prevout.ToStringShort(), otherTxHashOpt->ToString()); return false; } alreadyVotedCount++; diff --git a/src/llmq/signing.cpp b/src/llmq/signing.cpp index cd9bb07758b43..d42b347697464 100644 --- a/src/llmq/signing.cpp +++ b/src/llmq/signing.cpp @@ -23,6 +23,7 @@ #include #include +#include #include namespace llmq @@ -96,37 +97,38 @@ bool CRecoveredSigsDb::HasRecoveredSigForHash(const uint256& hash) const return ret; } -bool CRecoveredSigsDb::ReadRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const +std::optional CRecoveredSigsDb::ReadRecoveredSig(Consensus::LLMQType llmqType, const uint256& id) const { auto k = std::make_tuple(std::string("rs_r"), llmqType, id); CDataStream ds(SER_DISK, CLIENT_VERSION); if (!db->ReadDataStream(k, ds)) { - return false; + return std::nullopt; } try { + CRecoveredSig ret; ret.Unserialize(ds); - return true; + return ret; } catch (std::exception&) { - return false; + return std::nullopt; } } -bool CRecoveredSigsDb::GetRecoveredSigByHash(const uint256& hash, CRecoveredSig& ret) const +std::optional CRecoveredSigsDb::GetRecoveredSigByHash(const uint256& hash) const { auto k1 = std::make_tuple(std::string("rs_h"), hash); std::pair k2; if (!db->Read(k1, k2)) { - return false; + return std::nullopt; } - return ReadRecoveredSig(k2.first, k2.second, ret); + return ReadRecoveredSig(k2.first, k2.second); } -bool CRecoveredSigsDb::GetRecoveredSigById(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const +std::optional CRecoveredSigsDb::GetRecoveredSigById(Consensus::LLMQType llmqType, const uint256& id) const { - return ReadRecoveredSig(llmqType, id, ret); + return ReadRecoveredSig(llmqType, id); } void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig) @@ -168,10 +170,11 @@ void CRecoveredSigsDb::WriteRecoveredSig(const llmq::CRecoveredSig& recSig) void CRecoveredSigsDb::RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey) { - CRecoveredSig recSig; - if (!ReadRecoveredSig(llmqType, id, recSig)) { + auto recSigOpt = ReadRecoveredSig(llmqType, id); + if (!recSigOpt.has_value()) { return; } + const auto& recSig = *recSigOpt; auto signHash = recSig.buildSignHash(); @@ -271,10 +274,14 @@ bool CRecoveredSigsDb::HasVotedOnId(Consensus::LLMQType llmqType, const uint256& return db->Exists(k); } -bool CRecoveredSigsDb::GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet) const +std::optional CRecoveredSigsDb::GetVoteForId(Consensus::LLMQType llmqType, const uint256& id) const { auto k = std::make_tuple(std::string("rs_v"), llmqType, id); - return db->Read(k, msgHashRet); + uint256 msgHashRet; + if (!db->Read(k, msgHashRet)) { + return std::nullopt; + } + return msgHashRet; } void CRecoveredSigsDb::WriteVoteForId(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) @@ -357,16 +364,18 @@ bool CSigningManager::AlreadyHave(const CInv& inv) const return db.HasRecoveredSigForHash(inv.hash); } -bool CSigningManager::GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret) const +std::optional CSigningManager::GetRecoveredSigForGetData(const uint256& hash) const { - if (!db.GetRecoveredSigByHash(hash, ret)) { - return false; + auto retOpt = db.GetRecoveredSigByHash(hash); + if (!retOpt.has_value()) { + return std::nullopt; } + const auto& ret = *retOpt; if (!IsQuorumActive(ret.getLlmqType(), qman, ret.getQuorumHash())) { // we don't want to propagate sigs from inactive quorums - return false; + return std::nullopt; } - return true; + return ret; } static bool PreVerifyRecoveredSig(const CQuorumManager& quorum_manager, const CRecoveredSig& recoveredSig, bool& retBan) @@ -600,8 +609,9 @@ void CSigningManager::ProcessRecoveredSig(const std::shared_ptrgetId().ToString(), recoveredSig->getMsgHash().ToString()); if (db.HasRecoveredSigForId(llmqType, recoveredSig->getId())) { - CRecoveredSig otherRecoveredSig; - if (db.GetRecoveredSigById(llmqType, recoveredSig->getId(), otherRecoveredSig)) { + auto otherRecoveredSigOpt = db.GetRecoveredSigById(llmqType, recoveredSig->getId()); + if (otherRecoveredSigOpt.has_value()) { + const auto& otherRecoveredSig = *otherRecoveredSigOpt; auto otherSignHash = otherRecoveredSig.buildSignHash(); if (signHash.Get() != otherSignHash.Get()) { // this should really not happen, as each masternode is participating in only one vote, @@ -684,12 +694,9 @@ bool CSigningManager::HasRecoveredSigForSession(const uint256& signHash) const return db.HasRecoveredSigForSession(signHash); } -bool CSigningManager::GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id, llmq::CRecoveredSig& retRecSig) const +std::optional CSigningManager::GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const { - if (!db.GetRecoveredSigById(llmqType, id, retRecSig)) { - return false; - } - return true; + return db.GetRecoveredSigById(llmqType, id); } bool CSigningManager::IsConflicting(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const @@ -708,9 +715,9 @@ bool CSigningManager::IsConflicting(Consensus::LLMQType llmqType, const uint256& return false; } -bool CSigningManager::GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet) const +std::optional CSigningManager::GetVoteForId(Consensus::LLMQType llmqType, const uint256& id) const { - return db.GetVoteForId(llmqType, id, msgHashRet); + return db.GetVoteForId(llmqType, id); } void CSigningManager::StartWorkerThread(PeerManager& peerman) diff --git a/src/llmq/signing.h b/src/llmq/signing.h index 7e03fd89d6139..6babe7f8af37b 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -135,8 +136,8 @@ class CRecoveredSigsDb bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); bool HasRecoveredSigForSession(const uint256& signHash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); bool HasRecoveredSigForHash(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); - bool GetRecoveredSigByHash(const uint256& hash, CRecoveredSig& ret) const; - bool GetRecoveredSigById(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const; + std::optional GetRecoveredSigByHash(const uint256& hash) const; + std::optional GetRecoveredSigById(Consensus::LLMQType llmqType, const uint256& id) const; void WriteRecoveredSig(const CRecoveredSig& recSig) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); void TruncateRecoveredSig(Consensus::LLMQType llmqType, const uint256& id) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); @@ -144,13 +145,13 @@ class CRecoveredSigsDb // votes are removed when the recovered sig is written to the db bool HasVotedOnId(Consensus::LLMQType llmqType, const uint256& id) const; - bool GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet) const; + std::optional GetVoteForId(Consensus::LLMQType llmqType, const uint256& id) const; void WriteVoteForId(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash); void CleanupOldVotes(int64_t maxAge); private: - bool ReadRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& ret) const; + std::optional ReadRecoveredSig(Consensus::LLMQType llmqType, const uint256& id) const; void RemoveRecoveredSig(CDBBatch& batch, Consensus::LLMQType llmqType, const uint256& id, bool deleteHashKey, bool deleteTimeKey) EXCLUSIVE_LOCKS_REQUIRED(!cs_cache); }; @@ -192,7 +193,7 @@ class CSigningManager ~CSigningManager(); bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); - bool GetRecoveredSigForGetData(const uint256& hash, CRecoveredSig& ret) const; + std::optional GetRecoveredSigForGetData(const uint256& hash) const; [[nodiscard]] MessageProcessingResult ProcessMessage(NodeId from, std::string_view msg_type, CDataStream& vRecv) EXCLUSIVE_LOCKS_REQUIRED(!cs_pending); @@ -234,10 +235,10 @@ class CSigningManager bool HasRecoveredSig(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const; bool HasRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const; bool HasRecoveredSigForSession(const uint256& signHash) const; - bool GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id, CRecoveredSig& retRecSig) const; + std::optional GetRecoveredSigForId(Consensus::LLMQType llmqType, const uint256& id) const; bool IsConflicting(Consensus::LLMQType llmqType, const uint256& id, const uint256& msgHash) const; - bool GetVoteForId(Consensus::LLMQType llmqType, const uint256& id, uint256& msgHashRet) const; + std::optional GetVoteForId(Consensus::LLMQType llmqType, const uint256& id) const; private: std::thread workThread; diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 43bc21af4af93..4bebae68e4bf7 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -923,26 +923,25 @@ bool CSigSharesManager::AsyncSignIfMember(Consensus::LLMQType llmqType, CSigning auto& db = sigman.GetDb(); bool hasVoted = db.HasVotedOnId(llmqType, id); if (hasVoted) { - uint256 prevMsgHash; - db.GetVoteForId(llmqType, id, prevMsgHash); - if (msgHash != prevMsgHash) { + auto prevMsgHashOpt = db.GetVoteForId(llmqType, id); + if (prevMsgHashOpt.has_value() && msgHash != *prevMsgHashOpt) { if (allowDiffMsgHashSigning) { LogPrintf("%s -- already voted for id=%s and msgHash=%s. Signing for different " /* Continued */ "msgHash=%s\n", - __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); + __func__, id.ToString(), prevMsgHashOpt->ToString(), msgHash.ToString()); hasVoted = false; } else { LogPrintf("%s -- already voted for id=%s and msgHash=%s. Not voting on " /* Continued */ "conflicting msgHash=%s\n", - __func__, id.ToString(), prevMsgHash.ToString(), msgHash.ToString()); + __func__, id.ToString(), prevMsgHashOpt->ToString(), msgHash.ToString()); return false; } } else if (allowReSign) { LogPrint(BCLog::LLMQ, "%s -- already voted for id=%s and msgHash=%s. Resigning!\n", __func__, - id.ToString(), prevMsgHash.ToString()); + id.ToString(), prevMsgHashOpt.has_value() ? prevMsgHashOpt->ToString() : "unknown"); } else { LogPrint(BCLog::LLMQ, "%s -- already voted for id=%s and msgHash=%s. Not voting again.\n", __func__, - id.ToString(), prevMsgHash.ToString()); + id.ToString(), prevMsgHashOpt.has_value() ? prevMsgHashOpt->ToString() : "unknown"); return false; } } diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 392d1d41ef35f..ddccb7ffe8152 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2925,9 +2925,8 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } if (!push && (inv.type == MSG_QUORUM_RECOVERED_SIG)) { - llmq::CRecoveredSig o; - if (m_llmq_ctx->sigman->GetRecoveredSigForGetData(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QSIGREC, o)); + if (auto opt_rec_sig = m_llmq_ctx->sigman->GetRecoveredSigForGetData(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QSIGREC, *opt_rec_sig)); push = true; } } diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index fbac23cd76dc3..db632f8f12070 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -750,10 +750,11 @@ static RPCHelpMan quorum_getrecsig() const uint256 id(ParseHashV(request.params[1], "id")); const uint256 msgHash(ParseHashV(request.params[2], "msgHash")); - llmq::CRecoveredSig recSig; - if (!llmq_ctx.sigman->GetRecoveredSigForId(llmqType, id, recSig)) { + auto recSigOpt = llmq_ctx.sigman->GetRecoveredSigForId(llmqType, id); + if (!recSigOpt.has_value()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "recovered signature not found"); } + const auto& recSig = *recSigOpt; if (recSig.getMsgHash() != msgHash) { throw JSONRPCError(RPC_INVALID_PARAMETER, "recovered signature not found"); } From 52748fc4f8e31891ed60e4f4d3d36f20247764d6 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Nov 2025 19:15:21 -0600 Subject: [PATCH 02/10] refac: Update methods to return std::optional for DKG-related data retrieval Refactored several methods in the DKG session and block processor classes to return std::optional types instead of using output parameters. This change enhances error handling and code clarity. Adjusted related logic in net processing and chain lock handling to accommodate the new return types. --- src/Makefile.am | 1 + src/chainlock/chainlock.cpp | 7 +- src/chainlock/chainlock.h | 3 +- src/instantsend/instantsend.cpp | 9 +- src/instantsend/instantsend.h | 3 +- src/llmq/blockprocessor.cpp | 7 +- src/llmq/blockprocessor.h | 2 +- src/llmq/dkgsession.h | 168 +------------------------------- src/llmq/dkgsessionhandler.cpp | 28 +++--- src/llmq/dkgsessionhandler.h | 9 +- src/llmq/dkgsessionmgr.cpp | 40 ++++---- src/llmq/dkgsessionmgr.h | 10 +- src/net_processing.cpp | 36 +++---- 13 files changed, 74 insertions(+), 249 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 9a3b18c65482a..ccb3440962d98 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -262,6 +262,7 @@ BITCOIN_CORE_H = \ llmq/commitment.h \ llmq/context.h \ llmq/debug.h \ + llmq/dkgtypes.h \ llmq/dkgsession.h \ llmq/dkgsessionhandler.h \ llmq/dkgsessionmgr.h \ diff --git a/src/chainlock/chainlock.cpp b/src/chainlock/chainlock.cpp index 8ea6e20c39122..a2c9fb8eb8baf 100644 --- a/src/chainlock/chainlock.cpp +++ b/src/chainlock/chainlock.cpp @@ -94,17 +94,16 @@ bool CChainLocksHandler::AlreadyHave(const CInv& inv) const return seenChainLocks.count(inv.hash) != 0; } -bool CChainLocksHandler::GetChainLockByHash(const uint256& hash, chainlock::ChainLockSig& ret) const +std::optional CChainLocksHandler::GetChainLockByHash(const uint256& hash) const { LOCK(cs); if (hash != bestChainLockHash) { // we only propagate the best one and ditch all the old ones - return false; + return std::nullopt; } - ret = bestChainLock; - return true; + return bestChainLock; } chainlock::ChainLockSig CChainLocksHandler::GetBestChainLock() const diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h index 14c9aa9a3f55a..527559d6b4c87 100644 --- a/src/chainlock/chainlock.h +++ b/src/chainlock/chainlock.h @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -91,7 +92,7 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool GetChainLockByHash(const uint256& hash, chainlock::ChainLockSig& ret) const + std::optional GetChainLockByHash(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); chainlock::ChainLockSig GetBestChainLock() const EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/instantsend/instantsend.cpp b/src/instantsend/instantsend.cpp index 0415a19d53776..30d61a8a7ce12 100644 --- a/src/instantsend/instantsend.cpp +++ b/src/instantsend/instantsend.cpp @@ -617,10 +617,10 @@ bool CInstantSendManager::AlreadyHave(const CInv& inv) const db.KnownInstantSendLock(inv.hash); } -bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, instantsend::InstantSendLock& ret) const +std::optional CInstantSendManager::GetInstantSendLockByHash(const uint256& hash) const { if (!IsInstantSendEnabled()) { - return false; + return std::nullopt; } auto islock = db.GetInstantSendLockByHash(hash); @@ -634,12 +634,11 @@ bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, instants if (itNoTx != pendingNoTxInstantSendLocks.end()) { islock = itNoTx->second.islock; } else { - return false; + return std::nullopt; } } } - ret = *islock; - return true; + return *islock; } instantsend::InstantSendLockPtr CInstantSendManager::GetInstantSendLockByTxid(const uint256& txid) const diff --git a/src/instantsend/instantsend.h b/src/instantsend/instantsend.h index 30e8ba4a8ebfd..50fc56e6bec52 100644 --- a/src/instantsend/instantsend.h +++ b/src/instantsend/instantsend.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -152,7 +153,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent void BlockDisconnected(const std::shared_ptr& pblock, const CBlockIndex* pindexDisconnected); bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); - bool GetInstantSendLockByHash(const uint256& hash, instantsend::InstantSendLock& ret) const + std::optional GetInstantSendLockByHash(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks); instantsend::InstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const; diff --git a/src/llmq/blockprocessor.cpp b/src/llmq/blockprocessor.cpp index 0baf23fb9abff..832032d458831 100644 --- a/src/llmq/blockprocessor.cpp +++ b/src/llmq/blockprocessor.cpp @@ -720,15 +720,14 @@ std::optional CQuorumBlockProcessor::AddMineableCommitment(const CFinalCom return relay ? std::make_optional(CInv{MSG_QUORUM_FINAL_COMMITMENT, commitmentHash}) : std::nullopt; } -bool CQuorumBlockProcessor::GetMineableCommitmentByHash(const uint256& commitmentHash, llmq::CFinalCommitment& ret) const +std::optional CQuorumBlockProcessor::GetMineableCommitmentByHash(const uint256& commitmentHash) const { LOCK(minableCommitmentsCs); auto it = minableCommitments.find(commitmentHash); if (it == minableCommitments.end()) { - return false; + return std::nullopt; } - ret = it->second; - return true; + return it->second; } // Will return nullopt if no commitment should be mined diff --git a/src/llmq/blockprocessor.h b/src/llmq/blockprocessor.h index 97e4edf3f30e1..71729f2017bc3 100644 --- a/src/llmq/blockprocessor.h +++ b/src/llmq/blockprocessor.h @@ -73,7 +73,7 @@ class CQuorumBlockProcessor //! it returns hash of commitment if it should be relay, otherwise nullopt std::optional AddMineableCommitment(const CFinalCommitment& fqc) EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); bool HasMineableCommitment(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); - bool GetMineableCommitmentByHash(const uint256& commitmentHash, CFinalCommitment& ret) const + std::optional GetMineableCommitmentByHash(const uint256& commitmentHash) const EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs); std::optional> GetMineableCommitments(const Consensus::LLMQParams& llmqParams, int nHeight) const diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index 2b1b095b93fe0..e4d8f9119ea4c 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -6,6 +6,7 @@ #define BITCOIN_LLMQ_DKGSESSION_H #include +#include #include #include @@ -38,173 +39,6 @@ class CDKGSessionManager; class CDKGPendingMessages; class CQuorumSnapshotManager; -class CDKGContribution -{ -public: - Consensus::LLMQType llmqType; - uint256 quorumHash; - uint256 proTxHash; - BLSVerificationVectorPtr vvec; - std::shared_ptr> contributions; - CBLSSignature sig; - -public: - template - inline void SerializeWithoutSig(Stream& s) const - { - s << ToUnderlying(llmqType); - s << quorumHash; - s << proTxHash; - s << *vvec; - s << *contributions; - } - template - inline void Serialize(Stream& s) const - { - SerializeWithoutSig(s); - s << sig; - } - template - inline void Unserialize(Stream& s) - { - std::vector tmp1; - CBLSIESMultiRecipientObjects tmp2; - - s >> llmqType; - s >> quorumHash; - s >> proTxHash; - s >> tmp1; - s >> tmp2; - s >> sig; - - vvec = std::make_shared>(std::move(tmp1)); - contributions = std::make_shared>(std::move(tmp2)); - } - - [[nodiscard]] uint256 GetSignHash() const - { - CHashWriter hw(SER_GETHASH, 0); - SerializeWithoutSig(hw); - hw << CBLSSignature(); - return hw.GetHash(); - } -}; - -class CDKGComplaint -{ -public: - Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE}; - uint256 quorumHash; - uint256 proTxHash; - std::vector badMembers; - std::vector complainForMembers; - CBLSSignature sig; - -public: - CDKGComplaint() = default; - explicit CDKGComplaint(const Consensus::LLMQParams& params) : - badMembers((size_t)params.size), complainForMembers((size_t)params.size) {}; - - SERIALIZE_METHODS(CDKGComplaint, obj) - { - READWRITE( - obj.llmqType, - obj.quorumHash, - obj.proTxHash, - DYNBITSET(obj.badMembers), - DYNBITSET(obj.complainForMembers), - obj.sig - ); - } - - [[nodiscard]] uint256 GetSignHash() const - { - CDKGComplaint tmp(*this); - tmp.sig = CBLSSignature(); - return ::SerializeHash(tmp); - } -}; - -class CDKGJustification -{ -public: - Consensus::LLMQType llmqType; - uint256 quorumHash; - uint256 proTxHash; - struct Contribution { - uint32_t index; - CBLSSecretKey key; - SERIALIZE_METHODS(Contribution, obj) - { - READWRITE(obj.index, obj.key); - } - }; - std::vector contributions; - CBLSSignature sig; - -public: - SERIALIZE_METHODS(CDKGJustification, obj) - { - READWRITE(obj.llmqType, obj.quorumHash, obj.proTxHash, obj.contributions, obj.sig); - } - - [[nodiscard]] uint256 GetSignHash() const - { - CDKGJustification tmp(*this); - tmp.sig = CBLSSignature(); - return ::SerializeHash(tmp); - } -}; - -// each member commits to a single set of valid members with this message -// then each node aggregate all received premature commitments -// into a single CFinalCommitment, which is only valid if -// enough (>=minSize) premature commitments were aggregated -class CDKGPrematureCommitment -{ -public: - Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE}; - uint256 quorumHash; - uint256 proTxHash; - std::vector validMembers; - - CBLSPublicKey quorumPublicKey; - uint256 quorumVvecHash; - - CBLSSignature quorumSig; // threshold sig share of quorumHash+validMembers+pubKeyHash+vvecHash - CBLSSignature sig; // single member sig of quorumHash+validMembers+pubKeyHash+vvecHash - -public: - CDKGPrematureCommitment() = default; - explicit CDKGPrematureCommitment(const Consensus::LLMQParams& params) : - validMembers((size_t)params.size) {}; - - [[nodiscard]] int CountValidMembers() const - { - return int(std::count(validMembers.begin(), validMembers.end(), true)); - } - -public: - SERIALIZE_METHODS(CDKGPrematureCommitment, obj) - { - READWRITE( - obj.llmqType, - obj.quorumHash, - obj.proTxHash, - DYNBITSET(obj.validMembers), - obj.quorumPublicKey, - obj.quorumVvecHash, - obj.quorumSig, - obj.sig - ); - } - - [[nodiscard]] uint256 GetSignHash() const - { - return BuildCommitmentHash(llmqType, quorumHash, validMembers, quorumPublicKey, quorumVvecHash); - } -}; - class CDKGMember { public: diff --git a/src/llmq/dkgsessionhandler.cpp b/src/llmq/dkgsessionhandler.cpp index 1a200be7cf341..5f626e502d547 100644 --- a/src/llmq/dkgsessionhandler.cpp +++ b/src/llmq/dkgsessionhandler.cpp @@ -627,48 +627,44 @@ void CDKGSessionHandler::PhaseHandlerThread(CConnman& connman, PeerManager& peer } } -bool CDKGSessionHandler::GetContribution(const uint256& hash, CDKGContribution& ret) const +std::optional CDKGSessionHandler::GetContribution(const uint256& hash) const { LOCK(curSession->invCs); auto it = curSession->contributions.find(hash); if (it != curSession->contributions.end()) { - ret = it->second; - return true; + return it->second; } - return false; + return std::nullopt; } -bool CDKGSessionHandler::GetComplaint(const uint256& hash, CDKGComplaint& ret) const +std::optional CDKGSessionHandler::GetComplaint(const uint256& hash) const { LOCK(curSession->invCs); auto it = curSession->complaints.find(hash); if (it != curSession->complaints.end()) { - ret = it->second; - return true; + return it->second; } - return false; + return std::nullopt; } -bool CDKGSessionHandler::GetJustification(const uint256& hash, CDKGJustification& ret) const +std::optional CDKGSessionHandler::GetJustification(const uint256& hash) const { LOCK(curSession->invCs); auto it = curSession->justifications.find(hash); if (it != curSession->justifications.end()) { - ret = it->second; - return true; + return it->second; } - return false; + return std::nullopt; } -bool CDKGSessionHandler::GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const +std::optional CDKGSessionHandler::GetPrematureCommitment(const uint256& hash) const { LOCK(curSession->invCs); auto it = curSession->prematureCommitments.find(hash); if (it != curSession->prematureCommitments.end() && curSession->validCommitments.count(hash)) { - ret = it->second; - return true; + return it->second; } - return false; + return std::nullopt; } } // namespace llmq diff --git a/src/llmq/dkgsessionhandler.h b/src/llmq/dkgsessionhandler.h index e40611a0238a9..78f128259dabb 100644 --- a/src/llmq/dkgsessionhandler.h +++ b/src/llmq/dkgsessionhandler.h @@ -7,6 +7,7 @@ #include +#include #include // for NodeId #include @@ -175,10 +176,10 @@ class CDKGSessionHandler void StartThread(CConnman& connman, PeerManager& peerman); void StopThread(); - bool GetContribution(const uint256& hash, CDKGContribution& ret) const; - bool GetComplaint(const uint256& hash, CDKGComplaint& ret) const; - bool GetJustification(const uint256& hash, CDKGJustification& ret) const; - bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const; + std::optional GetContribution(const uint256& hash) const; + std::optional GetComplaint(const uint256& hash) const; + std::optional GetJustification(const uint256& hash) const; + std::optional GetPrematureCommitment(const uint256& hash) const; private: bool InitNewQuorum(const CBlockIndex* pQuorumBaseBlockIndex); diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 3c2fdc135fbc0..68c0323d1aaaf 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -203,10 +203,10 @@ bool CDKGSessionManager::AlreadyHave(const CInv& inv) const return false; } -bool CDKGSessionManager::GetContribution(const uint256& hash, CDKGContribution& ret) const +std::optional CDKGSessionManager::GetContribution(const uint256& hash) const { if (!IsQuorumDKGEnabled(spork_manager)) - return false; + return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -214,17 +214,17 @@ bool CDKGSessionManager::GetContribution(const uint256& hash, CDKGContribution& if (dkgType.phase < QuorumPhase::Initialized || dkgType.phase > QuorumPhase::Contribute) { continue; } - if (dkgType.GetContribution(hash, ret)) { - return true; + if (auto opt = dkgType.GetContribution(hash)) { + return opt; } } - return false; + return std::nullopt; } -bool CDKGSessionManager::GetComplaint(const uint256& hash, CDKGComplaint& ret) const +std::optional CDKGSessionManager::GetComplaint(const uint256& hash) const { if (!IsQuorumDKGEnabled(spork_manager)) - return false; + return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -232,17 +232,17 @@ bool CDKGSessionManager::GetComplaint(const uint256& hash, CDKGComplaint& ret) c if (dkgType.phase < QuorumPhase::Contribute || dkgType.phase > QuorumPhase::Complain) { continue; } - if (dkgType.GetComplaint(hash, ret)) { - return true; + if (auto opt = dkgType.GetComplaint(hash)) { + return opt; } } - return false; + return std::nullopt; } -bool CDKGSessionManager::GetJustification(const uint256& hash, CDKGJustification& ret) const +std::optional CDKGSessionManager::GetJustification(const uint256& hash) const { if (!IsQuorumDKGEnabled(spork_manager)) - return false; + return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -250,17 +250,17 @@ bool CDKGSessionManager::GetJustification(const uint256& hash, CDKGJustification if (dkgType.phase < QuorumPhase::Complain || dkgType.phase > QuorumPhase::Justify) { continue; } - if (dkgType.GetJustification(hash, ret)) { - return true; + if (auto opt = dkgType.GetJustification(hash)) { + return opt; } } - return false; + return std::nullopt; } -bool CDKGSessionManager::GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const +std::optional CDKGSessionManager::GetPrematureCommitment(const uint256& hash) const { if (!IsQuorumDKGEnabled(spork_manager)) - return false; + return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -268,11 +268,11 @@ bool CDKGSessionManager::GetPrematureCommitment(const uint256& hash, CDKGPrematu if (dkgType.phase < QuorumPhase::Justify || dkgType.phase > QuorumPhase::Commit) { continue; } - if (dkgType.GetPrematureCommitment(hash, ret)) { - return true; + if (auto opt = dkgType.GetPrematureCommitment(hash)) { + return opt; } } - return false; + return std::nullopt; } void CDKGSessionManager::WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec) diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index 806d82eabaaee..b45f27b223e76 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -7,12 +7,14 @@ #include #include +#include #include #include #include #include #include +#include #include template @@ -100,10 +102,10 @@ class CDKGSessionManager [[nodiscard]] MessageProcessingResult ProcessMessage(CNode& pfrom, bool is_masternode, std::string_view msg_type, CDataStream& vRecv); bool AlreadyHave(const CInv& inv) const; - bool GetContribution(const uint256& hash, CDKGContribution& ret) const; - bool GetComplaint(const uint256& hash, CDKGComplaint& ret) const; - bool GetJustification(const uint256& hash, CDKGJustification& ret) const; - bool GetPrematureCommitment(const uint256& hash, CDKGPrematureCommitment& ret) const; + std::optional GetContribution(const uint256& hash) const; + std::optional GetComplaint(const uint256& hash) const; + std::optional GetJustification(const uint256& hash) const; + std::optional GetPrematureCommitment(const uint256& hash) const; // Contributions are written while in the DKG void WriteVerifiedVvecContribution(Consensus::LLMQType llmqType, const CBlockIndex* pQuorumBaseBlockIndex, const uint256& proTxHash, const BLSVerificationVectorPtr& vvec); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index ddccb7ffe8152..eba5812c296f8 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2884,42 +2884,36 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } if (!push && (inv.type == MSG_QUORUM_FINAL_COMMITMENT)) { - llmq::CFinalCommitment o; - if (m_llmq_ctx->quorum_block_processor->GetMineableCommitmentByHash( - inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QFCOMMITMENT, o)); + if (auto opt_commitment = m_llmq_ctx->quorum_block_processor->GetMineableCommitmentByHash(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QFCOMMITMENT, *opt_commitment)); push = true; } } if (!push && (inv.type == MSG_QUORUM_CONTRIB)) { - llmq::CDKGContribution o; - if (m_llmq_ctx->qdkgsman->GetContribution(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QCONTRIB, o)); + if (auto opt_contrib = m_llmq_ctx->qdkgsman->GetContribution(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QCONTRIB, *opt_contrib)); push = true; } } if (!push && (inv.type == MSG_QUORUM_COMPLAINT)) { - llmq::CDKGComplaint o; - if (m_llmq_ctx->qdkgsman->GetComplaint(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QCOMPLAINT, o)); + if (auto opt_complaint = m_llmq_ctx->qdkgsman->GetComplaint(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QCOMPLAINT, *opt_complaint)); push = true; } } if (!push && (inv.type == MSG_QUORUM_JUSTIFICATION)) { - llmq::CDKGJustification o; - if (m_llmq_ctx->qdkgsman->GetJustification(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QJUSTIFICATION, o)); + if (auto opt_justification = m_llmq_ctx->qdkgsman->GetJustification(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QJUSTIFICATION, *opt_justification)); push = true; } } if (!push && (inv.type == MSG_QUORUM_PREMATURE_COMMITMENT)) { - llmq::CDKGPrematureCommitment o; - if (m_llmq_ctx->qdkgsman->GetPrematureCommitment(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QPCOMMITMENT, o)); + if (auto opt_premature = m_llmq_ctx->qdkgsman->GetPrematureCommitment(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::QPCOMMITMENT, *opt_premature)); push = true; } } @@ -2932,17 +2926,15 @@ void PeerManagerImpl::ProcessGetData(CNode& pfrom, Peer& peer, const std::atomic } if (!push && (inv.type == MSG_CLSIG)) { - chainlock::ChainLockSig o; - if (m_llmq_ctx->clhandler->GetChainLockByHash(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CLSIG, o)); + if (auto opt_clsig = m_llmq_ctx->clhandler->GetChainLockByHash(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::CLSIG, *opt_clsig)); push = true; } } if (!push && inv.type == MSG_ISDLOCK) { - instantsend::InstantSendLock o; - if (m_llmq_ctx->isman->GetInstantSendLockByHash(inv.hash, o)) { - m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::ISDLOCK, o)); + if (auto opt_islock = m_llmq_ctx->isman->GetInstantSendLockByHash(inv.hash)) { + m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::ISDLOCK, *opt_islock)); push = true; } } From 95e9f43e0982271f06dc57d867c724486e68fb1b Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Nov 2025 19:28:08 -0600 Subject: [PATCH 03/10] refac: Change session info retrieval methods to return std::optional Updated methods in CSigSharesNodeState and CSigSharesManager to return std::optional types instead of using output parameters for session information retrieval. This refactor enhances error handling and improves code clarity, aligning with recent changes in the codebase. --- src/llmq/signing_shares.cpp | 172 +++++++++++++++++------------------- src/llmq/signing_shares.h | 4 +- src/masternode/node.cpp | 15 +++- src/masternode/node.h | 4 +- 4 files changed, 99 insertions(+), 96 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 4bebae68e4bf7..19bef0b788108 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -152,20 +152,20 @@ CSigSharesNodeState::Session* CSigSharesNodeState::GetSessionByRecvId(uint32_t s return it->second; } -bool CSigSharesNodeState::GetSessionInfoByRecvId(uint32_t sessionId, SessionInfo& retInfo) +std::optional CSigSharesNodeState::GetSessionInfoByRecvId(uint32_t sessionId) { const auto* s = GetSessionByRecvId(sessionId); if (s == nullptr) { - return false; + return std::nullopt; } + SessionInfo retInfo; retInfo.llmqType = s->llmqType; retInfo.quorumHash = s->quorumHash; retInfo.id = s->id; retInfo.msgHash = s->msgHash; retInfo.signHash = s->signHash; retInfo.quorum = s->quorum; - - return true; + return retInfo; } void CSigSharesNodeState::RemoveSession(const uint256& signHash) @@ -352,122 +352,116 @@ bool CSigSharesManager::VerifySigSharesInv(Consensus::LLMQType llmqType, const C bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) { - CSigSharesNodeState::SessionInfo sessionInfo; - if (!GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId, sessionInfo)) { - return true; - } - - if (!VerifySigSharesInv(sessionInfo.llmqType, inv)) { - return false; - } + if (auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId)) { + if (!VerifySigSharesInv(sessionInfo->llmqType, inv)) { + return false; + } - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForSession(sessionInfo.signHash.Get())) { - return true; - } + // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig + if (sigman.HasRecoveredSigForSession(sessionInfo->signHash.Get())) { + return true; + } - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo.signHash.ToString(), inv.ToString(), pfrom.GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, + sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); - if (!sessionInfo.quorum->HasVerificationVector()) { - // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", __func__, - sessionInfo.quorumHash.ToString(), pfrom.GetId()); - return true; - } + if (!sessionInfo->quorum->HasVerificationVector()) { + // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", __func__, + sessionInfo->quorumHash.ToString(), pfrom.GetId()); + return true; + } - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto* session = nodeState.GetSessionByRecvId(inv.sessionId); - if (session == nullptr) { + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; + auto* session = nodeState.GetSessionByRecvId(inv.sessionId); + if (session == nullptr) { + return true; + } + session->announced.Merge(inv); + session->knows.Merge(inv); return true; } - session->announced.Merge(inv); - session->knows.Merge(inv); return true; } bool CSigSharesManager::ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv) { - CSigSharesNodeState::SessionInfo sessionInfo; - if (!GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId, sessionInfo)) { - return true; - } - - if (!VerifySigSharesInv(sessionInfo.llmqType, inv)) { - return false; - } + if (auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId)) { + if (!VerifySigSharesInv(sessionInfo->llmqType, inv)) { + return false; + } - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForSession(sessionInfo.signHash.Get())) { - return true; - } + // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig + if (sigman.HasRecoveredSigForSession(sessionInfo->signHash.Get())) { + return true; + } - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo.signHash.ToString(), inv.ToString(), pfrom.GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, + sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto* session = nodeState.GetSessionByRecvId(inv.sessionId); - if (session == nullptr) { + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; + auto* session = nodeState.GetSessionByRecvId(inv.sessionId); + if (session == nullptr) { + return true; + } + session->requested.Merge(inv); + session->knows.Merge(inv); return true; } - session->requested.Merge(inv); - session->knows.Merge(inv); return true; } bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) { - CSigSharesNodeState::SessionInfo sessionInfo; - if (!GetSessionInfoByRecvId(pfrom.GetId(), batchedSigShares.sessionId, sessionInfo)) { - return true; - } + if (auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), batchedSigShares.sessionId)) { + if (bool ban{false}; !PreVerifyBatchedSigShares(m_mn_activeman, qman, *sessionInfo, batchedSigShares, ban)) { + return !ban; + } - if (bool ban{false}; !PreVerifyBatchedSigShares(m_mn_activeman, qman, sessionInfo, batchedSigShares, ban)) { - return !ban; - } + std::vector sigSharesToProcess; + sigSharesToProcess.reserve(batchedSigShares.sigShares.size()); - std::vector sigSharesToProcess; - sigSharesToProcess.reserve(batchedSigShares.sigShares.size()); + { + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; - { - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; + for (const auto& sigSharetmp : batchedSigShares.sigShares) { + CSigShare sigShare = RebuildSigShare(*sessionInfo, sigSharetmp); + nodeState.requestedSigShares.Erase(sigShare.GetKey()); + + // TODO track invalid sig shares received for PoSe? + // It's important to only skip seen *valid* sig shares here. If a node sends us a + // batch of mostly valid sig shares with a single invalid one and thus batched + // verification fails, we'd skip the valid ones in the future if received from other nodes + if (sigShares.Has(sigShare.GetKey())) { + continue; + } - for (const auto& sigSharetmp : batchedSigShares.sigShares) { - CSigShare sigShare = RebuildSigShare(sessionInfo, sigSharetmp); - nodeState.requestedSigShares.Erase(sigShare.GetKey()); + // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig + if (sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId())) { + continue; + } - // TODO track invalid sig shares received for PoSe? - // It's important to only skip seen *valid* sig shares here. If a node sends us a - // batch of mostly valid sig shares with a single invalid one and thus batched - // verification fails, we'd skip the valid ones in the future if received from other nodes - if (sigShares.Has(sigShare.GetKey())) { - continue; + sigSharesToProcess.emplace_back(sigShare); } + } - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId())) { - continue; - } + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", __func__, + sessionInfo->signHash.ToString(), batchedSigShares.sigShares.size(), sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); - sigSharesToProcess.emplace_back(sigShare); + if (sigSharesToProcess.empty()) { + return true; } - } - - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", __func__, - sessionInfo.signHash.ToString(), batchedSigShares.sigShares.size(), sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); - if (sigSharesToProcess.empty()) { + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; + for (const auto& s : sigSharesToProcess) { + nodeState.pendingIncomingSigShares.Add(s.GetKey(), s); + } return true; } - - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - for (const auto& s : sigSharesToProcess) { - nodeState.pendingIncomingSigShares.Add(s.GetKey(), s); - } return true; } @@ -1379,10 +1373,10 @@ bool CSigSharesManager::SendMessages() return didSend; } -bool CSigSharesManager::GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo) +std::optional CSigSharesManager::GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId) { LOCK(cs); - return nodeStates[nodeId].GetSessionInfoByRecvId(sessionId, retInfo); + return nodeStates[nodeId].GetSessionInfoByRecvId(sessionId); } CSigShare CSigSharesManager::RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair& in) diff --git a/src/llmq/signing_shares.h b/src/llmq/signing_shares.h index d729fe31f4427..13d97547e6e3d 100644 --- a/src/llmq/signing_shares.h +++ b/src/llmq/signing_shares.h @@ -345,7 +345,7 @@ class CSigSharesNodeState Session& GetOrCreateSessionFromAnn(const CSigSesAnn& ann); Session* GetSessionBySignHash(const uint256& signHash); Session* GetSessionByRecvId(uint32_t sessionId); - bool GetSessionInfoByRecvId(uint32_t sessionId, SessionInfo& retInfo); + std::optional GetSessionInfoByRecvId(uint32_t sessionId); void RemoveSession(const uint256& signHash); }; @@ -478,7 +478,7 @@ class CSigSharesManager : public CRecoveredSigsListener void ProcessSigShare(const CSigShare& sigShare, const CQuorumCPtr& quorum) EXCLUSIVE_LOCKS_REQUIRED(!cs); void TryRecoverSig(const CQuorum& quorum, const uint256& id, const uint256& msgHash) EXCLUSIVE_LOCKS_REQUIRED(!cs); - bool GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId, CSigSharesNodeState::SessionInfo& retInfo) + std::optional GetSessionInfoByRecvId(NodeId nodeId, uint32_t sessionId) EXCLUSIVE_LOCKS_REQUIRED(!cs); static CSigShare RebuildSigShare(const CSigSharesNodeState::SessionInfo& session, const std::pair& in); diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index 7c48ce7f4fbae..ff8b8b2c6e894 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -123,7 +123,9 @@ void CActiveMasternodeManager::InitInternal(const CBlockIndex* pindex) return; } - if (!GetLocalAddress(m_info.service)) { + if (auto opt_addr = GetLocalAddress()) { + m_info.service = *opt_addr; + } else { m_state = MasternodeState::SOME_ERROR; return; } @@ -213,7 +215,7 @@ void CActiveMasternodeManager::UpdatedBlockTip(const CBlockIndex* pindexNew, con } } -bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet) +std::optional CActiveMasternodeManager::GetLocalAddress() { AssertLockHeld(cs); // First try to find whatever our own local address is known internally. @@ -221,6 +223,7 @@ bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet) // or added by TorController. Use some random dummy IPv4 peer to prefer the one // reachable via IPv4. bool fFoundLocal{false}; + CService addrRet; if (auto peerAddr = LookupHost("8.8.8.8", false); peerAddr.has_value()) { fFoundLocal = GetLocal(addrRet, &peerAddr.value()) && IsValidNetAddr(addrRet); } @@ -247,10 +250,13 @@ bool CActiveMasternodeManager::GetLocalAddress(CService& addrRet) if (empty) { m_error = "Can't detect valid external address. Please consider using the externalip configuration option if problem persists. Make sure to use IPv4 address only."; LogPrintf("CActiveMasternodeManager::GetLocalAddress -- ERROR: %s\n", m_error); - return false; + return std::nullopt; } } - return true; + if (!fFoundLocal) { + return std::nullopt; + } + return addrRet; } bool CActiveMasternodeManager::IsValidNetAddr(const CService& addrIn) @@ -294,3 +300,4 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjec READ_LOCK(cs); return m_info.blsPubKeyOperator; } + diff --git a/src/masternode/node.h b/src/masternode/node.h index 980a2f96814dd..74d81a3fca0c6 100644 --- a/src/masternode/node.h +++ b/src/masternode/node.h @@ -11,6 +11,8 @@ #include #include +#include + class CConnman; class CDeterministicMNManager; @@ -81,7 +83,7 @@ class CActiveMasternodeManager private: void InitInternal(const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs); - bool GetLocalAddress(CService& addrRet) EXCLUSIVE_LOCKS_REQUIRED(cs); + std::optional GetLocalAddress() EXCLUSIVE_LOCKS_REQUIRED(cs); }; #endif // BITCOIN_MASTERNODE_NODE_H From de63190dae279217adb21c9ee1c80f36d89b16ca Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Nov 2025 19:28:39 -0600 Subject: [PATCH 04/10] chore: clang format --- src/chainlock/chainlock.h | 3 +-- src/llmq/dkgsessionmgr.cpp | 12 ++++-------- src/llmq/dkgsessionmgr.h | 2 +- src/llmq/signing_shares.cpp | 15 +++++++++------ src/masternode/node.cpp | 1 - 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h index 527559d6b4c87..f75735d6e62b7 100644 --- a/src/chainlock/chainlock.h +++ b/src/chainlock/chainlock.h @@ -92,8 +92,7 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs); - std::optional GetChainLockByHash(const uint256& hash) const - EXCLUSIVE_LOCKS_REQUIRED(!cs); + std::optional GetChainLockByHash(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(!cs); chainlock::ChainLockSig GetBestChainLock() const EXCLUSIVE_LOCKS_REQUIRED(!cs); void UpdateTxFirstSeenMap(const Uint256HashSet& tx, const int64_t& time) override EXCLUSIVE_LOCKS_REQUIRED(!cs); diff --git a/src/llmq/dkgsessionmgr.cpp b/src/llmq/dkgsessionmgr.cpp index 68c0323d1aaaf..799db168b2c67 100644 --- a/src/llmq/dkgsessionmgr.cpp +++ b/src/llmq/dkgsessionmgr.cpp @@ -205,8 +205,7 @@ bool CDKGSessionManager::AlreadyHave(const CInv& inv) const std::optional CDKGSessionManager::GetContribution(const uint256& hash) const { - if (!IsQuorumDKGEnabled(spork_manager)) - return std::nullopt; + if (!IsQuorumDKGEnabled(spork_manager)) return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -223,8 +222,7 @@ std::optional CDKGSessionManager::GetContribution(const uint25 std::optional CDKGSessionManager::GetComplaint(const uint256& hash) const { - if (!IsQuorumDKGEnabled(spork_manager)) - return std::nullopt; + if (!IsQuorumDKGEnabled(spork_manager)) return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -241,8 +239,7 @@ std::optional CDKGSessionManager::GetComplaint(const uint256& has std::optional CDKGSessionManager::GetJustification(const uint256& hash) const { - if (!IsQuorumDKGEnabled(spork_manager)) - return std::nullopt; + if (!IsQuorumDKGEnabled(spork_manager)) return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; @@ -259,8 +256,7 @@ std::optional CDKGSessionManager::GetJustification(const uint std::optional CDKGSessionManager::GetPrematureCommitment(const uint256& hash) const { - if (!IsQuorumDKGEnabled(spork_manager)) - return std::nullopt; + if (!IsQuorumDKGEnabled(spork_manager)) return std::nullopt; for (const auto& p : dkgSessionHandlers) { const auto& dkgType = p.second; diff --git a/src/llmq/dkgsessionmgr.h b/src/llmq/dkgsessionmgr.h index b45f27b223e76..bed8022a83de4 100644 --- a/src/llmq/dkgsessionmgr.h +++ b/src/llmq/dkgsessionmgr.h @@ -7,8 +7,8 @@ #include #include -#include #include +#include #include #include diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 19bef0b788108..a34127d0c3dd4 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -363,12 +363,14 @@ bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSi } LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); + sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); if (!sessionInfo->quorum->HasVerificationVector()) { // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", __func__, - sessionInfo->quorumHash.ToString(), pfrom.GetId()); + LogPrint(BCLog::LLMQ_SIGS, + "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. " + "node=%d\n", + __func__, sessionInfo->quorumHash.ToString(), pfrom.GetId()); return true; } @@ -398,7 +400,7 @@ bool CSigSharesManager::ProcessMessageGetSigShares(const CNode& pfrom, const CSi } LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); + sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); LOCK(cs); auto& nodeState = nodeStates[pfrom.GetId()]; @@ -448,8 +450,9 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const } } - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", __func__, - sessionInfo->signHash.ToString(), batchedSigShares.sigShares.size(), sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", + __func__, sessionInfo->signHash.ToString(), batchedSigShares.sigShares.size(), + sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); if (sigSharesToProcess.empty()) { return true; diff --git a/src/masternode/node.cpp b/src/masternode/node.cpp index ff8b8b2c6e894..7675e19b82b8b 100644 --- a/src/masternode/node.cpp +++ b/src/masternode/node.cpp @@ -300,4 +300,3 @@ template bool CActiveMasternodeManager::Decrypt(const CBLSIESMultiRecipientObjec READ_LOCK(cs); return m_info.blsPubKeyOperator; } - From 4a55cd54dd54b56c6c24fe2a8bbb0ee1864d15bd Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Nov 2025 19:48:44 -0600 Subject: [PATCH 05/10] fix: improve logging format in CSigSharesManager Updated the logging statement in CSigSharesManager to enhance readability by continuing the format string on a new line. This change maintains the existing logging functionality while improving code clarity. --- src/llmq/signing_shares.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index a34127d0c3dd4..4aa1e8bf3a7a0 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -367,9 +367,8 @@ bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSi if (!sessionInfo->quorum->HasVerificationVector()) { // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrint(BCLog::LLMQ_SIGS, - "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. " - "node=%d\n", + LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", __func__, sessionInfo->quorumHash.ToString(), pfrom.GetId()); return true; } @@ -450,7 +449,8 @@ bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const } } - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", + LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", __func__, sessionInfo->signHash.ToString(), batchedSigShares.sigShares.size(), sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); From e05d96ac347d740868b70936653bc70889e3e1ed Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 10 Nov 2025 19:57:22 -0600 Subject: [PATCH 06/10] fix: add src/llmq/dkgtypes.h --- src/llmq/dkgtypes.h | 193 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 src/llmq/dkgtypes.h diff --git a/src/llmq/dkgtypes.h b/src/llmq/dkgtypes.h new file mode 100644 index 0000000000000..818de4c8203e5 --- /dev/null +++ b/src/llmq/dkgtypes.h @@ -0,0 +1,193 @@ +// Copyright (c) 2018-2025 The Dash Core developers +// Distributed under the MIT/X11 software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_LLMQ_DKGTYPES_H +#define BITCOIN_LLMQ_DKGTYPES_H + +#include +#include +#include +#include +#include +#include + +#include + +#include +#include + +namespace llmq +{ + +class CDKGContribution +{ +public: + Consensus::LLMQType llmqType; + uint256 quorumHash; + uint256 proTxHash; + BLSVerificationVectorPtr vvec; + std::shared_ptr> contributions; + CBLSSignature sig; + +public: + template + inline void SerializeWithoutSig(Stream& s) const + { + s << ToUnderlying(llmqType); + s << quorumHash; + s << proTxHash; + s << *vvec; + s << *contributions; + } + template + inline void Serialize(Stream& s) const + { + SerializeWithoutSig(s); + s << sig; + } + template + inline void Unserialize(Stream& s) + { + std::vector tmp1; + CBLSIESMultiRecipientObjects tmp2; + + s >> llmqType; + s >> quorumHash; + s >> proTxHash; + s >> tmp1; + s >> tmp2; + s >> sig; + + vvec = std::make_shared>(std::move(tmp1)); + contributions = std::make_shared>(std::move(tmp2)); + } + + [[nodiscard]] uint256 GetSignHash() const + { + CHashWriter hw(SER_GETHASH, 0); + SerializeWithoutSig(hw); + hw << CBLSSignature(); + return hw.GetHash(); + } +}; + +class CDKGComplaint +{ +public: + Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE}; + uint256 quorumHash; + uint256 proTxHash; + std::vector badMembers; + std::vector complainForMembers; + CBLSSignature sig; + +public: + CDKGComplaint() = default; + explicit CDKGComplaint(const Consensus::LLMQParams& params) : + badMembers((size_t)params.size), complainForMembers((size_t)params.size) {}; + + SERIALIZE_METHODS(CDKGComplaint, obj) + { + READWRITE( + obj.llmqType, + obj.quorumHash, + obj.proTxHash, + DYNBITSET(obj.badMembers), + DYNBITSET(obj.complainForMembers), + obj.sig + ); + } + + [[nodiscard]] uint256 GetSignHash() const + { + CDKGComplaint tmp(*this); + tmp.sig = CBLSSignature(); + return ::SerializeHash(tmp); + } +}; + +class CDKGJustification +{ +public: + Consensus::LLMQType llmqType; + uint256 quorumHash; + uint256 proTxHash; + struct Contribution { + uint32_t index; + CBLSSecretKey key; + SERIALIZE_METHODS(Contribution, obj) + { + READWRITE(obj.index, obj.key); + } + }; + std::vector contributions; + CBLSSignature sig; + +public: + SERIALIZE_METHODS(CDKGJustification, obj) + { + READWRITE(obj.llmqType, obj.quorumHash, obj.proTxHash, obj.contributions, obj.sig); + } + + [[nodiscard]] uint256 GetSignHash() const + { + CDKGJustification tmp(*this); + tmp.sig = CBLSSignature(); + return ::SerializeHash(tmp); + } +}; + +// each member commits to a single set of valid members with this message +// then each node aggregate all received premature commitments +// into a single CFinalCommitment, which is only valid if +// enough (>=minSize) premature commitments were aggregated +class CDKGPrematureCommitment +{ +public: + Consensus::LLMQType llmqType{Consensus::LLMQType::LLMQ_NONE}; + uint256 quorumHash; + uint256 proTxHash; + std::vector validMembers; + + CBLSPublicKey quorumPublicKey; + uint256 quorumVvecHash; + + CBLSSignature quorumSig; // threshold sig share of quorumHash+validMembers+pubKeyHash+vvecHash + CBLSSignature sig; // single member sig of quorumHash+validMembers+pubKeyHash+vvecHash + +public: + CDKGPrematureCommitment() = default; + explicit CDKGPrematureCommitment(const Consensus::LLMQParams& params) : + validMembers((size_t)params.size) {}; + + [[nodiscard]] int CountValidMembers() const + { + return int(std::count(validMembers.begin(), validMembers.end(), true)); + } + +public: + SERIALIZE_METHODS(CDKGPrematureCommitment, obj) + { + READWRITE( + obj.llmqType, + obj.quorumHash, + obj.proTxHash, + DYNBITSET(obj.validMembers), + obj.quorumPublicKey, + obj.quorumVvecHash, + obj.quorumSig, + obj.sig + ); + } + + [[nodiscard]] uint256 GetSignHash() const + { + return BuildCommitmentHash(llmqType, quorumHash, validMembers, quorumPublicKey, quorumVvecHash); + } +}; + +} // namespace llmq + +#endif // BITCOIN_LLMQ_DKGTYPES_H + From d007f01f34ded7b3390894ebd5f4454a2dda909e Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Mon, 10 Nov 2025 20:03:30 -0600 Subject: [PATCH 07/10] fix: include Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/llmq/dkgtypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/llmq/dkgtypes.h b/src/llmq/dkgtypes.h index 818de4c8203e5..0300101dceb0b 100644 --- a/src/llmq/dkgtypes.h +++ b/src/llmq/dkgtypes.h @@ -15,6 +15,7 @@ #include #include +#include #include namespace llmq @@ -29,7 +30,6 @@ class CDKGContribution BLSVerificationVectorPtr vvec; std::shared_ptr> contributions; CBLSSignature sig; - public: template inline void SerializeWithoutSig(Stream& s) const From 96e3eb1110941486d8073ab3dccb91f00df599d7 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 19 Nov 2025 08:56:09 -0600 Subject: [PATCH 08/10] fix: minor fixes --- src/llmq/dkgtypes.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/llmq/dkgtypes.h b/src/llmq/dkgtypes.h index 0300101dceb0b..fae6c98554067 100644 --- a/src/llmq/dkgtypes.h +++ b/src/llmq/dkgtypes.h @@ -9,11 +9,10 @@ #include #include #include +#include #include #include -#include - #include #include #include @@ -190,4 +189,3 @@ class CDKGPrematureCommitment } // namespace llmq #endif // BITCOIN_LLMQ_DKGTYPES_H - From 5b7c51e3acd8face4d21557558d8aaf96a8549f0 Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 19 Nov 2025 09:05:11 -0600 Subject: [PATCH 09/10] chore: avoid extra nesting --- src/llmq/signing_shares.cpp | 166 +++++++++++++++++++----------------- 1 file changed, 86 insertions(+), 80 deletions(-) diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 4aa1e8bf3a7a0..13a7c1044eda0 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -352,119 +352,125 @@ bool CSigSharesManager::VerifySigSharesInv(Consensus::LLMQType llmqType, const C bool CSigSharesManager::ProcessMessageSigSharesInv(const CNode& pfrom, const CSigSharesInv& inv) { - if (auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId)) { - if (!VerifySigSharesInv(sessionInfo->llmqType, inv)) { - return false; - } + auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId); + if (!sessionInfo) { + return true; + } - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForSession(sessionInfo->signHash.Get())) { - return true; - } + if (!VerifySigSharesInv(sessionInfo->llmqType, inv)) { + return false; + } - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); + // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig + if (sigman.HasRecoveredSigForSession(sessionInfo->signHash.Get())) { + return true; + } - if (!sessionInfo->quorum->HasVerificationVector()) { - // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ - "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", - __func__, sessionInfo->quorumHash.ToString(), pfrom.GetId()); - return true; - } + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, + sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto* session = nodeState.GetSessionByRecvId(inv.sessionId); - if (session == nullptr) { - return true; - } - session->announced.Merge(inv); - session->knows.Merge(inv); + if (!sessionInfo->quorum->HasVerificationVector()) { + // TODO we should allow to ask other nodes for the quorum vvec if we missed it in the DKG + LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + "CSigSharesManager::%s -- we don't have the quorum vvec for %s, not requesting sig shares. node=%d\n", + __func__, sessionInfo->quorumHash.ToString(), pfrom.GetId()); return true; } + + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; + auto* session = nodeState.GetSessionByRecvId(inv.sessionId); + if (session == nullptr) { + return true; + } + session->announced.Merge(inv); + session->knows.Merge(inv); return true; } bool CSigSharesManager::ProcessMessageGetSigShares(const CNode& pfrom, const CSigSharesInv& inv) { - if (auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId)) { - if (!VerifySigSharesInv(sessionInfo->llmqType, inv)) { - return false; - } + auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), inv.sessionId); + if (!sessionInfo) { + return true; + } - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForSession(sessionInfo->signHash.Get())) { - return true; - } + if (!VerifySigSharesInv(sessionInfo->llmqType, inv)) { + return false; + } - LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, - sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); + // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig + if (sigman.HasRecoveredSigForSession(sessionInfo->signHash.Get())) { + return true; + } - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - auto* session = nodeState.GetSessionByRecvId(inv.sessionId); - if (session == nullptr) { - return true; - } - session->requested.Merge(inv); - session->knows.Merge(inv); + LogPrint(BCLog::LLMQ_SIGS, "CSigSharesManager::%s -- signHash=%s, inv={%s}, node=%d\n", __func__, + sessionInfo->signHash.ToString(), inv.ToString(), pfrom.GetId()); + + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; + auto* session = nodeState.GetSessionByRecvId(inv.sessionId); + if (session == nullptr) { return true; } + session->requested.Merge(inv); + session->knows.Merge(inv); return true; } bool CSigSharesManager::ProcessMessageBatchedSigShares(const CNode& pfrom, const CBatchedSigShares& batchedSigShares) { - if (auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), batchedSigShares.sessionId)) { - if (bool ban{false}; !PreVerifyBatchedSigShares(m_mn_activeman, qman, *sessionInfo, batchedSigShares, ban)) { - return !ban; - } + auto sessionInfo = GetSessionInfoByRecvId(pfrom.GetId(), batchedSigShares.sessionId); + if (!sessionInfo) { + return true; + } - std::vector sigSharesToProcess; - sigSharesToProcess.reserve(batchedSigShares.sigShares.size()); + if (bool ban{false}; !PreVerifyBatchedSigShares(m_mn_activeman, qman, *sessionInfo, batchedSigShares, ban)) { + return !ban; + } - { - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; + std::vector sigSharesToProcess; + sigSharesToProcess.reserve(batchedSigShares.sigShares.size()); - for (const auto& sigSharetmp : batchedSigShares.sigShares) { - CSigShare sigShare = RebuildSigShare(*sessionInfo, sigSharetmp); - nodeState.requestedSigShares.Erase(sigShare.GetKey()); + { + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; - // TODO track invalid sig shares received for PoSe? - // It's important to only skip seen *valid* sig shares here. If a node sends us a - // batch of mostly valid sig shares with a single invalid one and thus batched - // verification fails, we'd skip the valid ones in the future if received from other nodes - if (sigShares.Has(sigShare.GetKey())) { - continue; - } + for (const auto& sigSharetmp : batchedSigShares.sigShares) { + CSigShare sigShare = RebuildSigShare(*sessionInfo, sigSharetmp); + nodeState.requestedSigShares.Erase(sigShare.GetKey()); - // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig - if (sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId())) { - continue; - } - - sigSharesToProcess.emplace_back(sigShare); + // TODO track invalid sig shares received for PoSe? + // It's important to only skip seen *valid* sig shares here. If a node sends us a + // batch of mostly valid sig shares with a single invalid one and thus batched + // verification fails, we'd skip the valid ones in the future if received from other nodes + if (sigShares.Has(sigShare.GetKey())) { + continue; } - } - LogPrint(BCLog::LLMQ_SIGS, /* Continued */ - "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", - __func__, sessionInfo->signHash.ToString(), batchedSigShares.sigShares.size(), - sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); + // TODO for PoSe, we should consider propagating shares even if we already have a recovered sig + if (sigman.HasRecoveredSigForId(sigShare.getLlmqType(), sigShare.getId())) { + continue; + } - if (sigSharesToProcess.empty()) { - return true; + sigSharesToProcess.emplace_back(sigShare); } + } - LOCK(cs); - auto& nodeState = nodeStates[pfrom.GetId()]; - for (const auto& s : sigSharesToProcess) { - nodeState.pendingIncomingSigShares.Add(s.GetKey(), s); - } + LogPrint(BCLog::LLMQ_SIGS, /* Continued */ + "CSigSharesManager::%s -- signHash=%s, shares=%d, new=%d, inv={%s}, node=%d\n", + __func__, sessionInfo->signHash.ToString(), batchedSigShares.sigShares.size(), + sigSharesToProcess.size(), batchedSigShares.ToInvString(), pfrom.GetId()); + + if (sigSharesToProcess.empty()) { return true; } + + LOCK(cs); + auto& nodeState = nodeStates[pfrom.GetId()]; + for (const auto& s : sigSharesToProcess) { + nodeState.pendingIncomingSigShares.Add(s.GetKey(), s); + } return true; } From d9393a87d6be54db162b832998ab3d1f95e6c8ae Mon Sep 17 00:00:00 2001 From: pasta Date: Wed, 19 Nov 2025 09:45:20 -0600 Subject: [PATCH 10/10] refactor: drop unused imports --- src/llmq/dkgsession.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/llmq/dkgsession.h b/src/llmq/dkgsession.h index e4d8f9119ea4c..12cf23f2fb57b 100644 --- a/src/llmq/dkgsession.h +++ b/src/llmq/dkgsession.h @@ -10,13 +10,11 @@ #include #include -#include #include #include #include #include -#include #include #include