From 6e8f50aa5515cf41968cde6d50e81a882fd076b1 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Fri, 15 Feb 2019 14:09:47 +0100 Subject: [PATCH 1/3] Faster default-initialization of BLS primitives by re-using the null-hash --- src/bls/bls.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/bls/bls.h b/src/bls/bls.h index dc67b9175cb00..3c953050c7e90 100644 --- a/src/bls/bls.h +++ b/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; From 02b68885a0fbecb1d5a8d2a52fe566b1c6fa127b Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Fri, 15 Feb 2019 14:11:47 +0100 Subject: [PATCH 2/3] Implement CBLSLazySignature for lazy serialization/deserialization In some cases it takes too much time to perform full deserialization of BLS signatures in the message handler thread. Better to just read the buffer and do the actual deserialization when the signature is needed for the first time (which is can be in another thread). --- src/bls/bls.cpp | 28 ++++++++++++++++++++++++++++ src/bls/bls.h | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index bb44f50296acb..534fd8ad0bd9c 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -423,6 +423,34 @@ bool CBLSSignature::Recover(const std::vector& 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; diff --git a/src/bls/bls.h b/src/bls/bls.h index 3c953050c7e90..01f730244daa4 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -308,6 +308,45 @@ class CBLSSignature : public CBLSWrapper + 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 + 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 BLSIdVector; typedef std::vector BLSVerificationVector; typedef std::vector BLSPublicKeyVector; From 500b9c89a7a5e526b9ee9754519eb6966136dfd2 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Fri, 15 Feb 2019 15:04:48 +0100 Subject: [PATCH 3/3] Use CBLSLazySignature in CBatchedSigShares 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. --- src/llmq/quorums_signing_shares.cpp | 21 ++++++++++-------- src/llmq/quorums_signing_shares.h | 34 +++++------------------------ 2 files changed, 18 insertions(+), 37 deletions(-) diff --git a/src/llmq/quorums_signing_shares.cpp b/src/llmq/quorums_signing_shares.cpp index 41b21bb3d3cd3..f962547720141 100644 --- a/src/llmq/quorums_signing_shares.cpp +++ b/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; diff --git a/src/llmq/quorums_signing_shares.h b/src/llmq/quorums_signing_shares.h index d59756de9d28e..6ae42f6785ed6 100644 --- a/src/llmq/quorums_signing_shares.h +++ b/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> sigShares; + std::vector> sigShares; public: + ADD_SERIALIZE_METHODS; + template - 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 - inline void Serialize(Stream& s) const - { - NCONST_PTR(this)->SerializationOpBase(s, CSerActionSerialize()); - s << sigShares; - } - template - 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());