Optimize LLMQs initialization and (de)serialization of BLS signatures

src/bls/bls.cpp

@@ -423,6 +423,34 @@ bool CBLSSignature::Recover(const std::vector<CBLSSignature>& sigs, const std::v
     return true;
 }
 
+CBLSLazySignature::CBLSLazySignature(CBLSSignature& _sig) :
+    bufValid(false),
+    sigInitialized(true),
+    sig(_sig)
+{
+
+}
+
+void CBLSLazySignature::SetSig(const CBLSSignature& _sig)
+{
+    bufValid = false;
+    sigInitialized = true;
+    sig = _sig;
+}
+
+const CBLSSignature& CBLSLazySignature::GetSig() const
+{
+    if (!bufValid && !sigInitialized) {
+        static CBLSSignature invalidSig;
+        return invalidSig;
+    }
+    if (!sigInitialized) {
+        sig.SetBuf(buf, sizeof(buf));
+        sigInitialized = true;
+    }
+    return sig;
+}
+
 #ifndef BUILD_BITCOIN_INTERNAL
 
 static std::once_flag init_flag;

src/bls/bls.h

@@ -51,7 +51,18 @@ class CBLSWrapper
 
     CBLSWrapper()
     {
-        UpdateHash();
+        struct NullHash {
+            uint256 hash;
+            NullHash() {
+                char buf[_SerSize];
+                memset(buf, 0, _SerSize);
+                CHashWriter ss(SER_GETHASH, 0);
+                ss.write(buf, _SerSize);
+                hash = ss.GetHash();
+            }
+        };
+        static NullHash nullHash;
+        cachedHash = nullHash.hash;
     }
 
     CBLSWrapper(const CBLSWrapper& ref) = default;
@@ -297,6 +308,45 @@ class CBLSSignature : public CBLSWrapper<bls::InsecureSignature, BLS_CURVE_SIG_S
     bool InternalGetBuf(void* buf) const;
 };
 
+class CBLSLazySignature
+{
+private:
+    mutable char buf[BLS_CURVE_SIG_SIZE];
+    mutable bool bufValid{false};
+
+    mutable CBLSSignature sig;
+    mutable bool sigInitialized{false};
+
+public:
+    template<typename Stream>
+    inline void Serialize(Stream& s) const
+    {
+        if (!sigInitialized && !bufValid) {
+            throw std::ios_base::failure("sig and buf not initialized");
+        }
+        if (!bufValid) {
+            sig.GetBuf(buf, sizeof(buf));
+            bufValid = true;
+        }
+        s.write(buf, sizeof(buf));
+    }
+
+    template<typename Stream>
+    inline void Unserialize(Stream& s)
+    {
+        s.read(buf, sizeof(buf));
+        bufValid = true;
+        sigInitialized = false;
+    }
+
+public:
+    CBLSLazySignature() = default;
+    CBLSLazySignature(CBLSSignature& _sig);
+
+    void SetSig(const CBLSSignature& _sig);
+    const CBLSSignature& GetSig() const;
+};
+
 typedef std::vector<CBLSId> BLSIdVector;
 typedef std::vector<CBLSPublicKey> BLSVerificationVector;
 typedef std::vector<CBLSPublicKey> BLSPublicKeyVector;

src/llmq/quorums_signing_shares.cpp

@@ -373,11 +373,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;
@@ -483,6 +478,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.GetSig().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);
 
@@ -493,7 +496,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.GetSig(), pubKeyShare);
             verifyCount++;
         }
     }
@@ -617,7 +620,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.GetSig());
             idsForRecovery.emplace_back(CBLSId::FromHash(quorum->members[sigShare.quorumMember]->proTxHash));
         }
 
@@ -1168,8 +1171,8 @@ void CSigSharesManager::Sign(const CQuorumCPtr& quorum, const uint256& id, const
     sigShare.quorumMember = (uint16_t)memberIdx;
     uint256 signHash = CLLMQUtils::BuildSignHash(sigShare);
 
-    sigShare.sigShare = skShare.Sign(signHash);
-    if (!sigShare.sigShare.IsValid()) {
+    sigShare.sigShare.SetSig(skShare.Sign(signHash));
+    if (!sigShare.sigShare.GetSig().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;

src/llmq/quorums_signing_shares.h

@@ -37,7 +37,7 @@ class CSigShare
     uint16_t quorumMember;
     uint256 id;
     uint256 msgHash;
-    CBLSSignature sigShare;
+    CBLSLazySignature sigShare;
 
     SigShareKey key;
 
@@ -97,43 +97,21 @@ 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:
+    ADD_SERIALIZE_METHODS;
+
     template<typename Stream, typename Operation>
-    inline void SerializationOpBase(Stream& s, Operation ser_action)
+    inline void SerializationOp(Stream& s, Operation ser_action)
     {
         READWRITE(llmqType);
         READWRITE(quorumHash);
         READWRITE(id);
         READWRITE(msgHash);
+        READWRITE(sigShares);
     }
 
-    template<typename Stream>
-    inline void Serialize(Stream& s) const
-    {
-        NCONST_PTR(this)->SerializationOpBase(s, CSerActionSerialize());
-        s << sigShares;
-    }
-    template<typename Stream>
-    inline void Unserialize(Stream& s)
-    {
-        NCONST_PTR(this)->SerializationOpBase(s, CSerActionUnserialize());
-
-        // 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
     {
         assert(idx < sigShares.size());