Use CBLSLazySignature in CBatchedSigShares

codablock · panleone · commit 6c63a776724a · 2024-04-07T13:02:22.000+02:00
This removes the burden on the message handler thread when many sig batches
arrive. The expensive part of deserialization is now performed in the sig
shares worker thread.

This also removes the need for the specialized deserialization of the sig
shares which tried to avoid the malleability check, as CBLSLazySignature does
not perform malleability checks at all.
diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp
@@ -374,11 +374,6 @@ bool CSigSharesManager::PreVerifyBatchedSigShares(NodeId nodeId, const CBatchedS
 
     for (size_t i = 0; i < batchedSigShares.sigShares.size(); i++) {
         auto quorumMember = batchedSigShares.sigShares[i].first;
-        auto& sigShare = batchedSigShares.sigShares[i].second;
-        if (!sigShare.IsValid()) {
-            retBan = true;
-            return false;
-        }
         if (!dupMembers.emplace(quorumMember).second) {
             retBan = true;
             return false;
@@ -482,6 +477,14 @@ void CSigSharesManager::ProcessPendingSigShares(CConnman& connman)
                 continue;
             }
 
+            // we didn't check this earlier because we use a lazy BLS signature and tried to avoid doing the expensive
+            // deserialization in the message thread
+            if (!sigShare.sigShare.Get().IsValid()) {
+                BanNode(nodeId);
+                // don't process any additional shares from this node
+                break;
+            }
+
             auto quorum = quorums.at(std::make_pair((Consensus::LLMQType)sigShare.llmqType, sigShare.quorumHash));
             auto pubKeyShare = quorum->GetPubKeyShare(sigShare.quorumMember);
 
@@ -492,7 +495,7 @@ void CSigSharesManager::ProcessPendingSigShares(CConnman& connman)
                 assert(false);
             }
 
-            batchVerifier.PushMessage(nodeId, sigShare.GetKey(), sigShare.GetSignHash(), sigShare.sigShare, pubKeyShare);
+            batchVerifier.PushMessage(nodeId, sigShare.GetKey(), sigShare.GetSignHash(), sigShare.sigShare.Get(), pubKeyShare);
             verifyCount++;
         }
     }
@@ -616,7 +619,7 @@ void CSigSharesManager::TryRecoverSig(const CQuorumCPtr& quorum, const uint256&
         idsForRecovery.reserve((size_t)quorum->params.threshold);
         for (auto it = itPair.first; it != itPair.second && sigSharesForRecovery.size() < quorum->params.threshold; ++it) {
             auto& sigShare = it->second;
-            sigSharesForRecovery.emplace_back(sigShare.sigShare);
+            sigSharesForRecovery.emplace_back(sigShare.sigShare.Get());
             idsForRecovery.emplace_back(CBLSId(quorum->members[sigShare.quorumMember]->proTxHash));
         }
 
@@ -1160,8 +1163,8 @@ void CSigSharesManager::Sign(const CQuorumCPtr& quorum, const uint256& id, const
     sigShare.quorumMember = (uint16_t)memberIdx;
     uint256 signHash = llmq::utils::BuildSignHash(sigShare);
 
-    sigShare.sigShare = skShare.Sign(signHash);
-    if (!sigShare.sigShare.IsValid()) {
+    sigShare.sigShare.Set(skShare.Sign(signHash));
+    if (!sigShare.sigShare.Get().IsValid()) {
         LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. id=%s, msgHash=%s, time=%s\n", __func__,
             sigShare.id.ToString(), sigShare.msgHash.ToString(), t.count());
         return;
diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h
@@ -38,7 +38,7 @@ class CSigShare
     uint16_t quorumMember;
     uint256 id;
     uint256 msgHash;
-    CBLSSignature sigShare;
+    CBLSLazySignature sigShare;
 
     SigShareKey key;
 
@@ -94,50 +94,17 @@ class CBatchedSigShares
     uint256 quorumHash;
     uint256 id;
     uint256 msgHash;
-    std::vector<std::pair<uint16_t, CBLSSignature>> sigShares;
+    std::vector<std::pair<uint16_t, CBLSLazySignature>> sigShares;
 
 public:
-    template <typename Stream, typename Operation>
-    inline void SerializationOpBase(Stream& s, Operation ser_action)
+    SERIALIZE_METHODS(CBatchedSigShares, obj)
     {
-        READWRITE(llmqType);
-        READWRITE(quorumHash);
-        READWRITE(id);
-        READWRITE(msgHash);
-        ::Serialize(s, llmqType);
-    }
-
-    template <typename Stream>
-    inline void Serialize(Stream& s) const
-    {
-        s << llmqType;
-        s << quorumHash;
-        s << id;
-        s << msgHash;
-        // this->SerializationOpBase(s, CSerActionSerialize());
-        s << sigShares;
+        READWRITE(obj.llmqType);
+        READWRITE(obj.quorumHash);
+        READWRITE(obj.id);
+        READWRITE(obj.msgHash);
+        READWRITE(obj.sigShares);
     }
-    template <typename Stream>
-    inline void Unserialize(Stream& s)
-    {
-        // this->SerializationOpBase(s, CSerActionUnserialize());
-        s >> llmqType;
-        s >> quorumHash;
-        s >> id;
-        s >> msgHash;
-        // we do custom deserialization here with the malleability check skipped for signatures
-        // we can do this here because we never use the hash of a sig share for identification and are only interested
-        // in validity
-        uint64_t nSize = ReadCompactSize(s);
-        if (nSize > 400) { // we don't support larger quorums, so this is the limit
-            throw std::ios_base::failure(strprintf("too many elements (%d) in CBatchedSigShares", nSize));
-        }
-        sigShares.resize(nSize);
-        for (size_t i = 0; i < nSize; i++) {
-            s >> sigShares[i].first;
-            sigShares[i].second.Unserialize(s, false);
-        }
-    };
 
     CSigShare RebuildSigShare(size_t idx) const
     {
