refactor: avoid return by reference and prefer return by value

src/Makefile.am

@@ -267,6 +267,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 \

src/chainlock/chainlock.cpp

@@ -95,17 +95,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<chainlock::ChainLockSig> 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

src/chainlock/chainlock.h

@@ -22,6 +22,7 @@
 #include <chrono>
 #include <map>
 #include <memory>
+#include <optional>
 #include <thread>
 #include <unordered_map>
 
@@ -90,8 +91,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
-        EXCLUSIVE_LOCKS_REQUIRED(!cs);
+    std::optional<chainlock::ChainLockSig> 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);

src/instantsend/instantsend.cpp

@@ -837,10 +837,10 @@ bool CInstantSendManager::AlreadyHave(const CInv& inv) const
            db.KnownInstantSendLock(inv.hash);
 }
 
-bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, instantsend::InstantSendLock& ret) const
+std::optional<instantsend::InstantSendLock> CInstantSendManager::GetInstantSendLockByHash(const uint256& hash) const
 {
     if (!IsInstantSendEnabled()) {
-        return false;
+        return std::nullopt;
     }
 
     auto islock = db.GetInstantSendLockByHash(hash);
@@ -854,12 +854,11 @@ bool CInstantSendManager::GetInstantSendLockByHash(const uint256& hash, instants
             if (itNoTx != pendingNoTxInstantSendLocks.end()) {
                 islock = itNoTx->second.second;
             } else {
-                return false;
+                return std::nullopt;
             }
         }
     }
-    ret = *islock;
-    return true;
+    return *islock;
 }
 
 instantsend::InstantSendLockPtr CInstantSendManager::GetInstantSendLockByTxid(const uint256& txid) const

src/instantsend/instantsend.h

@@ -18,6 +18,7 @@
 #include <unordered_lru_cache.h>
 
 #include <atomic>
+#include <optional>
 #include <unordered_map>
 #include <unordered_set>
 
@@ -159,7 +160,7 @@ class CInstantSendManager final : public instantsend::InstantSendSignerParent
     void BlockDisconnected(const std::shared_ptr<const CBlock>& 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<instantsend::InstantSendLock> GetInstantSendLockByHash(const uint256& hash) const
         EXCLUSIVE_LOCKS_REQUIRED(!cs_pendingLocks);
     instantsend::InstantSendLockPtr GetInstantSendLockByTxid(const uint256& txid) const;
 

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++;

src/llmq/blockprocessor.cpp

@@ -720,15 +720,14 @@ std::optional<CInv> 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<llmq::CFinalCommitment> 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

src/llmq/blockprocessor.h

@@ -73,7 +73,7 @@ class CQuorumBlockProcessor
     //! it returns hash of commitment if it should be relay, otherwise nullopt
     std::optional<CInv> 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<CFinalCommitment> GetMineableCommitmentByHash(const uint256& commitmentHash) const
         EXCLUSIVE_LOCKS_REQUIRED(!minableCommitmentsCs);
     std::optional<std::vector<CFinalCommitment>> GetMineableCommitments(const Consensus::LLMQParams& llmqParams,
                                                                         int nHeight) const

src/llmq/dkgsession.h

@@ -6,6 +6,7 @@
 #define BITCOIN_LLMQ_DKGSESSION_H
 
 #include <llmq/commitment.h>
+#include <llmq/dkgtypes.h>
 
 #include <batchedlogger.h>
 #include <bls/bls.h>
@@ -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<CBLSIESMultiRecipientObjects<CBLSSecretKey>> contributions;
-    CBLSSignature sig;
-
-public:
-    template<typename Stream>
-    inline void SerializeWithoutSig(Stream& s) const
-    {
-        s << ToUnderlying(llmqType);
-        s << quorumHash;
-        s << proTxHash;
-        s << *vvec;
-        s << *contributions;
-    }
-    template<typename Stream>
-    inline void Serialize(Stream& s) const
-    {
-        SerializeWithoutSig(s);
-        s << sig;
-    }
-    template<typename Stream>
-    inline void Unserialize(Stream& s)
-    {
-        std::vector<CBLSPublicKey> tmp1;
-        CBLSIESMultiRecipientObjects<CBLSSecretKey> tmp2;
-
-        s >> llmqType;
-        s >> quorumHash;
-        s >> proTxHash;
-        s >> tmp1;
-        s >> tmp2;
-        s >> sig;
-
-        vvec = std::make_shared<std::vector<CBLSPublicKey>>(std::move(tmp1));
-        contributions = std::make_shared<CBLSIESMultiRecipientObjects<CBLSSecretKey>>(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<bool> badMembers;
-    std::vector<bool> 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<Contribution> 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<bool> 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:

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<CDKGContribution> 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<CDKGComplaint> 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<CDKGJustification> 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<CDKGPrematureCommitment> 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