From 34233a15c19c1e82fa853202209b8304524dbe03 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 6 Nov 2026 01:18:52 -0600 Subject: [PATCH 1/3] refactor(bls): resolve the implicit BLS scheme per thread BLS operations that are not given a scheme read the process-wide bls::bls_legacy_scheme. Add bls::EffectiveLegacyScheme(), which returns a thread-local override when one is set (bls::ScopedLegacySchemeOverride) and the process-wide value otherwise, and route every implicit read through it: the CBLSWrapper and CBLSLazyWrapper serialization defaults, the implicit aggregate and verify overloads, sig share signing, recovered sigs, DKG contributions, the coinbase ChainLock decode cache, the miner and `verifyrecsig`. Work that runs on another thread captures the scheme where it is created: utils::BlsCheck (run by the commitment CCheckQueue), CBLSBatchVerifier, and CBLSWorker's AsyncSign and AsyncVerifySig jobs, using new explicit-scheme VerifyInsecureAggregated/VerifySecureAggregated overloads. AsyncVerifySig no longer batches jobs of different schemes. Nothing sets an override yet, so behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 --- src/bls/bls.cpp | 60 +++++++++++++--- src/bls/bls.h | 52 ++++++++++++-- src/bls/bls_batchverifier.h | 11 +-- src/bls/bls_worker.cpp | 19 ++--- src/bls/bls_worker.h | 8 ++- src/chainlock/clsig.cpp | 6 +- src/llmq/dkgmessages.h | 2 +- src/llmq/signing.h | 2 +- src/llmq/signing_shares.cpp | 4 +- src/llmq/utils.cpp | 5 +- src/llmq/utils.h | 2 + src/node/miner.cpp | 2 +- src/rpc/quorums.cpp | 2 +- src/test/bls_tests.cpp | 119 +++++++++++++++++++++++++++++-- src/test/llmq_utils_tests.cpp | 48 +++++++++++++ src/test/quorum_proofs_tests.cpp | 61 ++++++++++++++++ 16 files changed, 356 insertions(+), 47 deletions(-) diff --git a/src/bls/bls.cpp b/src/bls/bls.cpp index 96a188e2ae93..1a8bab710d83 100644 --- a/src/bls/bls.cpp +++ b/src/bls/bls.cpp @@ -15,9 +15,40 @@ #include namespace bls { - std::atomic bls_legacy_scheme = std::atomic(true); +std::atomic bls_legacy_scheme = std::atomic(true); + +namespace { +//! Trivially destructible, like util/threadnames.cpp's thread-local name. +thread_local std::optional g_thread_legacy_scheme; +} // namespace + +bool EffectiveLegacyScheme() { return g_thread_legacy_scheme ? *g_thread_legacy_scheme : bls_legacy_scheme.load(); } + +void SetEffectiveLegacyScheme(bool legacy) +{ + if (g_thread_legacy_scheme) { + g_thread_legacy_scheme = legacy; + } else { + bls_legacy_scheme.store(legacy); + } } +ScopedLegacySchemeOverride::ScopedLegacySchemeOverride(bool legacy) noexcept : + m_saved{g_thread_legacy_scheme} +{ + g_thread_legacy_scheme = legacy; +} + +ScopedLegacySchemeOverride::~ScopedLegacySchemeOverride() noexcept +{ + if (!m_committed || !m_saved) { + g_thread_legacy_scheme = m_saved; + } +} + +bool ScopedLegacySchemeOverride::Active() noexcept { return g_thread_legacy_scheme.has_value(); } +} // namespace bls + static const std::unique_ptr pSchemeLegacy{std::make_unique()}; static const std::unique_ptr pScheme(std::make_unique()); @@ -145,7 +176,7 @@ void CBLSPublicKey::AggregateInsecure(const CBLSPublicKey& o) { assert(IsValid() && o.IsValid()); try { - impl = Scheme(bls::bls_legacy_scheme.load())->Aggregate({impl, o.impl}); + impl = Scheme(bls::EffectiveLegacyScheme())->Aggregate({impl, o.impl}); } catch (...) { fValid = false; } @@ -166,7 +197,7 @@ CBLSPublicKey CBLSPublicKey::AggregateInsecure(Span pks) CBLSPublicKey ret; try { - ret.impl = Scheme(bls::bls_legacy_scheme.load())->Aggregate(vecPublicKeys); + ret.impl = Scheme(bls::EffectiveLegacyScheme())->Aggregate(vecPublicKeys); ret.fValid = true; } catch (...) { ret.fValid = false; @@ -230,7 +261,7 @@ void CBLSSignature::AggregateInsecure(const CBLSSignature& o) { assert(IsValid() && o.IsValid()); try { - impl = Scheme(bls::bls_legacy_scheme.load())->Aggregate({impl, o.impl}); + impl = Scheme(bls::EffectiveLegacyScheme())->Aggregate({impl, o.impl}); } catch (...) { fValid = false; } @@ -251,7 +282,7 @@ CBLSSignature CBLSSignature::AggregateInsecure(Span sigs) CBLSSignature ret; try { - ret.impl = Scheme(bls::bls_legacy_scheme.load())->Aggregate(v); + ret.impl = Scheme(bls::EffectiveLegacyScheme())->Aggregate(v); ret.fValid = true; } catch (...) { ret.fValid = false; @@ -283,7 +314,7 @@ CBLSSignature CBLSSignature::AggregateSecure(Span sigs, CBLSSignature ret; try { - ret.impl = Scheme(bls::bls_legacy_scheme.load())->AggregateSecure(vecPublicKeys, vecSignatures, bls::Bytes(hash.begin(), hash.size())); + ret.impl = Scheme(bls::EffectiveLegacyScheme())->AggregateSecure(vecPublicKeys, vecSignatures, bls::Bytes(hash.begin(), hash.size())); ret.fValid = true; } catch (...) { ret.fValid = false; @@ -322,10 +353,16 @@ bool CBLSSignature::VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& h bool CBLSSignature::VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash) const { - return VerifyInsecure(pubKey, hash, bls::bls_legacy_scheme.load()); + return VerifyInsecure(pubKey, hash, bls::EffectiveLegacyScheme()); } bool CBLSSignature::VerifyInsecureAggregated(Span pubKeys, Span hashes) const +{ + return VerifyInsecureAggregated(pubKeys, hashes, bls::EffectiveLegacyScheme()); +} + +bool CBLSSignature::VerifyInsecureAggregated(Span pubKeys, Span hashes, + const bool specificLegacyScheme) const { if (!IsValid()) { return false; @@ -346,13 +383,18 @@ bool CBLSSignature::VerifyInsecureAggregated(Span pubKeys, SpanAggregateVerify(pubKeyVec, hashes2, impl); + return Scheme(specificLegacyScheme)->AggregateVerify(pubKeyVec, hashes2, impl); } catch (...) { return false; } } bool CBLSSignature::VerifySecureAggregated(Span pks, const uint256& hash) const +{ + return VerifySecureAggregated(pks, hash, bls::EffectiveLegacyScheme()); +} + +bool CBLSSignature::VerifySecureAggregated(Span pks, const uint256& hash, const bool specificLegacyScheme) const { if (pks.empty()) { return false; @@ -365,7 +407,7 @@ bool CBLSSignature::VerifySecureAggregated(Span pks, const uint25 } try { - return Scheme(bls::bls_legacy_scheme.load())->VerifySecure(vecPublicKeys, impl, bls::Bytes(hash.begin(), hash.size())); + return Scheme(specificLegacyScheme)->VerifySecure(vecPublicKeys, impl, bls::Bytes(hash.begin(), hash.size())); } catch (...) { return false; } diff --git a/src/bls/bls.h b/src/bls/bls.h index 82abc2c6de04..023b927312cb 100644 --- a/src/bls/bls.h +++ b/src/bls/bls.h @@ -24,11 +24,47 @@ #include #include #include +#include #include namespace bls { - extern std::atomic bls_legacy_scheme; -} +//! Process-wide default scheme of BLS operations that are not given one. It +//! follows the active chainstate's tip only: other threads read it outside +//! cs_main, so work for any other chain context (a background chainstate, a dry +//! run) sets a ScopedLegacySchemeOverride instead. +extern std::atomic bls_legacy_scheme; + +//! The scheme of BLS operations on this thread that are not given one: the +//! innermost ScopedLegacySchemeOverride if one is set, else bls_legacy_scheme. +bool EffectiveLegacyScheme(); + +//! Switches EffectiveLegacyScheme(): this thread's override if one is set, +//! else bls_legacy_scheme. +void SetEffectiveLegacyScheme(bool legacy); + +//! Sets EffectiveLegacyScheme() on this thread only, for the object's lifetime. +//! Work handed to another thread does not see it and must capture the scheme. +class ScopedLegacySchemeOverride +{ +public: + explicit ScopedLegacySchemeOverride(bool legacy) noexcept; + ~ScopedLegacySchemeOverride() noexcept; + + ScopedLegacySchemeOverride(const ScopedLegacySchemeOverride&) = delete; + ScopedLegacySchemeOverride& operator=(const ScopedLegacySchemeOverride&) = delete; + + //! Leaves the scheme in force at scope exit to the enclosing override. The + //! outermost override always ends with its scope. + void Commit() noexcept { m_committed = true; } + + //! Whether an override is set on this thread. + static bool Active() noexcept; + +private: + const std::optional m_saved; + bool m_committed{false}; +}; +} // namespace bls // reversed BLS12-381 constexpr int BLS_CURVE_ID_SIZE{32}; @@ -177,7 +213,7 @@ class CBLSWrapper template inline void Serialize(Stream& s) const { - Serialize(s, bls::bls_legacy_scheme.load()); + Serialize(s, bls::EffectiveLegacyScheme()); } template @@ -205,7 +241,7 @@ class CBLSWrapper template inline void Unserialize(Stream& s) { - Unserialize(s, bls::bls_legacy_scheme.load()); + Unserialize(s, bls::EffectiveLegacyScheme()); } inline bool CheckMalleable(Span vecBytes, const bool specificLegacyScheme) const @@ -228,7 +264,7 @@ class CBLSWrapper inline std::string ToString() const { - return ToString(bls::bls_legacy_scheme.load()); + return ToString(bls::EffectiveLegacyScheme()); } }; @@ -365,8 +401,12 @@ class CBLSSignature : public CBLSWrapper pubKeys, Span hashes, + const bool specificLegacyScheme) const; [[nodiscard]] bool VerifyInsecureAggregated(Span pubKeys, Span hashes) const; + [[nodiscard]] bool VerifySecureAggregated(Span pks, const uint256& hash, + const bool specificLegacyScheme) const; [[nodiscard]] bool VerifySecureAggregated(Span pks, const uint256& hash) const; bool Recover(Span sigs, Span ids); @@ -410,7 +450,7 @@ class CBLSLazyWrapper public: CBLSLazyWrapper() : - bufLegacyScheme(bls::bls_legacy_scheme.load()) + bufLegacyScheme(bls::EffectiveLegacyScheme()) {} explicit CBLSLazyWrapper(const CBLSLazyWrapper& r) diff --git a/src/bls/bls_batchverifier.h b/src/bls/bls_batchverifier.h index 647d38e675e3..843fe85cfcbf 100644 --- a/src/bls/bls_batchverifier.h +++ b/src/bls/bls_batchverifier.h @@ -47,6 +47,8 @@ class CBLSBatchVerifier bool secureVerification; bool perMessageFallback; size_t subBatchSize; + //! Taken from the constructing thread, so a batch verifies under one scheme wherever Verify() runs + const bool legacyScheme; MessageMap messages; MessagesBySourceMap messagesBySource; @@ -61,7 +63,8 @@ class CBLSBatchVerifier CBLSBatchVerifier(bool _secureVerification, bool _perMessageFallback, size_t _subBatchSize = 0) : secureVerification(_secureVerification), perMessageFallback(_perMessageFallback), - subBatchSize(_subBatchSize) + subBatchSize(_subBatchSize), + legacyScheme(bls::EffectiveLegacyScheme()) { } @@ -135,7 +138,7 @@ class CBLSBatchVerifier } const auto& msg = msgIt->second; - if (!msg.sig.VerifyInsecure(msg.pubKey, msg.msgHash)) { + if (!msg.sig.VerifyInsecure(msg.pubKey, msg.msgHash, legacyScheme)) { badMessages.emplace(msg.msgId); } } @@ -201,7 +204,7 @@ class CBLSBatchVerifier } CBLSSignature aggSig = CBLSSignature::AggregateInsecure(sigsToAggregate); - return aggSig.VerifyInsecureAggregated(pubKeys, msgHashes); + return aggSig.VerifyInsecureAggregated(pubKeys, msgHashes, legacyScheme); } bool VerifyBatchSecure(std::map>& byMessageHash) @@ -251,7 +254,7 @@ class CBLSBatchVerifier assert(!msgHashes.empty()); CBLSSignature aggSig = CBLSSignature::AggregateInsecure(sigsToAggregate); - return aggSig.VerifyInsecureAggregated(pubKeys, msgHashes); + return aggSig.VerifyInsecureAggregated(pubKeys, msgHashes, legacyScheme); } }; diff --git a/src/bls/bls_worker.cpp b/src/bls/bls_worker.cpp index 7e383a644ba3..a387670d7141 100644 --- a/src/bls/bls_worker.cpp +++ b/src/bls/bls_worker.cpp @@ -825,8 +825,8 @@ std::set CBLSWorker::FindVerificationVectorsWithSharedCoefficients(Span< void CBLSWorker::AsyncSign(const CBLSSecretKey& secKey, const uint256& msgHash, const CBLSWorker::SignDoneCallback& doneCallback) { - workerPool.push([secKey, msgHash, doneCallback](int threadId) { - doneCallback(secKey.Sign(msgHash, bls::bls_legacy_scheme.load())); + workerPool.push([secKey, msgHash, doneCallback, legacy = bls::EffectiveLegacyScheme()](int threadId) { + doneCallback(secKey.Sign(msgHash, legacy)); }); } @@ -838,18 +838,19 @@ void CBLSWorker::AsyncVerifySig(const CBLSSignature& sig, const CBLSPublicKey& p return; } + const bool legacy{bls::EffectiveLegacyScheme()}; std::unique_lock l(sigVerifyMutex); bool foundDuplicate = std::ranges::any_of(sigVerifyQueue, [&msgHash](const auto& job) { return job.msgHash == msgHash; }); - if (foundDuplicate) { - // batched/aggregated verification does not allow duplicate hashes, so we push what we currently have and start - // with a fresh batch + if (foundDuplicate || (!sigVerifyQueue.empty() && sigVerifyQueue.front().legacyScheme != legacy)) { + // batched/aggregated verification does not allow duplicate hashes or mixed schemes, so we push what we + // currently have and start with a fresh batch PushSigVerifyBatch(); } - sigVerifyQueue.emplace_back(std::move(doneCallback), std::move(cancelCond), sig, pubKey, msgHash); + sigVerifyQueue.emplace_back(std::move(doneCallback), std::move(cancelCond), sig, pubKey, msgHash, legacy); if (sigVerifyBatchesInProgress == 0 || sigVerifyQueue.size() >= SIG_VERIFY_BATCH_SIZE) { PushSigVerifyBatch(); } @@ -876,7 +877,7 @@ void CBLSWorker::PushSigVerifyBatch() if (jobs.size() == 1) { const auto& job = jobs[0]; if (!job.cancelCond()) { - bool valid = job.sig.VerifyInsecure(job.pubKey, job.msgHash); + bool valid = job.sig.VerifyInsecure(job.pubKey, job.msgHash, job.legacyScheme); job.doneCallback(valid); } std::unique_lock l(sigVerifyMutex); @@ -910,7 +911,7 @@ void CBLSWorker::PushSigVerifyBatch() } if (!pubKeys.empty()) { - bool allValid = aggSig.VerifyInsecureAggregated(pubKeys, msgHashes); + bool allValid = aggSig.VerifyInsecureAggregated(pubKeys, msgHashes, jobs.front().legacyScheme); if (allValid) { for (size_t i = 0; i < pubKeys.size(); i++) { jobs[indexes[i]].doneCallback(true); @@ -920,7 +921,7 @@ void CBLSWorker::PushSigVerifyBatch() // TODO this could be improved if we would cache pairing results in some way as the previous aggregated verification already calculated all the pairings for the hashes for (size_t i = 0; i < pubKeys.size(); i++) { const auto& job = jobs[indexes[i]]; - bool valid = job.sig.VerifyInsecure(job.pubKey, job.msgHash); + bool valid = job.sig.VerifyInsecure(job.pubKey, job.msgHash, job.legacyScheme); job.doneCallback(valid); } } diff --git a/src/bls/bls_worker.h b/src/bls/bls_worker.h index 74c7ffe67d9b..fe3fc71410b6 100644 --- a/src/bls/bls_worker.h +++ b/src/bls/bls_worker.h @@ -35,12 +35,16 @@ class CBLSWorker CBLSSignature sig; CBLSPublicKey pubKey; uint256 msgHash; - SigVerifyJob(SigVerifyDoneCallback&& _doneCallback, CancelCond&& _cancelCond, const CBLSSignature& _sig, CBLSPublicKey _pubKey, const uint256& _msgHash) : + //! The submitting thread's scheme; the job runs on a worker thread + bool legacyScheme; + SigVerifyJob(SigVerifyDoneCallback&& _doneCallback, CancelCond&& _cancelCond, const CBLSSignature& _sig, + CBLSPublicKey _pubKey, const uint256& _msgHash, bool _legacyScheme) : doneCallback(_doneCallback), cancelCond(_cancelCond), sig(_sig), pubKey(std::move(_pubKey)), - msgHash(_msgHash) + msgHash(_msgHash), + legacyScheme(_legacyScheme) { } }; diff --git a/src/chainlock/clsig.cpp b/src/chainlock/clsig.cpp index 9313824eb007..8b1dc44491a7 100644 --- a/src/chainlock/clsig.cpp +++ b/src/chainlock/clsig.cpp @@ -86,9 +86,9 @@ class DecodedSignatureCache CBLSSignature Decode(const std::array& bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { // Decode exactly as the coinbase payload was always decoded: under the - // process-wide scheme flag, including its opposite-scheme rejection. The - // flag is part of the key so a transient value cannot leak into later calls. - const bool legacy = bls::bls_legacy_scheme.load(); + // effective scheme, including its opposite-scheme rejection. The scheme is + // part of the key so another context's value cannot leak into later calls. + const bool legacy = bls::EffectiveLegacyScheme(); const uint256 key = (CHashWriter(SER_GETHASH, 0) << Span{bytes} << legacy).GetHash(); CBLSSignature sig; { diff --git a/src/llmq/dkgmessages.h b/src/llmq/dkgmessages.h index 7080779abf2f..6ddba6a752a2 100644 --- a/src/llmq/dkgmessages.h +++ b/src/llmq/dkgmessages.h @@ -55,7 +55,7 @@ class CDKGContribution s >> quorumHash; s >> proTxHash; // Decode the whole vvec under one scheme, so that vvecLegacyScheme describes every point in it - vvecLegacyScheme = bls::bls_legacy_scheme.load(); + vvecLegacyScheme = bls::EffectiveLegacyScheme(); for (uint64_t i = 0, size = ReadCompactSize(s); i < size; i++) { s >> CBLSPublicKeyVersionWrapper(tmp1.emplace_back(), vvecLegacyScheme); } diff --git a/src/llmq/signing.h b/src/llmq/signing.h index b2ee2d22ba00..b11cba7379c8 100644 --- a/src/llmq/signing.h +++ b/src/llmq/signing.h @@ -82,7 +82,7 @@ class CRecoveredSig : virtual public CSigBase CRecoveredSig(Consensus::LLMQType _llmqType, const uint256& _quorumHash, const uint256& _id, const uint256& _msgHash, const CBLSLazySignature& _sig) : CSigBase(_llmqType, _quorumHash, _id, _msgHash), sig(_sig) {UpdateHash();}; CRecoveredSig(Consensus::LLMQType _llmqType, const uint256& _quorumHash, const uint256& _id, const uint256& _msgHash, const CBLSSignature& _sig) : - CSigBase(_llmqType, _quorumHash, _id, _msgHash) {const_cast(sig).Set(_sig, bls::bls_legacy_scheme.load()); UpdateHash();}; + CSigBase(_llmqType, _quorumHash, _id, _msgHash) {const_cast(sig).Set(_sig, bls::EffectiveLegacyScheme()); UpdateHash();}; private: // only in-memory diff --git a/src/llmq/signing_shares.cpp b/src/llmq/signing_shares.cpp index 5dd6ce91a3bc..566bccf8a361 100644 --- a/src/llmq/signing_shares.cpp +++ b/src/llmq/signing_shares.cpp @@ -1569,7 +1569,7 @@ std::optional CSigSharesManager::CreateSigShareForSingleMember(const // TODO: This one should be SIGN by QUORUM key, not by OPERATOR key // see TODO in CDKGSession::FinalizeSingleCommitment for details - auto bls_scheme = bls::bls_legacy_scheme.load(); + auto bls_scheme = bls::EffectiveLegacyScheme(); sigShare.sigShare.Set(m_mn_activeman.Sign(signHash, bls_scheme), bls_scheme); if (!sigShare.sigShare.Get().IsValid()) { @@ -1624,7 +1624,7 @@ std::optional CSigSharesManager::CreateSigShare(const CQuorum& quorum CSigShare sigShare(quorum.params.type, quorum.qc->quorumHash, id, msgHash, static_cast(memberIdx), {}); uint256 signHash = sigShare.buildSignHash().Get(); - auto bls_scheme = bls::bls_legacy_scheme.load(); + auto bls_scheme = bls::EffectiveLegacyScheme(); sigShare.sigShare.Set(skShare.Sign(signHash, bls_scheme), bls_scheme); if (!sigShare.sigShare.Get().IsValid()) { LogPrintf("CSigSharesManager::%s -- failed to sign sigShare. signHash=%s, id=%s, msgHash=%s, time=%s\n", __func__, diff --git a/src/llmq/utils.cpp b/src/llmq/utils.cpp index 0d34b0c43dcf..20a8c0ec3bd8 100644 --- a/src/llmq/utils.cpp +++ b/src/llmq/utils.cpp @@ -563,12 +563,12 @@ BlsCheck::~BlsCheck() = default; bool BlsCheck::operator()() { if (m_pubkeys.size() > 1) { - if (!m_sig.VerifySecureAggregated(m_pubkeys, m_msg_hash)) { + if (!m_sig.VerifySecureAggregated(m_pubkeys, m_msg_hash, m_legacy_scheme)) { LogPrint(BCLog::LLMQ, "%s\n", m_id_string); return false; } } else if (m_pubkeys.size() == 1) { - if (!m_sig.VerifyInsecure(m_pubkeys.back(), m_msg_hash)) { + if (!m_sig.VerifyInsecure(m_pubkeys.back(), m_msg_hash, m_legacy_scheme)) { LogPrint(BCLog::LLMQ, "%s\n", m_id_string); return false; } @@ -586,6 +586,7 @@ void BlsCheck::swap(BlsCheck& obj) noexcept std::swap(m_pubkeys, obj.m_pubkeys); std::swap(m_msg_hash, obj.m_msg_hash); std::swap(m_id_string, obj.m_id_string); + std::swap(m_legacy_scheme, obj.m_legacy_scheme); } std::optional> ComputeQuorumMembersFromWorkBlock( diff --git a/src/llmq/utils.h b/src/llmq/utils.h index 65c4c2682808..fe45ecf195d1 100644 --- a/src/llmq/utils.h +++ b/src/llmq/utils.h @@ -48,6 +48,8 @@ struct BlsCheck { std::vector m_pubkeys; uint256 m_msg_hash; std::string m_id_string; + //! The building thread's scheme; CCheckQueue runs the check on another thread + bool m_legacy_scheme{bls::EffectiveLegacyScheme()}; public: BlsCheck(); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index ca84a4b531da..aa3373b449a3 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -356,7 +356,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc if (fV20Active_context) { CBLSSignature best_cl_sig; const bool has_best_cl{CalcCbTxBestChainlock(m_chainlocks, m_chainstate.m_blockman, pindexPrev, cbTx.bestCLHeightDiff, best_cl_sig)}; - cbTx.bestCLSignature.Set(best_cl_sig, bls::bls_legacy_scheme.load()); + cbTx.bestCLSignature.Set(best_cl_sig, bls::EffectiveLegacyScheme()); if (has_best_cl) { LogPrintf("CreateNewBlock() h[%d] CbTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); } else { diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 3d211138d3ea..90691979bfc6 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -702,7 +702,7 @@ static RPCHelpMan quorum_verify() const uint256 id(ParseHashV(request.params[1], "id")); const uint256 msgHash(ParseHashV(request.params[2], "msgHash")); - const bool use_bls_legacy = bls::bls_legacy_scheme.load(); + const bool use_bls_legacy = bls::EffectiveLegacyScheme(); CBLSSignature sig; if (!sig.SetHexStr(request.params[3].get_str(), use_bls_legacy)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index 83715e84f296..57257277fd5a 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -21,6 +21,10 @@ #include +#include +#include +#include + BOOST_AUTO_TEST_SUITE(bls_tests) void FuncSign(const bool legacy_scheme) @@ -317,7 +321,8 @@ static void AddCancellingMessages(std::vector& vec, uint32_t sourceId1, m2.valid = false; } -static void Verify(std::vector& vec, bool secureVerification, bool perMessageFallback) +static void Verify(std::vector& vec, bool secureVerification, bool perMessageFallback, + bool verifyUnderOppositeScheme) { CBLSBatchVerifier batchVerifier(secureVerification, perMessageFallback); @@ -332,7 +337,14 @@ static void Verify(std::vector& vec, bool secureVerification, bool perM batchVerifier.PushMessage(m.sourceId, m.msgId, m.msgHash, m.sig, m.pk); } - batchVerifier.Verify(); + if (verifyUnderOppositeScheme) { + std::thread{[&] { + const bls::ScopedLegacySchemeOverride opposite{!bls::EffectiveLegacyScheme()}; + batchVerifier.Verify(); + }}.join(); + } else { + batchVerifier.Verify(); + } BOOST_CHECK(batchVerifier.badSources == expectedBadSources); @@ -345,10 +357,13 @@ static void Verify(std::vector& vec, bool secureVerification, bool perM static void Verify(std::vector& vec) { - Verify(vec, false, false); - Verify(vec, true, false); - Verify(vec, false, true); - Verify(vec, true, true); + // A batch verifies under the scheme it was built under, wherever Verify() runs + for (const bool verifyUnderOppositeScheme : {false, true}) { + Verify(vec, false, false, verifyUnderOppositeScheme); + Verify(vec, true, false, verifyUnderOppositeScheme); + Verify(vec, false, true, verifyUnderOppositeScheme); + Verify(vec, true, true, verifyUnderOppositeScheme); + } } void FuncBatchVerifier(const bool legacy_scheme) @@ -956,6 +971,98 @@ BOOST_AUTO_TEST_CASE(test_get_hash_consistency) BOOST_CHECK(hash1 == hash2); } +BOOST_AUTO_TEST_CASE(scheme_override_is_per_thread_and_nests) +{ + const bool saved_scheme{bls::bls_legacy_scheme.load()}; + bls::bls_legacy_scheme.store(false); + { + bls::ScopedLegacySchemeOverride outer{true}; + BOOST_CHECK(bls::EffectiveLegacyScheme()); + bool other_thread{true}; + std::thread{[&] { other_thread = bls::EffectiveLegacyScheme(); }}.join(); + BOOST_CHECK(!other_thread); + { + // A committed scope hands its scheme to the enclosing override... + bls::ScopedLegacySchemeOverride inner{true}; + bls::SetEffectiveLegacyScheme(false); + inner.Commit(); + } + BOOST_CHECK(!bls::EffectiveLegacyScheme()); + { + // ...an uncommitted one does not. + bls::ScopedLegacySchemeOverride inner{true}; + } + BOOST_CHECK(!bls::EffectiveLegacyScheme()); + outer.Commit(); + } + // The outermost override ends with its scope, committed or not, and never + // touched the process-wide scheme. + BOOST_CHECK(!bls::ScopedLegacySchemeOverride::Active()); + BOOST_CHECK(!bls::bls_legacy_scheme.load()); + bls::SetEffectiveLegacyScheme(true); + BOOST_CHECK(bls::bls_legacy_scheme.load()); + bls::bls_legacy_scheme.store(saved_scheme); +} + +BOOST_AUTO_TEST_CASE(worker_jobs_keep_the_submitting_scheme) +{ + const bool saved_scheme{bls::bls_legacy_scheme.load()}; + CBLSSecretKey sk; + sk.MakeNewKey(); + const CBLSPublicKey pk{sk.GetPublicKey()}; + + struct Job { + bool submitLegacy; + bool signedLegacy; + }; + // The single worker thread is held while a phase is queued, so its verification batches are fixed: the first + // job runs alone and each scheme change starts a new batch. Phase 1: an aggregated batch that passes, then a + // signature that would pass if it joined that batch. Phase 2: a batch that falls back to per-signature checks, + // then one whose signatures are valid only under the scheme it was not submitted under. + const std::vector> phases{ + {{true, true}, {true, true}, {true, true}, {false, true}}, + {{false, false}, {false, true}, {false, false}, {true, false}, {true, false}}, + }; + + for (const bool processScheme : {false, true}) { + bls::bls_legacy_scheme.store(processScheme); + CBLSWorker worker; + worker.Start(1); + for (const auto& phase : phases) { + std::promise held; + std::promise release; + worker.AsyncSign(sk, GetRandHash(), [&held, released = release.get_future().share()](const CBLSSignature&) { + held.set_value(); + released.wait(); + }); + held.get_future().wait(); + BOOST_REQUIRE(!worker.IsAsyncVerifyInProgress()); + + std::vector hashes; + std::vector> verified; + std::vector> signatures; + for (const auto& job : phase) { + const bls::ScopedLegacySchemeOverride submit{job.submitLegacy}; + const uint256& hash{hashes.emplace_back(GetRandHash())}; + verified.emplace_back(worker.AsyncVerifySig(sk.Sign(hash, job.signedLegacy), pk, hash)); + auto signature{std::make_shared>()}; + signatures.emplace_back(signature->get_future()); + worker.AsyncSign(sk, hash, [signature](const CBLSSignature& sig) { signature->set_value(sig); }); + } + release.set_value(); + + for (size_t i{0}; i < phase.size(); ++i) { + BOOST_CHECK_EQUAL(verified[i].get(), phase[i].submitLegacy == phase[i].signedLegacy); + const CBLSSignature sig{signatures[i].get()}; + BOOST_CHECK(sig.VerifyInsecure(pk, hashes[i], phase[i].submitLegacy)); + BOOST_CHECK(!sig.VerifyInsecure(pk, hashes[i], !phase[i].submitLegacy)); + } + } + worker.Stop(); + } + bls::bls_legacy_scheme.store(saved_scheme); +} + BOOST_AUTO_TEST_CASE(v19_boundary_validation_failure_restores_bls_scheme) { TestChainV19Setup setup; diff --git a/src/test/llmq_utils_tests.cpp b/src/test/llmq_utils_tests.cpp index 0978a2e3ba3e..afd224a08c1e 100644 --- a/src/test/llmq_utils_tests.cpp +++ b/src/test/llmq_utils_tests.cpp @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -459,6 +460,53 @@ BOOST_AUTO_TEST_CASE(sig_share_batch_verifier_judges_each_sender_by_its_own_shar } } +BOOST_AUTO_TEST_CASE(bls_check_keeps_the_scheme_it_was_built_under) +{ + // ConnectBlock builds commitment checks under its chainstate's scheme, which + // may be a thread-local one, and CCheckQueue runs them on other threads. + CCheckQueue queue{/*nBatchSizeIn=*/1}; + queue.StartWorkerThreads(/*threads_num=*/2); + const bool saved_scheme{bls::bls_legacy_scheme.load()}; + const uint256 msg_hash{GetRandHash()}; + std::vector sks(3); + std::vector pks; + for (auto& sk : sks) { + sk.MakeNewKey(); + pks.push_back(sk.GetPublicKey()); + } + for (const bool legacy : {true, false}) { + bls::bls_legacy_scheme.store(!legacy); + std::vector sigs; + sigs.reserve(sks.size()); + for (const auto& sk : sks) { + sigs.push_back(sk.Sign(msg_hash, legacy)); + } + const CBLSSignature members_sig{[&] { + const bls::ScopedLegacySchemeOverride signer_scheme{legacy}; + return CBLSSignature::AggregateSecure(sigs, pks, msg_hash); + }()}; + const auto build = [&](bool scheme) { + const bls::ScopedLegacySchemeOverride override{scheme}; + std::vector checks; + checks.emplace_back(sigs[0], std::vector{pks[0]}, msg_hash, "quorum sig"); + checks.emplace_back(members_sig, pks, msg_hash, "members sig"); + return checks; + }; + const auto run = [&](std::vector checks) { + CCheckQueueControl control{&queue}; + control.Add(std::move(checks)); + return control.Wait(); + }; + BOOST_CHECK(run(build(legacy))); + // One at a time, so neither check's rejection masks the other's + for (const auto& check : build(!legacy)) { + BOOST_CHECK(!run({check})); + } + } + bls::bls_legacy_scheme.store(saved_scheme); + queue.StopWorkerThreads(); +} + BOOST_AUTO_TEST_CASE(batched_sig_shares_rejects_oversized_inner_vector) { CDataStream stream{SER_NETWORK, PROTOCOL_VERSION}; diff --git a/src/test/quorum_proofs_tests.cpp b/src/test/quorum_proofs_tests.cpp index 7aab713dcd3b..a605ed75c70b 100644 --- a/src/test/quorum_proofs_tests.cpp +++ b/src/test/quorum_proofs_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying file COPYING. #include #include +#include #include #include #include @@ -22,6 +23,7 @@ #include #include #include +#include #include // Actual testnet archive proof, heights 1548500 -> 1549547. Independently @@ -476,6 +478,65 @@ BOOST_FIXTURE_TEST_CASE(consensus_valid_envelopes_are_provable, QuorumProofGener BOOST_CHECK_THROW(llmq::EncodeBootstrap(*proof, wrong, {record}), std::runtime_error); BOOST_CHECK(state.blockHash == proof->target.header.GetHash()); } +BOOST_FIXTURE_TEST_CASE(chainlock_verification_ignores_another_threads_scheme, QuorumProofGenerationSetup) +{ + // A background chainstate validating history across V19 holds its own BLS + // scheme while other threads verify ChainLocks and recovered signatures for + // the active chain. + CreateHistory(); + const auto& consensus = Params().GetConsensus(); + const auto params = *Params().GetLLMQ(consensus.llmqTypeChainLocks); + const auto& qman = *m_node.llmq_ctx->qman; + const auto& chain = *WITH_LOCK(cs_main, return &m_node.chainman->ActiveChain()); + const auto* locked = WITH_LOCK(cs_main, return chain.Tip()); + const auto* start = WITH_LOCK(cs_main, return llmq::SelectQuorumForSigningStartBlock(chain, locked->nHeight)); + const uint256 request_id{chainlock::GenSigRequestId(locked->nHeight)}; + const auto signer = llmq::SelectCommitmentForSigning(params, qman, request_id, start); + BOOST_REQUIRE(signer); + const auto match = std::find_if(commitments.begin(), commitments.end(), + [&](const auto& qc) { return qc.quorumHash == signer->quorumHash; }); + BOOST_REQUIRE(match != commitments.end()); + const CBLSSecretKey& key{keys[match - commitments.begin()]}; + const uint256 sign_hash{llmq::SignHash{params.type, signer->quorumHash, request_id, locked->GetBlockHash()}.Get()}; + const auto make_clsig = [&](bool legacy) { + return chainlock::ChainLockSig{locked->nHeight, locked->GetBlockHash(), key.Sign(sign_hash, legacy)}; + }; + const auto verify = [&](const chainlock::ChainLockSig& clsig) { + return chainlock::VerifyChainLock(consensus, qman, clsig, start); + }; + const auto batch_clean = [&](const chainlock::ChainLockSig& clsig) { + CBLSBatchVerifier batch_verifier(/*_secureVerification=*/false, /*_perMessageFallback=*/false); + batch_verifier.PushMessage(/*sourceId=*/0, ::SerializeHash(clsig), sign_hash, clsig.getSig(), + signer->quorumPublicKey); + batch_verifier.Verify(); + return batch_verifier.badSources.empty(); + }; + + const bool saved_scheme{bls::bls_legacy_scheme.load()}; + for (const bool background_legacy : {true, false}) { + bls::bls_legacy_scheme.store(!background_legacy); + const auto active_clsig{make_clsig(!background_legacy)}; + llmq::VerifyRecSigStatus background_own{llmq::VerifyRecSigStatus::NoQuorum}; + llmq::VerifyRecSigStatus background_active{llmq::VerifyRecSigStatus::NoQuorum}; + llmq::VerifyRecSigStatus other_thread{llmq::VerifyRecSigStatus::NoQuorum}; + bool other_thread_batch_clean{false}; + { + const bls::ScopedLegacySchemeOverride scheme{background_legacy}; + background_own = verify(make_clsig(background_legacy)); + background_active = verify(active_clsig); + std::thread{[&] { + other_thread = verify(active_clsig); + other_thread_batch_clean = batch_clean(active_clsig); + }}.join(); + } + BOOST_CHECK(background_own == llmq::VerifyRecSigStatus::Valid); + BOOST_CHECK(background_active == llmq::VerifyRecSigStatus::Invalid); + BOOST_CHECK(other_thread == llmq::VerifyRecSigStatus::Valid); + BOOST_CHECK(other_thread_batch_clean); + } + bls::bls_legacy_scheme.store(saved_scheme); +} + BOOST_AUTO_TEST_CASE(real_testnet_wire_and_crypto) { auto bytes = ParseHex(TESTNET_PROOF); auto proof = llmq::QuorumProofChain::Decode(bytes); From a83ceb30378ee28dd02312bd08929bb19ffbcc13 Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 6 Nov 2026 01:18:52 -0600 Subject: [PATCH 2/3] fix(validation): keep a non-active chainstate's BLS scheme on its thread ConnectBlock establishes the BLS scheme of the block it connects by writing the process-wide bls::bls_legacy_scheme, and ConnectTip and DisconnectTip restore it only on return. A background chainstate validating pre-V19 history under a post-V19 snapshot therefore leaves the whole process on the legacy scheme for most of each background block connection, while other threads verify ChainLocks, recovered signatures and InstantSend locks for the active chain outside cs_main. Valid ChainLocks then fail (Misbehaving(10) and a poisoned seen set) and batched recovered-signature verification bans honest peers. ScopedBLSLegacyScheme now writes the process-wide scheme only for a scope that may commit the active chainstate's tip. Every other scope (a background chainstate, a TestBlockValidity dry run, VerifyDB, RollforwardBlock), and any scope nested in one, sets a thread-local override instead. The V19 switch in Process/UndoSpecialTxsInBlock goes through bls::SetEffectiveLegacyScheme(), and the EHF signal rebuild at startup uses a scoped override instead of flipping the global. ConnectTip and DisconnectTip Assume() that no thread-local scope encloses a move of the active chainstate's tip. With a single chainstate the active chain's path is unchanged. The thread-local scopes give their own thread the values the process-wide flag used to take, but other threads no longer see them; for example a `verifychain 4` reaching below V19 no longer switches the running node to legacy while it runs. VerifyDB no longer touches the process-wide scheme at all, so VerifyLoadedChainstate's re-store of it after VerifyDB (already unreachable, because VerifyDB restored the scheme on return) is removed. The existing background-connect test never connected a block (the snapshot setup drops the data below the base). It now restores one block's data, connects it, and checks from inside ConnectTip that another thread still verifies a basic signature and batch. The V19 boundary test also runs a level-4 VerifyDB across V19 and checks that the process-wide scheme stays basic throughout. Co-Authored-By: Claude Opus 5.5 --- src/evo/mnhftx.cpp | 8 +-- src/evo/specialtxman.cpp | 16 ++--- src/node/chainstate.cpp | 7 --- src/test/bls_tests.cpp | 29 +++++++++ src/test/validation_chainstate_tests.cpp | 66 +++++++++++++++++--- src/validation.cpp | 77 ++++++++++++++---------- 6 files changed, 142 insertions(+), 61 deletions(-) diff --git a/src/evo/mnhftx.cpp b/src/evo/mnhftx.cpp index 1a2555cd3628..a113e487604f 100644 --- a/src/evo/mnhftx.cpp +++ b/src/evo/mnhftx.cpp @@ -387,10 +387,10 @@ bool CMNHFManager::ForceSignalDBUpdate(const CBlockIndex* tip) // force ehf signals db update auto dbTx = m_evoDb.BeginTransaction(); - const bool last_legacy = bls::bls_legacy_scheme.load(); - bls::bls_legacy_scheme.store(false); - GetSignalsStage(tip); - bls::bls_legacy_scheme.store(last_legacy); + { + const bls::ScopedLegacySchemeOverride basic_scheme{/*legacy=*/false}; + GetSignalsStage(tip); + } dbTx->Commit(); // flush it to disk diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index eb2f68cb730a..fb2bd6b890cf 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -1092,11 +1092,11 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(Chainstate& chainstate, const LogPrint(BCLog::BENCHMARK, " - m_mnhfman->ProcessBlock: %.2fms [%.2fs]\n", Ticks(nTime8 - nTime7), Ticks(nTimeMnehf)); - if (DeploymentActiveAfter(pindex, m_consensus_params, Consensus::DEPLOYMENT_V19) && bls::bls_legacy_scheme.load()) { + if (DeploymentActiveAfter(pindex, m_consensus_params, Consensus::DEPLOYMENT_V19) && bls::EffectiveLegacyScheme()) { // NOTE: The block next to the activation is the one that is using new rules. // V19 activated just activated, so we must switch to the new rules here. - bls::bls_legacy_scheme.store(false); - LogPrintf("CSpecialTxProcessor::%s -- bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); + bls::SetEffectiveLegacyScheme(false); + LogPrintf("CSpecialTxProcessor::%s -- bls_legacy_scheme=%d\n", __func__, bls::EffectiveLegacyScheme()); } } catch (const EvoDbInconsistencyError& e) { // Local EvoDB corruption detected below (the node is already @@ -1114,14 +1114,14 @@ bool CSpecialTxProcessor::UndoSpecialTxsInBlock(const Chainstate& chainstate, co { AssertLockHeld(::cs_main); - auto bls_legacy_scheme = bls::bls_legacy_scheme.load(); + const bool bls_legacy_scheme{bls::EffectiveLegacyScheme()}; try { if (!DeploymentActiveAt(*pindex, m_consensus_params, Consensus::DEPLOYMENT_V19) && !bls_legacy_scheme) { // NOTE: The block next to the activation is the one that is using new rules. // Removing the activation block here, so we must switch back to the old rules. - bls::bls_legacy_scheme.store(true); - LogPrintf("CSpecialTxProcessor::%s -- bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); + bls::SetEffectiveLegacyScheme(true); + LogPrintf("CSpecialTxProcessor::%s -- bls_legacy_scheme=%d\n", __func__, bls::EffectiveLegacyScheme()); } if (!m_mnhfman->UndoBlock(block, pindex)) { @@ -1136,8 +1136,8 @@ bool CSpecialTxProcessor::UndoSpecialTxsInBlock(const Chainstate& chainstate, co return false; } } catch (const std::exception& e) { - bls::bls_legacy_scheme.store(bls_legacy_scheme); - LogPrintf("CSpecialTxProcessor::%s -- bls_legacy_scheme=%d\n", __func__, bls::bls_legacy_scheme.load()); + bls::SetEffectiveLegacyScheme(bls_legacy_scheme); + LogPrintf("CSpecialTxProcessor::%s -- bls_legacy_scheme=%d\n", __func__, bls::EffectiveLegacyScheme()); LogError("CSpecialTxProcessor::%s -- FAILURE! %s\n", __func__, e.what()); return false; } diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index ea001791d571..13ecf5e54839 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -437,13 +437,6 @@ ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const C break; } // no default case, so the compiler can warn about missing cases - // VerifyDB() disconnects blocks which might result in us switching back to legacy. - // Make sure we use the right scheme. - if (v19active && bls::bls_legacy_scheme.load()) { - bls::bls_legacy_scheme.store(false); - if (notify_bls_state) notify_bls_state(bls::bls_legacy_scheme.load()); - } - if (options.check_level >= 3) { chainstate->ResetBlockFailureFlags(nullptr); } diff --git a/src/test/bls_tests.cpp b/src/test/bls_tests.cpp index 57257277fd5a..9becd1b6f4df 100644 --- a/src/test/bls_tests.cpp +++ b/src/test/bls_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include