Skip to content

fix: keep a background chainstate's BLS scheme off other threads #8066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PastaPastaPasta wants to merge 3 commits into
base: develop
Choose a base branch
from
+589 −108
Open

fix: keep a background chainstate's BLS scheme off other threads #8066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34233a1
refactor(bls): resolve the implicit BLS scheme per thread
PastaPastaPasta Nov 6, 2026
a83ceb3
fix(validation): keep a non-active chainstate's BLS scheme on its thread
PastaPastaPasta Nov 6, 2026
7e080ec
fix(validation): establish the BLS scheme when the active chainstate …
PastaPastaPasta Nov 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 51 additions & 9 deletions src/bls/bls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,40 @@
#include <cstring>

namespace bls {
std::atomic<bool> bls_legacy_scheme = std::atomic<bool>(true);
std::atomic<bool> bls_legacy_scheme = std::atomic<bool>(true);

namespace {
//! Trivially destructible, like util/threadnames.cpp's thread-local name.
thread_local std::optional<bool> 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<bls::CoreMPL> pSchemeLegacy{std::make_unique<bls::LegacySchemeMPL>()};
static const std::unique_ptr<bls::CoreMPL> pScheme(std::make_unique<bls::BasicSchemeMPL>());

Expand Down Expand Up @@ -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;
}
Expand All @@ -166,7 +197,7 @@ CBLSPublicKey CBLSPublicKey::AggregateInsecure(Span<CBLSPublicKey> 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;
Expand Down Expand Up @@ -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;
}
Expand All @@ -251,7 +282,7 @@ CBLSSignature CBLSSignature::AggregateInsecure(Span<CBLSSignature> 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;
Expand Down Expand Up @@ -283,7 +314,7 @@ CBLSSignature CBLSSignature::AggregateSecure(Span<CBLSSignature> 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;
Expand Down Expand Up @@ -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<CBLSPublicKey> pubKeys, Span<uint256> hashes) const
{
return VerifyInsecureAggregated(pubKeys, hashes, bls::EffectiveLegacyScheme());
}

bool CBLSSignature::VerifyInsecureAggregated(Span<CBLSPublicKey> pubKeys, Span<uint256> hashes,
const bool specificLegacyScheme) const
{
if (!IsValid()) {
return false;
Expand All @@ -346,13 +383,18 @@ bool CBLSSignature::VerifyInsecureAggregated(Span<CBLSPublicKey> pubKeys, Span<u
}

try {
return Scheme(bls::bls_legacy_scheme.load())->AggregateVerify(pubKeyVec, hashes2, impl);
return Scheme(specificLegacyScheme)->AggregateVerify(pubKeyVec, hashes2, impl);
} catch (...) {
return false;
}
}

bool CBLSSignature::VerifySecureAggregated(Span<CBLSPublicKey> pks, const uint256& hash) const
{
return VerifySecureAggregated(pks, hash, bls::EffectiveLegacyScheme());
}

bool CBLSSignature::VerifySecureAggregated(Span<CBLSPublicKey> pks, const uint256& hash, const bool specificLegacyScheme) const
{
if (pks.empty()) {
return false;
Expand All @@ -365,7 +407,7 @@ bool CBLSSignature::VerifySecureAggregated(Span<CBLSPublicKey> 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;
}
Expand Down
52 changes: 46 additions & 6 deletions src/bls/bls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,47 @@
#include <array>
#include <atomic>
#include <mutex>
#include <optional>
#include <ranges>

namespace bls {
extern std::atomic<bool> 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<bool> 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<bool> m_saved;
bool m_committed{false};
};
} // namespace bls

// reversed BLS12-381
constexpr int BLS_CURVE_ID_SIZE{32};
Expand Down Expand Up @@ -177,7 +213,7 @@ class CBLSWrapper
template <typename Stream>
inline void Serialize(Stream& s) const
{
Serialize(s, bls::bls_legacy_scheme.load());
Serialize(s, bls::EffectiveLegacyScheme());
Comment thread
PastaPastaPasta marked this conversation as resolved.
Resolved
}

template <typename Stream>
Expand Down Expand Up @@ -205,7 +241,7 @@ class CBLSWrapper
template <typename Stream>
inline void Unserialize(Stream& s)
{
Unserialize(s, bls::bls_legacy_scheme.load());
Unserialize(s, bls::EffectiveLegacyScheme());
}

inline bool CheckMalleable(Span<uint8_t> vecBytes, const bool specificLegacyScheme) const
Expand All @@ -228,7 +264,7 @@ class CBLSWrapper

inline std::string ToString() const
{
return ToString(bls::bls_legacy_scheme.load());
return ToString(bls::EffectiveLegacyScheme());
}
};

Expand Down Expand Up @@ -365,8 +401,12 @@ class CBLSSignature : public CBLSWrapper<bls::G2Element, BLS_CURVE_SIG_SIZE, CBL
void MulInsecure(const CBLSSecretKey& factor);
[[nodiscard]] bool VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash, const bool specificLegacyScheme) const;
[[nodiscard]] bool VerifyInsecure(const CBLSPublicKey& pubKey, const uint256& hash) const;
[[nodiscard]] bool VerifyInsecureAggregated(Span<CBLSPublicKey> pubKeys, Span<uint256> hashes,
const bool specificLegacyScheme) const;
[[nodiscard]] bool VerifyInsecureAggregated(Span<CBLSPublicKey> pubKeys, Span<uint256> hashes) const;

[[nodiscard]] bool VerifySecureAggregated(Span<CBLSPublicKey> pks, const uint256& hash,
const bool specificLegacyScheme) const;
[[nodiscard]] bool VerifySecureAggregated(Span<CBLSPublicKey> pks, const uint256& hash) const;

bool Recover(Span<CBLSSignature> sigs, Span<CBLSId> ids);
Expand Down Expand Up @@ -410,7 +450,7 @@ class CBLSLazyWrapper

public:
CBLSLazyWrapper() :
bufLegacyScheme(bls::bls_legacy_scheme.load())
bufLegacyScheme(bls::EffectiveLegacyScheme())
{}

explicit CBLSLazyWrapper(const CBLSLazyWrapper& r)
Expand Down
11 changes: 7 additions & 4 deletions src/bls/bls_batchverifier.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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())
{
}

Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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<uint256, std::vector<MessageMapIterator>>& byMessageHash)
Expand Down Expand Up @@ -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);
}
};

Expand Down
19 changes: 10 additions & 9 deletions src/bls/bls_worker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,8 +825,8 @@ std::set<size_t> 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));
});
}

Expand All @@ -838,18 +838,19 @@ void CBLSWorker::AsyncVerifySig(const CBLSSignature& sig, const CBLSPublicKey& p
return;
}

const bool legacy{bls::EffectiveLegacyScheme()};
std::unique_lock<std::mutex> 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);
Comment on lines +847 to +853

@thepastaclaw thepastaclaw Nov 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Resolved at ba207fc2; see the replies below.

🟡 Suggestion: Add regression coverage for worker scheme capture and mixed-scheme queueing

The PR changes CBLSWorker::AsyncSign and AsyncVerifySig to capture the submitting scheme and adds this mixed-scheme queue-flush condition, but no unit test invokes either worker API. The new CBLSBatchVerifier and BlsCheck tests exercise separate implementations, and AsyncVerifySig's only external caller is the benchmark, so the functional suites do not cover this queue behavior either. Add deterministic coverage in bls_tests using worker.Start(1) and a promise-controlled callback to hold the worker while jobs are submitted under both scoped schemes. Verify signing, singleton and aggregated verification, per-signature fallback, and opposite-scheme rejection after the submitting scopes have ended and the process default differs from the queued jobs' schemes. This would catch loss of a scheme capture or removal of the flush condition without timing-dependent sleeps.

source: gpt-6.1-sol (phase2-reviewer: general)

@PastaPastaPasta PastaPastaPasta Nov 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and added in ba207fc, folded into "refactor(bls): resolve the implicit BLS scheme per thread": bls_tests/worker_jobs_keep_the_submitting_scheme.

  • A CBLSWorker started with one thread is held by a blocking AsyncSign callback, using a promise handshake with no sleeps. With nothing in flight, AsyncSign and AsyncVerifySig jobs are submitted under alternating ScopedLegacySchemeOverride scopes, so the batch layout is fixed. The worker is released only after the scopes have ended, and the test runs once with the process default set to each scheme.
  • Verification covers a singleton, an aggregated batch that passes, an aggregated batch that falls back to per-signature checks, and opposite-scheme rejection, including signatures that are valid only under the process default. Each AsyncSign result must verify under the submitting scheme and fail under the other.
  • One case sits at a scheme boundary: a basic-scope job carrying a legacy signature, right after an all-valid legacy batch. Without the mixed-scheme flush it is aggregated into that batch under legacy and reported valid, so the flush changes outcomes as well as cost.

Each of these mutants fails the test: AsyncSign reading the scheme when the job runs (18 failures); all verify calls reading it at run time (9), or only the singleton (3), aggregate (2) or per-signature fallback (4) call; dropping the mixed-scheme flush (2). The real code passes 21/21 runs under TSan, along with bls_tests, llmq_utils_tests, quorum_proofs_tests and validation_chainstate_tests. The only TSan report is the existing CBLSWorker::Start → RenameThreadPool race, which develop shows too.

🤖 Posted autonomously by Claude on behalf of pasta.

@thepastaclaw thepastaclaw Nov 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved (re-reviewed at ba207fc2): Your worker_jobs_keep_the_submitting_scheme test holds a single worker with a promise handshake until the submitting scopes have ended, and its job sequences cover signing, singleton verification, successful aggregation, fallback, and opposite-scheme rejection under both process defaults. I also verified that the legacy-to-basic boundary case would incorrectly pass without the mixed-scheme flush; this resolves the finding by static inspection, without independently rerunning the reported mutation or TSan checks.

if (sigVerifyBatchesInProgress == 0 || sigVerifyQueue.size() >= SIG_VERIFY_BATCH_SIZE) {
PushSigVerifyBatch();
}
Expand All @@ -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<std::mutex> l(sigVerifyMutex);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/bls/bls_worker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
}
};
Expand Down
6 changes: 3 additions & 3 deletions src/chainlock/clsig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ class DecodedSignatureCache
CBLSSignature Decode(const std::array<uint8_t, CBLSSignature::SerSize>& 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;
{
Expand Down
8 changes: 4 additions & 4 deletions src/evo/mnhftx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading