From 37f5af91ed3a8252dbd23fd3247aeedfcad743e4 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 22 Sep 2026 18:22:43 -0500 Subject: [PATCH 1/5] perf(chainlock): memoize coinbase ChainLocks and extend the proof chain snapshot A warm getquorumproofchain request from a release checkpoint spent about 60% of its time re-reading carrier coinbases for the ChainLock binary search and 20% rebuilding a 2.5M-entry CChain snapshot on every new tip. Most of the coinbase cost was decoding each carrier's BLS signature, although the search only compares heights. Memoize each carrier's ChainLock height offset and raw signature bytes process-wide, keyed by carrier block hash. A block hash fixes the block's contents, so entries never go stale and a reorg simply stops asking for orphaned hashes. Decode a signature only when a certificate is actually built, through a small cache of decoded points. Seed each new proof chain snapshot from the previous one so SetTip only rewrites heights that changed. Once memoized, a carrier's certificate stays available even if its block data is later removed; missing data that was never read is still an error. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/chain.h | 3 + src/chainlock/clsig.cpp | 153 +++++++++++++++++++++++++++--- src/chainlock/clsig.h | 10 ++ src/llmq/quorumproofs.cpp | 2 +- src/rpc/quorums.cpp | 10 +- src/test/llmq_chainlock_tests.cpp | 17 +++- src/test/quorum_proofs_tests.cpp | 4 +- 7 files changed, 177 insertions(+), 22 deletions(-) diff --git a/src/chain.h b/src/chain.h index d3ea67d24dfa..f7980ab4b45c 100644 --- a/src/chain.h +++ b/src/chain.h @@ -485,6 +485,9 @@ class CChain /** Set/initialize a chain with a given tip. */ void SetTip(CBlockIndex& block); + /** Copy another chain's entries so a following SetTip only rewrites the changed suffix. */ + void CopyFrom(const CChain& other) { vChain = other.vChain; } + /** Return a CBlockLocator that refers to the tip in of this chain. */ CBlockLocator GetLocator() const; diff --git a/src/chainlock/clsig.cpp b/src/chainlock/clsig.cpp index 4977ba9567bf..4174f67eb7a6 100644 --- a/src/chainlock/clsig.cpp +++ b/src/chainlock/clsig.cpp @@ -9,9 +9,13 @@ #include #include #include +#include #include #include +#include #include +#include +#include #include #include @@ -21,19 +25,101 @@ namespace chainlock { static constexpr std::string_view CLSIG_REQUESTID_PREFIX{"clsig"}; static constexpr size_t MAX_HISTORICAL_CARRIER_READS{16384}; -CoinbaseChainLockReader::CoinbaseChainLockReader(const CChain& chain) : - m_tip(chain.Tip()) +namespace { +/** What one validated coinbase says about ChainLocks: the embedded signature + * bytes and height offset, or nothing. Holding raw bytes keeps a hit free of + * BLS point decompression; only certificates a proof actually uses are parsed. */ +struct CarrierEntry { + bool has_chainlock{false}; + uint32_t height_diff{0}; + std::array signature{}; +}; + +/** Process-wide memo of coinbase ChainLocks, keyed by carrier block hash. + * A block hash fixes the block's contents, so an entry never goes stale and a + * reorg simply stops asking for the orphaned hashes. Per-request binary search + * touches ~40 carriers per certificate, overwhelmingly the same ones for every + * request, so this replaces most block reads and signature decoding. */ +class CarrierCache { + Mutex m_mutex; + // 32 KiB * ~120 B ≈ 4 MiB; covers every probe of years of mainnet history. + Uint256LruHashMap m_entries GUARDED_BY(m_mutex); + +public: + bool Get(const uint256& hash, CarrierEntry& entry) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + LOCK(m_mutex); + return m_entries.get(hash, entry); + } + void Insert(const uint256& hash, const CarrierEntry& entry) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + LOCK(m_mutex); + m_entries.insert(hash, entry); + } + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + LOCK(m_mutex); + m_entries.clear(); + } +}; + +CarrierCache& GetCarrierCache() +{ + static CarrierCache cache; + return cache; } -std::optional CoinbaseChainLockReader::Read(int carrier_height) +/** Decoded ChainLock signatures, keyed by their serialized bytes. Decoding a G2 + * point (decompression plus subgroup check) costs far more than the lookups + * around it, and proofs sharing history ask for the same signatures each time. */ +class DecodedSignatureCache +{ + Mutex m_mutex; + // ~300 B per decoded point: 8192 entries ≈ 2.5 MiB. + Uint256LruHashMap m_entries GUARDED_BY(m_mutex); + +public: + CBLSSignature Decode(const std::array& bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + const uint256 key = Hash(bytes); + CBLSSignature sig; + { + LOCK(m_mutex); + if (m_entries.get(key, sig)) return sig; + } + sig.SetBytes(bytes, false); + if (sig.IsValid()) { + LOCK(m_mutex); + m_entries.insert(key, sig); + } + return sig; + } + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + LOCK(m_mutex); + m_entries.clear(); + } +}; + +DecodedSignatureCache& GetDecodedSignatureCache() +{ + static DecodedSignatureCache cache; + return cache; +} +} // namespace + +void ClearCoinbaseChainLockCacheForTesting() +{ + GetCarrierCache().Clear(); + GetDecodedSignatureCache().Clear(); +} + +namespace { + +/** Reads and validates one carrier coinbase. Throws on missing or invalid data. */ +CarrierEntry LoadCarrier(const CBlockIndex* carrier, int carrier_height) { - const auto* carrier = m_tip ? m_tip->GetAncestor(carrier_height) : nullptr; - if (!carrier || carrier_height < Params().GetConsensus().V20Height) return std::nullopt; - if (const auto it = m_cache.find(carrier_height); it != m_cache.end()) return it->second; - if (ShutdownRequested()) throw std::runtime_error("ChainLock lookup interrupted"); - if (m_cache.size() >= MAX_HISTORICAL_CARRIER_READS) - throw std::runtime_error("ChainLock disk-read budget exhausted"); CBlock block; if (!node::ReadBlockFromDisk(block, carrier, Params().GetConsensus()) || block.vtx.empty()) { throw std::runtime_error("Historical ChainLock block data unavailable"); @@ -46,19 +132,60 @@ std::optional CoinbaseChainLockReader::Read(int carrier_heigh if (cb->bestCLHeightDiff >= uint32_t(carrier_height)) { throw std::runtime_error("Invalid historical coinbase ChainLock height"); } - const auto chainlock = GetNonNullCoinbaseChainlock(block, carrier_height); - if (!chainlock) { + // Same conditions as GetNonNullCoinbaseChainlock, which would decode the + // payload (and its BLS signature) a second time; everything else is checked above. + CarrierEntry entry; + if (cb->bestCLSignature.IsValid()) { + entry.has_chainlock = true; + entry.height_diff = cb->bestCLHeightDiff; + entry.signature = cb->bestCLSignature.ToBytes(false); + } + return entry; +} +} // namespace + +CoinbaseChainLockReader::CoinbaseChainLockReader(const CChain& chain) : + m_tip(chain.Tip()) +{ +} + +std::optional CoinbaseChainLockReader::Read(int carrier_height) +{ + const auto* carrier = m_tip ? m_tip->GetAncestor(carrier_height) : nullptr; + if (!carrier || carrier_height < Params().GetConsensus().V20Height) return std::nullopt; + if (const auto it = m_cache.find(carrier_height); it != m_cache.end()) return it->second; + if (ShutdownRequested()) throw std::runtime_error("ChainLock lookup interrupted"); + if (m_cache.size() >= MAX_HISTORICAL_CARRIER_READS) + throw std::runtime_error("ChainLock disk-read budget exhausted"); + const uint256 carrier_hash = carrier->GetBlockHash(); + CarrierEntry entry; + if (!GetCarrierCache().Get(carrier_hash, entry)) { + entry = LoadCarrier(carrier, carrier_height); + GetCarrierCache().Insert(carrier_hash, entry); + } + if (!entry.has_chainlock) { return m_cache.emplace(carrier_height, std::nullopt).first->second; } - const int height = carrier_height - int(chainlock->second) - 1; + const int height = carrier_height - int(entry.height_diff) - 1; const auto* signed_block = m_tip->GetAncestor(height); if (!signed_block) throw std::runtime_error("Invalid historical ChainLock block height"); + // Deferred decode: binary search only compares heights, so most carriers + // never need their signature. Callers that build a certificate decode it. return m_cache .emplace(carrier_height, - CoinbaseChainLock{ChainLockSig{height, signed_block->GetBlockHash(), chainlock->first}, carrier}) + CoinbaseChainLock{ChainLockSig{height, signed_block->GetBlockHash(), {}}, carrier, entry.signature}) .first->second; } +ChainLockSig CoinbaseChainLock::Signed() const +{ + // Validated by consensus when the carrier was connected, so a failure here + // means corrupt block data; surface it rather than proving with it. + const CBLSSignature sig = GetDecodedSignatureCache().Decode(signature_bytes); + if (!sig.IsValid()) throw std::runtime_error("Invalid historical ChainLock signature"); + return ChainLockSig{clsig.getHeight(), clsig.getBlockHash(), sig}; +} + std::optional CoinbaseChainLockReader::Find(int minimum_height, int maximum_height) { const int tip_height = m_tip ? m_tip->nHeight : -1; diff --git a/src/chainlock/clsig.h b/src/chainlock/clsig.h index f7c02a456b97..a7571868726c 100644 --- a/src/chainlock/clsig.h +++ b/src/chainlock/clsig.h @@ -7,6 +7,7 @@ #include +#include #include #include #include @@ -28,6 +29,11 @@ namespace chainlock { struct CoinbaseChainLock { ChainLockSig clsig; const CBlockIndex* carrier{nullptr}; + /** Serialized signature as it appears in the carrier coinbase. `clsig` holds + * height and block hash only; call Signed() for a copy with the decoded + * signature. Decoding a BLS point is the costly part of reading a carrier. */ + std::array signature_bytes{}; + ChainLockSig Signed() const; }; /** Reads historical signatures from a fixed, validated chain view. @@ -47,6 +53,10 @@ class CoinbaseChainLockReader std::optional Find(int minimum_height, int maximum_height); }; +/** Coinbase ChainLocks are memoized process-wide by carrier block hash, so a + * carrier read once stays available even if its block data is later removed. */ +void ClearCoinbaseChainLockCacheForTesting(); + //! Generate clsig request ID with block height uint256 GenSigRequestId(const int32_t nHeight); diff --git a/src/llmq/quorumproofs.cpp b/src/llmq/quorumproofs.cpp index a91bb6f31aef..23ba74da8837 100644 --- a/src/llmq/quorumproofs.cpp +++ b/src/llmq/quorumproofs.cpp @@ -475,7 +475,7 @@ std::optional QuorumProofBuilder::Build(const CBlockIndex* che Require(targetIndex->GetBlockHash() == target.getBlockHash(), "target ChainLock block mismatch"); auto certificate = [&](const chainlock::CoinbaseChainLock& entry) { return ProofCertificate{uint32_t(entry.clsig.getHeight()), chain[entry.clsig.getHeight()]->GetBlockHeader(), - entry.clsig.getSig()}; + entry.Signed().getSig()}; }; QuorumProofChain proof; proof.anchor = StateAt(checkpoint); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 76e0e7198fd0..e380b41b52da 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -58,7 +58,13 @@ std::shared_ptr GetProofChainSnapshot(const ChainstateManager& cha { std::lock_guard lock(g_proof_chain_mutex); if (!g_proof_chain_snapshot || g_proof_chain_owner != &chainman || g_proof_chain_tip != tip) { + // Snapshots are shared and immutable, so a new tip needs a new object. Seed it + // from the previous one: CChain::SetTip then only rewrites heights that changed + // (new blocks, or the fork on a reorg) instead of all ~2.5M entries per block. auto snapshot = std::make_shared(); + if (g_proof_chain_snapshot && g_proof_chain_owner == &chainman) { + snapshot->CopyFrom(*g_proof_chain_snapshot); + } snapshot->SetTip(*tip); g_proof_chain_snapshot = std::move(snapshot); g_proof_chain_owner = &chainman; @@ -1457,7 +1463,7 @@ static RPCHelpMan getchainlockbyheight() UniValue result(UniValue::VOBJ); result.pushKV("height", height); result.pushKV("blockhash", entry->clsig.getBlockHash().ToString()); - result.pushKV("signature", entry->clsig.getSig().ToString()); + result.pushKV("signature", entry->Signed().getSig().ToString()); result.pushKV("cbtx_height", entry->carrier->nHeight); return result; } catch (const std::exception& e) { @@ -1536,7 +1542,7 @@ static RPCHelpMan getquorumproofchain() } chainlock::ChainLockSig target_signature; if (target_chainlock) { - target_signature = target_chainlock->clsig; + target_signature = target_chainlock->Signed(); target_guard = target_chainlock->carrier; } // Platform can already reference a tip ChainLock before another diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index d6cdec5acfaf..7e68769180ea 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -121,7 +121,7 @@ BOOST_FIXTURE_TEST_CASE(historical_coinbase_lookup_from_disk, RegTestingSetup) BOOST_REQUIRE(entry); BOOST_CHECK_EQUAL(entry->clsig.getHeight(), expected); BOOST_CHECK(entry->clsig.getBlockHash() == chain[expected]->GetBlockHash()); - BOOST_CHECK(entry->clsig.getSig() == signature); + BOOST_CHECK(entry->Signed().getSig() == signature); const int carrier = expected == activation + 1 ? activation + 20 : expected == activation + 65 ? activation + 80 : activation + 110; @@ -143,10 +143,19 @@ BOOST_FIXTURE_TEST_CASE(historical_coinbase_lookup_from_disk, RegTestingSetup) BOOST_CHECK(!after_disconnect.Find(activation + 65, activation + 65)); BOOST_REQUIRE(after_disconnect.Find(activation + 1, activation + 1)); - // Unavailable block data is an error, distinct from an absent certificate. - WITH_LOCK(cs_main, indexes[activation + 3].nStatus &= ~BLOCK_HAVE_DATA); + // A carrier's certificate is a fact about its block hash, so once read it is + // served from the process-wide memo even if the block data goes away... + BOOST_REQUIRE(chainlock::CoinbaseChainLockReader(chain).Read(activation + 30)); + WITH_LOCK(cs_main, indexes[activation + 30].nStatus &= ~BLOCK_HAVE_DATA); + const auto memoized = chainlock::CoinbaseChainLockReader(chain).Read(activation + 30); + BOOST_REQUIRE(memoized); + BOOST_CHECK(memoized->Signed().getSig() == signature); + + // ...while unavailable block data that was never read is an error, distinct + // from an absent certificate. + chainlock::ClearCoinbaseChainLockCacheForTesting(); chainlock::CoinbaseChainLockReader unavailable(chain); - BOOST_CHECK_THROW(unavailable.Read(activation + 3), std::runtime_error); + BOOST_CHECK_THROW(unavailable.Read(activation + 30), std::runtime_error); } BOOST_AUTO_TEST_CASE(chainlock_construction_test) diff --git a/src/test/quorum_proofs_tests.cpp b/src/test/quorum_proofs_tests.cpp index 04d1e9455d45..9d49c3657993 100644 --- a/src/test/quorum_proofs_tests.cpp +++ b/src/test/quorum_proofs_tests.cpp @@ -276,7 +276,7 @@ BOOST_FIXTURE_TEST_CASE(generation_retries_retired_checkpoint_signer, QuorumProo const auto first = reader.Find(checkpoint_height + 1, chain.Height()); BOOST_REQUIRE(first); BOOST_CHECK_EQUAL(first->clsig.getHeight(), checkpoint_height + 1); - BOOST_CHECK(!builder.Build(checkpoint, first->clsig)); + BOOST_CHECK(!builder.Build(checkpoint, first->Signed())); const auto result = Generate(checkpoint_height + 1); const auto proof = llmq::QuorumProofChain::Decode(ParseHex(result["proof_hex"].get_str())); @@ -311,7 +311,7 @@ BOOST_FIXTURE_TEST_CASE(consensus_valid_envelopes_are_provable, QuorumProofGener *m_node.chainman, reader); const auto entry = reader.Find(handoff_height + llmq::SIGN_HEIGHT_OFFSET, chain.Height()); BOOST_REQUIRE(entry); - const auto proof = builder.Build(checkpoint, entry->clsig); + const auto proof = builder.Build(checkpoint, entry->Signed()); BOOST_REQUIRE(proof); BOOST_REQUIRE_EQUAL(proof->links.size(), 1U); From c9a20b7cf2b98bd9e1df2bc6f4df2f7a7ab8a603 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 22 Sep 2026 18:22:43 -0500 Subject: [PATCH 2/5] perf(llmq): memoize mining transactions, payload parsing and signer selection After the carrier cache, a warm proof spent most of its time decoding the same BLS points again: every quorum mining block was re-read and every commitment in it deserialized to find one transaction, the constructed proof was verified three times with each mining and coinbase payload re-parsed, and signing-quorum selection re-read commitments from the evo database. Memoize each quorum's mining transaction and Merkle path by mining block hash, and match commitments on their raw type and quorum-hash prefix before deserializing. Memoize mining and coinbase payload parsing by transaction bytes inside proof verification. Store signing selection as height to commitment hash plus a deduplicated commitment table, and fill it outside cs_main. Every cache is keyed by content or exact block history and bounded. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/llmq/quorumproofs.cpp | 156 +++++++++++++++++++++++++++++--------- 1 file changed, 121 insertions(+), 35 deletions(-) diff --git a/src/llmq/quorumproofs.cpp b/src/llmq/quorumproofs.cpp index 23ba74da8837..7d4ea625f268 100644 --- a/src/llmq/quorumproofs.cpp +++ b/src/llmq/quorumproofs.cpp @@ -162,14 +162,63 @@ static CTransactionRef ParseTransaction(const std::vector& bytes) return MakeTransactionRef(tx); } +/** The fields Verify needs from a mined commitment transaction. Parsing the + * payload decodes the commitment's BLS key and signatures; the result is a pure + * function of the transaction bytes, so it is memoized across verifications. */ +struct MiningPayload { + bool envelope_ok{false}; + bool payload_ok{false}; + uint16_t version{0}; + uint32_t height{0}; + std::vector commitment; +}; + +static MiningPayload ParseMiningPayload(const std::vector& bytes) +{ + const auto key = Hash(bytes); + static ProofCache cache; + MiningPayload result; + if (cache.Get(key, result)) return result; + const auto tx = ParseTransaction(bytes); + result.envelope_ok = tx->IsSpecialTxVersion() && tx->nType == TRANSACTION_QUORUM_COMMITMENT; + if (result.envelope_ok) { + if (auto payload = GetTxPayload(*tx)) { + result.payload_ok = true; + result.version = payload->nVersion; + result.height = payload->nHeight; + result.commitment = ConsensusBytes(payload->commitment); + } + } + cache.Insert(key, result); + return result; +} + +/** Coinbase envelope and payload, memoized by transaction bytes for the same reason. */ +struct CoinbasePayload { + bool envelope_ok{false}; + std::optional payload; +}; + +static CoinbasePayload ParseCoinbasePayload(const std::vector& bytes) +{ + const auto key = Hash(bytes); + static ProofCache cache; + CoinbasePayload result; + if (cache.Get(key, result)) return result; + const auto tx = ParseTransaction(bytes); + result.envelope_ok = tx->IsCoinBase() && tx->IsSpecialTxVersion() && tx->nType == TRANSACTION_COINBASE && + !tx->vin[0].scriptSig.empty() && tx->vin[0].scriptSig.size() <= 100 && !tx->vout.empty(); + if (result.envelope_ok) result.payload = GetTxPayload(*tx); + cache.Insert(key, result); + return result; +} + static CCbTx Coinbase(const ProofTransaction& proof, const ProofCertificate& cert) { Require(proof.path.index == 0 && proof.Verify(cert.header), "coinbase inclusion"); - const auto tx = ParseTransaction(proof.transaction); - Require(tx->IsCoinBase() && tx->IsSpecialTxVersion() && tx->nType == TRANSACTION_COINBASE && - !tx->vin[0].scriptSig.empty() && tx->vin[0].scriptSig.size() <= 100 && !tx->vout.empty(), - "coinbase envelope"); - auto payload = GetTxPayload(*tx); + const auto parsed = ParseCoinbasePayload(proof.transaction); + Require(parsed.envelope_ok, "coinbase envelope"); + const auto& payload = parsed.payload; Require(payload && payload->nVersion >= CCbTx::Version::CLSIG_AND_BALANCE && payload->nHeight == int64_t(cert.height) && payload->bestCLHeightDiff < cert.height && !payload->merkleRootQuorums.IsNull(), "coinbase payload"); @@ -324,12 +373,12 @@ ProofState QuorumProofChain::Verify(const ProofState& trusted) const descendant = &*it; } Require(link.mining.path.index != 0 && link.mining.Verify(*descendant), "mining inclusion"); - auto tx = ParseTransaction(link.mining.transaction); - Require(tx->IsSpecialTxVersion() && tx->nType == TRANSACTION_QUORUM_COMMITMENT, - "quorum transaction envelope"); - auto payload = GetTxPayload(*tx); - Require(payload && payload->nVersion == 1 && payload->nHeight == link.certificate.height - link.ancestors.size(), "quorum mining height"); - auto next = ParseCommitment(ConsensusBytes(payload->commitment), kind); + const auto mining = ParseMiningPayload(link.mining.transaction); + Require(mining.envelope_ok, "quorum transaction envelope"); + Require(mining.payload_ok && mining.version == 1 && + mining.height == link.certificate.height - link.ancestors.size(), + "quorum mining height"); + auto next = ParseCommitment(mining.commitment, kind); Require(next.quorumHash != signer.quorumHash, "redundant signer"); signer = std::move(next); height = link.certificate.height; @@ -430,16 +479,29 @@ std::optional QuorumProofBuilder::DetermineChainlockSigningCom // This hash commits to the entire selection history, including its branch. // The type and request height fix the remaining signing-selection inputs. const auto key = (CHashWriter(SER_GETHASH, 0) << start->GetBlockHash() << params->type << height).GetHash(); - static ProofCache cache; + // Two levels: every candidate height needs an entry, but thousands of heights + // share a few dozen commitments. Heights map to a commitment's content hash, + // which cannot alias across branches, and each commitment is stored once. + // A year of testnet (hourly quorums) is ~80k candidate heights. + static ProofCache selected; + static ProofCache commitments; + uint256 commitment_hash; CFinalCommitment cached; - if (cache.Get(key, cached)) return cached; - LOCK(cs_main); - // The commitment database follows the active chain. Do not populate a - // snapshot's cache from another branch if a reorg raced this request. - Require(m_chainman.ActiveChain().Contains(start), "proof chain changed during construction"); - auto result = SelectCommitmentForSigning(*params, m_chain, m_qman, chainlock::GenSigRequestId(height), height, - SIGN_HEIGHT_OFFSET); - if (result) cache.Insert(key, *result); + if (selected.Get(key, commitment_hash) && commitments.Get(commitment_hash, cached)) return cached; + std::optional result; + { + LOCK(cs_main); + // The commitment database follows the active chain. Do not populate a + // snapshot's cache from another branch if a reorg raced this request. + Require(m_chainman.ActiveChain().Contains(start), "proof chain changed during construction"); + result = SelectCommitmentForSigning(*params, m_chain, m_qman, chainlock::GenSigRequestId(height), height, + SIGN_HEIGHT_OFFSET); + } + if (result) { + commitment_hash = ::SerializeHash(*result); + commitments.Insert(commitment_hash, *result); + selected.Insert(key, commitment_hash); + } return result; } @@ -464,6 +526,42 @@ static const CBlockIndex* MinedCommitmentBlock(const CQuorumBlockProcessor& proc return mined; } +/** The mining transaction of one quorum commitment, with its Merkle path. + * Every proof whose route passes through a quorum needs this exact result; it + * depends only on the mining block's contents, so it is memoized by block hash. + * A miss reads the block and scans its commitments by type and quorum hash + * before fully deserializing any payload (a mining block holds dozens of + * commitments, each carrying BLS points that are costly to decode). */ +static ProofTransaction MiningTransaction(const CBlockIndex* mined, Consensus::LLMQType kind, const uint256& quorum) +{ + const auto key = (CHashWriter(SER_GETHASH, 0) << mined->GetBlockHash() << kind << quorum).GetHash(); + static ProofCache cache; + ProofTransaction cached; + if (cache.Get(key, cached)) return cached; + CBlock block; + Require(node::ReadBlockFromDisk(block, mined, Params().GetConsensus()), "mining block unavailable"); + for (size_t i = 1; i < block.vtx.size(); ++i) { + const auto& tx = *block.vtx[i]; + if (tx.nType != TRANSACTION_QUORUM_COMMITMENT) continue; + // CFinalCommitmentTxPayload: nVersion u16, nHeight u32, then the commitment, + // which starts with nVersion u16, llmqType u8, quorumHash. Compare the + // prefix first; only a candidate match pays for full deserialization. + const auto& raw = tx.vExtraPayload; + if (raw.size() < 2 + 4 + 2 + 1 + 32 || raw[2 + 4 + 2] != uint8_t(kind) || + !std::equal(quorum.begin(), quorum.end(), raw.begin() + 2 + 4 + 2 + 1)) { + continue; + } + const auto payload = GetTxPayload(tx); + if (payload && !payload->commitment.IsNull() && payload->commitment.llmqType == kind && + payload->commitment.quorumHash == quorum) { + auto proof = ProofTransaction::Build(block, i); + cache.Insert(key, proof); + return proof; + } + } + throw std::runtime_error("mining transaction unavailable"); +} + std::optional QuorumProofBuilder::Build(const CBlockIndex* checkpoint, const chainlock::ChainLockSig& target) const { @@ -498,19 +596,7 @@ std::optional QuorumProofBuilder::Build(const CBlockIndex* che const auto* mined = MinedCommitmentBlock(m_quorum_block_processor, chain, blocks, kind, needed->quorumHash); Require(mined != nullptr, "mining block unavailable"); if (mined->nHeight <= checkpoint->nHeight) return std::nullopt; - CBlock block; - Require(node::ReadBlockFromDisk(block, mined, Params().GetConsensus()), "mining block unavailable"); - std::optional mining; - for (size_t i = 1; i < block.vtx.size(); ++i) { - if (block.vtx[i]->nType != TRANSACTION_QUORUM_COMMITMENT) continue; - const auto payload = GetTxPayload(*block.vtx[i]); - if (payload && !payload->commitment.IsNull() && payload->commitment.llmqType == kind && - payload->commitment.quorumHash == needed->quorumHash) { - mining = ProofTransaction::Build(block, i); - break; - } - } - Require(mining.has_value(), "mining transaction unavailable"); + const auto mining = MiningTransaction(mined, kind, needed->quorumHash); std::optional best; std::optional predecessor; double bestScore = std::numeric_limits::infinity(); @@ -529,12 +615,12 @@ std::optional QuorumProofBuilder::Build(const CBlockIndex* che if (!previous || previous->nHeight >= mined->nHeight || (previous->nHeight <= checkpoint->nHeight && !seedHashes.count(signer->quorumHash))) continue; auto cert = certificate(*entry); if (!cert.Verify(*signer, checkpoint->nHeight, kind)) continue; - const auto cost = 180 + 4 + mining->transaction.size() + 9 + 32 * mining->path.siblings.size() + 2 + + const auto cost = 180 + 4 + mining.transaction.size() + 9 + 32 * mining.path.siblings.size() + 2 + 80 * (height - mined->nHeight); const double score = double(cost) / (mined->nHeight - std::max(checkpoint->nHeight, previous->nHeight)); if (score >= bestScore) continue; bestScore = score; - best = ProofLink{cert, *mining, {}}; + best = ProofLink{cert, mining, {}}; predecessor = std::move(signer); } Require(best && predecessor, "no B-only route within proof budget"); From e8904c2f1767b502524de1904e3de82cc64bbcf2 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 22 Sep 2026 19:07:51 -0500 Subject: [PATCH 3/5] fix(llmq): make proof caches independent of process-wide BLS state Store each carrier's ChainLock signature as the raw bytes from its coinbase payload instead of re-encoding a decoded CCbTx, whose signature depends on the process-wide BLS scheme flag at decode time; a transient legacy flag could otherwise have cached a carrier as having no ChainLock for the life of the process. Decoded signatures are cached per scheme value. Check each carrier's Merkle root before caching its result. Return the reader's height, block hash and signature bytes as separate fields so no caller can use an undecoded ChainLockSig. Do not cache oversized commitments from caller-supplied proofs, and keep only the coinbase fields proof verification reads. Add tests for decoy commitments mined ahead of the real one, a lookup under the opposite BLS scheme, and CChain::CopyFrom followed by SetTip to higher, lower and forked tips. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/chainlock/clsig.cpp | 73 +++++++++++++++++++++---------- src/chainlock/clsig.h | 20 ++++++--- src/llmq/quorumproofs.cpp | 59 ++++++++++++++++--------- src/rpc/quorums.cpp | 2 +- src/test/llmq_chainlock_tests.cpp | 21 +++++++-- src/test/quorum_proofs_tests.cpp | 24 +++++++++- src/test/skiplist_tests.cpp | 42 ++++++++++++++++++ 7 files changed, 186 insertions(+), 55 deletions(-) diff --git a/src/chainlock/clsig.cpp b/src/chainlock/clsig.cpp index 4174f67eb7a6..7222306f23a9 100644 --- a/src/chainlock/clsig.cpp +++ b/src/chainlock/clsig.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -14,6 +15,7 @@ #include #include #include +#include #include #include @@ -23,7 +25,7 @@ namespace chainlock { static constexpr std::string_view CLSIG_REQUESTID_PREFIX{"clsig"}; -static constexpr size_t MAX_HISTORICAL_CARRIER_READS{16384}; +static constexpr size_t MAX_HISTORICAL_CARRIER_LOOKUPS{16384}; namespace { /** What one validated coinbase says about ChainLocks: the embedded signature @@ -43,7 +45,7 @@ struct CarrierEntry { class CarrierCache { Mutex m_mutex; - // 32 KiB * ~120 B ≈ 4 MiB; covers every probe of years of mainnet history. + // 32768 entries (≈ 6–7 MiB with map overhead); covers every probe of years of mainnet history. Uint256LruHashMap m_entries GUARDED_BY(m_mutex); public: @@ -70,9 +72,10 @@ CarrierCache& GetCarrierCache() return cache; } -/** Decoded ChainLock signatures, keyed by their serialized bytes. Decoding a G2 - * point (decompression plus subgroup check) costs far more than the lookups - * around it, and proofs sharing history ask for the same signatures each time. */ +/** Decoded ChainLock signatures, keyed by their serialized bytes and the BLS + * scheme they were decoded under. Decoding a G2 point (decompression plus + * subgroup check) costs far more than the lookups around it, and proofs sharing + * history ask for the same signatures each time. */ class DecodedSignatureCache { Mutex m_mutex; @@ -82,13 +85,18 @@ class DecodedSignatureCache public: CBLSSignature Decode(const std::array& bytes) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) { - const uint256 key = Hash(bytes); + // 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(); + const uint256 key = (CHashWriter(SER_GETHASH, 0) << Span{bytes} << legacy).GetHash(); CBLSSignature sig; { LOCK(m_mutex); if (m_entries.get(key, sig)) return sig; } - sig.SetBytes(bytes, false); + CDataStream in(Span{bytes}, SER_NETWORK, PROTOCOL_VERSION); + sig.Unserialize(in, legacy); if (sig.IsValid()) { LOCK(m_mutex); m_entries.insert(key, sig); @@ -117,6 +125,24 @@ void ClearCoinbaseChainLockCacheForTesting() namespace { +/** Offset of bestCLHeightDiff in a CLSIG_AND_BALANCE coinbase payload: + * nVersion u16, nHeight i32, merkleRootMNList, merkleRootQuorums. */ +constexpr size_t CBTX_CLSIG_OFFSET{2 + 4 + 32 + 32}; + +/** The raw bestCLSignature bytes of a validated CLSIG_AND_BALANCE payload. + * Read from the wire rather than from the decoded CCbTx, whose signature + * depends on the process-wide BLS scheme flag at decode time; a cached value + * must be a function of the block alone. */ +std::array RawChainLockSignature(const std::vector& payload) +{ + CDataStream in(Span{payload}.subspan(CBTX_CLSIG_OFFSET), SER_NETWORK, PROTOCOL_VERSION); + uint64_t height_diff; + in >> COMPACTSIZE(height_diff); + std::array signature; + in.read(AsWritableBytes(Span{signature})); + return signature; +} + /** Reads and validates one carrier coinbase. Throws on missing or invalid data. */ CarrierEntry LoadCarrier(const CBlockIndex* carrier, int carrier_height) { @@ -124,6 +150,11 @@ CarrierEntry LoadCarrier(const CBlockIndex* carrier, int carrier_height) if (!node::ReadBlockFromDisk(block, carrier, Params().GetConsensus()) || block.vtx.empty()) { throw std::runtime_error("Historical ChainLock block data unavailable"); } + // ReadBlockFromDisk checks the header only. The result below is kept for the + // life of the process, so make sure the coinbase is the one the header commits to. + if (BlockMerkleRoot(block) != block.hashMerkleRoot) { + throw std::runtime_error("Historical ChainLock block data corrupted"); + } const auto cb = GetTxPayload(*block.vtx[0]); if (!block.vtx[0]->IsCoinBase() || block.vtx[0]->nType != TRANSACTION_COINBASE || !cb || cb->nVersion < CCbTx::Version::CLSIG_AND_BALANCE || cb->nHeight != carrier_height) { @@ -132,14 +163,13 @@ CarrierEntry LoadCarrier(const CBlockIndex* carrier, int carrier_height) if (cb->bestCLHeightDiff >= uint32_t(carrier_height)) { throw std::runtime_error("Invalid historical coinbase ChainLock height"); } - // Same conditions as GetNonNullCoinbaseChainlock, which would decode the - // payload (and its BLS signature) a second time; everything else is checked above. + // Consensus writes a null certificate as all-zero bytes. Anything else is a + // certificate; an encoding that does not decode surfaces later from Signed() + // instead of being remembered as absent. CarrierEntry entry; - if (cb->bestCLSignature.IsValid()) { - entry.has_chainlock = true; - entry.height_diff = cb->bestCLHeightDiff; - entry.signature = cb->bestCLSignature.ToBytes(false); - } + entry.signature = RawChainLockSignature(block.vtx[0]->vExtraPayload); + entry.has_chainlock = std::ranges::any_of(entry.signature, [](uint8_t b) { return b != 0; }); + entry.height_diff = cb->bestCLHeightDiff; return entry; } } // namespace @@ -155,8 +185,7 @@ std::optional CoinbaseChainLockReader::Read(int carrier_heigh if (!carrier || carrier_height < Params().GetConsensus().V20Height) return std::nullopt; if (const auto it = m_cache.find(carrier_height); it != m_cache.end()) return it->second; if (ShutdownRequested()) throw std::runtime_error("ChainLock lookup interrupted"); - if (m_cache.size() >= MAX_HISTORICAL_CARRIER_READS) - throw std::runtime_error("ChainLock disk-read budget exhausted"); + if (m_cache.size() >= MAX_HISTORICAL_CARRIER_LOOKUPS) throw std::runtime_error("ChainLock lookup budget exhausted"); const uint256 carrier_hash = carrier->GetBlockHash(); CarrierEntry entry; if (!GetCarrierCache().Get(carrier_hash, entry)) { @@ -172,18 +201,18 @@ std::optional CoinbaseChainLockReader::Read(int carrier_heigh // Deferred decode: binary search only compares heights, so most carriers // never need their signature. Callers that build a certificate decode it. return m_cache - .emplace(carrier_height, - CoinbaseChainLock{ChainLockSig{height, signed_block->GetBlockHash(), {}}, carrier, entry.signature}) + .emplace(carrier_height, CoinbaseChainLock{height, signed_block->GetBlockHash(), carrier, entry.signature}) .first->second; } ChainLockSig CoinbaseChainLock::Signed() const { // Validated by consensus when the carrier was connected, so a failure here - // means corrupt block data; surface it rather than proving with it. + // means corrupt block data (or a transient BLS scheme flag); surface it for + // this request rather than proving with it. const CBLSSignature sig = GetDecodedSignatureCache().Decode(signature_bytes); if (!sig.IsValid()) throw std::runtime_error("Invalid historical ChainLock signature"); - return ChainLockSig{clsig.getHeight(), clsig.getBlockHash(), sig}; + return ChainLockSig{height, block_hash, sig}; } std::optional CoinbaseChainLockReader::Find(int minimum_height, int maximum_height) @@ -198,7 +227,7 @@ std::optional CoinbaseChainLockReader::Find(int minimum_heigh // forbids null signatures after the first certificate on a validated chain. auto below_minimum = [&](int height) { const auto entry = Read(height); - return !entry || entry->clsig.getHeight() < minimum_height; + return !entry || entry->height < minimum_height; }; int low = first_carrier; int high = first_carrier; @@ -218,7 +247,7 @@ std::optional CoinbaseChainLockReader::Find(int minimum_heigh } } const auto entry = Read(low); - return entry && entry->clsig.getHeight() <= maximum_height ? entry : std::nullopt; + return entry && entry->height <= maximum_height ? entry : std::nullopt; } uint256 GenSigRequestId(const int32_t nHeight) diff --git a/src/chainlock/clsig.h b/src/chainlock/clsig.h index a7571868726c..e338da84a090 100644 --- a/src/chainlock/clsig.h +++ b/src/chainlock/clsig.h @@ -5,7 +5,9 @@ #ifndef BITCOIN_CHAINLOCK_CLSIG_H #define BITCOIN_CHAINLOCK_CLSIG_H +#include #include +#include #include #include @@ -26,19 +28,23 @@ enum class VerifyRecSigStatus : uint8_t; } // namespace llmq namespace chainlock { +/** A ChainLock certificate carried by a historical coinbase. The signature is + * kept as bytes: decoding a BLS point dominates the cost of reading a carrier, + * and binary search only needs heights. Signed() decodes it (and throws if the + * bytes are not a valid signature). */ struct CoinbaseChainLock { - ChainLockSig clsig; + int32_t height{-1}; + uint256 block_hash; const CBlockIndex* carrier{nullptr}; - /** Serialized signature as it appears in the carrier coinbase. `clsig` holds - * height and block hash only; call Signed() for a copy with the decoded - * signature. Decoding a BLS point is the costly part of reading a carrier. */ - std::array signature_bytes{}; + std::array signature_bytes{}; ChainLockSig Signed() const; }; /** Reads historical signatures from a fixed, validated chain view. - * Cache lifetime is one request; missing block data throws instead of implying - * that a certificate does not exist. Disk reads do not hold cs_main. + * What each carrier block says is memoized process-wide by block hash, so a + * carrier read once stays available even if its block data is later removed; + * data that was never read and is missing throws instead of implying that a + * certificate does not exist. Disk reads do not hold cs_main. */ class CoinbaseChainLockReader { diff --git a/src/llmq/quorumproofs.cpp b/src/llmq/quorumproofs.cpp index 7d4ea625f268..57d7e204198c 100644 --- a/src/llmq/quorumproofs.cpp +++ b/src/llmq/quorumproofs.cpp @@ -183,20 +183,30 @@ static MiningPayload ParseMiningPayload(const std::vector& bytes) result.envelope_ok = tx->IsSpecialTxVersion() && tx->nType == TRANSACTION_QUORUM_COMMITMENT; if (result.envelope_ok) { if (auto payload = GetTxPayload(*tx)) { - result.payload_ok = true; - result.version = payload->nVersion; - result.height = payload->nHeight; - result.commitment = ConsensusBytes(payload->commitment); + auto commitment = ConsensusBytes(payload->commitment); + // ParseCommitment rejects these anyway; do not let a caller-supplied + // proof park up to ~100 KB per entry in this cache first. + if (commitment.size() <= 1024) { + result.payload_ok = true; + result.version = payload->nVersion; + result.height = payload->nHeight; + result.commitment = std::move(commitment); + } } } cache.Insert(key, result); return result; } -/** Coinbase envelope and payload, memoized by transaction bytes for the same reason. */ +/** Coinbase envelope and the payload fields Verify reads, memoized by transaction bytes. */ struct CoinbasePayload { bool envelope_ok{false}; - std::optional payload; + bool payload_ok{false}; + CCbTx::Version version{CCbTx::Version::INVALID}; + int32_t height{0}; + uint32_t best_cl_height_diff{0}; + uint256 merkle_root_mn_list; + uint256 merkle_root_quorums; }; static CoinbasePayload ParseCoinbasePayload(const std::vector& bytes) @@ -208,21 +218,30 @@ static CoinbasePayload ParseCoinbasePayload(const std::vector& by const auto tx = ParseTransaction(bytes); result.envelope_ok = tx->IsCoinBase() && tx->IsSpecialTxVersion() && tx->nType == TRANSACTION_COINBASE && !tx->vin[0].scriptSig.empty() && tx->vin[0].scriptSig.size() <= 100 && !tx->vout.empty(); - if (result.envelope_ok) result.payload = GetTxPayload(*tx); + if (result.envelope_ok) { + if (const auto payload = GetTxPayload(*tx)) { + result.payload_ok = true; + result.version = payload->nVersion; + result.height = payload->nHeight; + result.best_cl_height_diff = payload->bestCLHeightDiff; + result.merkle_root_mn_list = payload->merkleRootMNList; + result.merkle_root_quorums = payload->merkleRootQuorums; + } + } cache.Insert(key, result); return result; } -static CCbTx Coinbase(const ProofTransaction& proof, const ProofCertificate& cert) +static CoinbasePayload Coinbase(const ProofTransaction& proof, const ProofCertificate& cert) { Require(proof.path.index == 0 && proof.Verify(cert.header), "coinbase inclusion"); - const auto parsed = ParseCoinbasePayload(proof.transaction); - Require(parsed.envelope_ok, "coinbase envelope"); - const auto& payload = parsed.payload; - Require(payload && payload->nVersion >= CCbTx::Version::CLSIG_AND_BALANCE && - payload->nHeight == int64_t(cert.height) && payload->bestCLHeightDiff < cert.height && - !payload->merkleRootQuorums.IsNull(), "coinbase payload"); - return *payload; + const auto payload = ParseCoinbasePayload(proof.transaction); + Require(payload.envelope_ok, "coinbase envelope"); + Require(payload.payload_ok && payload.version >= CCbTx::Version::CLSIG_AND_BALANCE && + payload.height == int64_t(cert.height) && payload.best_cl_height_diff < cert.height && + !payload.merkle_root_quorums.IsNull(), + "coinbase payload"); + return payload; } bool ProofMerklePath::Verify(uint256 leaf, const uint256& root) const @@ -385,7 +404,7 @@ ProofState QuorumProofChain::Verify(const ProofState& trusted) const } Require(target.Verify(signer, height, kind), "final ChainLock signature/height"); auto cb = Coinbase(coinbase, target); - return {anchor.network, target.height, target.header.GetHash(), cb.merkleRootMNList, cb.merkleRootQuorums}; + return {anchor.network, target.height, target.header.GetHash(), cb.merkle_root_mn_list, cb.merkle_root_quorums}; } std::vector EncodeBootstrap(const QuorumProofChain& proof, const std::vector& records) @@ -467,7 +486,8 @@ ProofState QuorumProofBuilder::StateAt(const CBlockIndex* index) Require(node::ReadBlockFromDisk(block, index, Params().GetConsensus()) && !block.vtx.empty(), "historical block unavailable"); ProofCertificate cert{uint32_t(index->nHeight), block.GetBlockHeader(), {}}; auto cb = Coinbase(ProofTransaction::Build(block, 0), cert); - return {uint8_t(network == "main" ? 0 : 1), uint32_t(index->nHeight), index->GetBlockHash(), cb.merkleRootMNList, cb.merkleRootQuorums}; + return {uint8_t(network == "main" ? 0 : 1), uint32_t(index->nHeight), index->GetBlockHash(), cb.merkle_root_mn_list, + cb.merkle_root_quorums}; } std::optional QuorumProofBuilder::DetermineChainlockSigningCommitment(int32_t height) const @@ -572,8 +592,7 @@ std::optional QuorumProofBuilder::Build(const CBlockIndex* che "checkpoint/target not on chain"); Require(targetIndex->GetBlockHash() == target.getBlockHash(), "target ChainLock block mismatch"); auto certificate = [&](const chainlock::CoinbaseChainLock& entry) { - return ProofCertificate{uint32_t(entry.clsig.getHeight()), chain[entry.clsig.getHeight()]->GetBlockHeader(), - entry.Signed().getSig()}; + return ProofCertificate{uint32_t(entry.height), chain[entry.height]->GetBlockHeader(), entry.Signed().getSig()}; }; QuorumProofChain proof; proof.anchor = StateAt(checkpoint); @@ -607,7 +626,7 @@ std::optional QuorumProofBuilder::Build(const CBlockIndex* che if (height > mined->nHeight + 8 && best) break; const auto entry = m_chainlocks.Find(height, maximum); if (!entry) break; - height = entry->clsig.getHeight(); + height = entry->height; if (height > mined->nHeight + 8 && best) break; auto signer = DetermineChainlockSigningCommitment(height); if (!signer || signer->quorumHash == needed->quorumHash) continue; diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index e380b41b52da..a9326a00c4ec 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -1462,7 +1462,7 @@ static RPCHelpMan getchainlockbyheight() } UniValue result(UniValue::VOBJ); result.pushKV("height", height); - result.pushKV("blockhash", entry->clsig.getBlockHash().ToString()); + result.pushKV("blockhash", entry->block_hash.ToString()); result.pushKV("signature", entry->Signed().getSig().ToString()); result.pushKV("cbtx_height", entry->carrier->nHeight); return result; diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index 7e68769180ea..5c39aafca578 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -119,8 +120,8 @@ BOOST_FIXTURE_TEST_CASE(historical_coinbase_lookup_from_disk, RegTestingSetup) continue; } BOOST_REQUIRE(entry); - BOOST_CHECK_EQUAL(entry->clsig.getHeight(), expected); - BOOST_CHECK(entry->clsig.getBlockHash() == chain[expected]->GetBlockHash()); + BOOST_CHECK_EQUAL(entry->height, expected); + BOOST_CHECK(entry->block_hash == chain[expected]->GetBlockHash()); BOOST_CHECK(entry->Signed().getSig() == signature); const int carrier = expected == activation + 1 ? activation + 20 : expected == activation + 65 ? activation + 80 @@ -134,7 +135,7 @@ BOOST_FIXTURE_TEST_CASE(historical_coinbase_lookup_from_disk, RegTestingSetup) const auto late = long_gap.Find(activation + 100, activation + 16999); BOOST_REQUIRE(late); BOOST_CHECK_EQUAL(late->carrier->nHeight, activation + 17000); - BOOST_CHECK_EQUAL(late->clsig.getHeight(), activation + 16999); + BOOST_CHECK_EQUAL(late->height, activation + 16999); BOOST_CHECK(!long_gap.Find(activation + 100, activation + 16998)); // A new request on a shorter chain must not reuse the old request's cache. CChain shorter; @@ -143,6 +144,20 @@ BOOST_FIXTURE_TEST_CASE(historical_coinbase_lookup_from_disk, RegTestingSetup) BOOST_CHECK(!after_disconnect.Find(activation + 65, activation + 65)); BOOST_REQUIRE(after_disconnect.Find(activation + 1, activation + 1)); + // What a carrier says must not depend on the process-wide BLS scheme flag at + // the time it is first read, because the answer is kept for the process. + chainlock::ClearCoinbaseChainLockCacheForTesting(); + const bool scheme = bls::bls_legacy_scheme.load(); + bls::bls_legacy_scheme.store(!scheme); + const auto under_other = chainlock::CoinbaseChainLockReader(chain).Read(activation + 40); + bls::bls_legacy_scheme.store(scheme); + BOOST_REQUIRE(under_other); + BOOST_CHECK_EQUAL(under_other->height, activation + 1); + const auto after = chainlock::CoinbaseChainLockReader(chain).Read(activation + 40); + BOOST_REQUIRE(after); + BOOST_CHECK(after->Signed().getSig() == signature); + BOOST_CHECK_EQUAL(after->height, activation + 1); + // A carrier's certificate is a fact about its block hash, so once read it is // served from the process-wide memo even if the block data goes away... BOOST_REQUIRE(chainlock::CoinbaseChainLockReader(chain).Read(activation + 30)); diff --git a/src/test/quorum_proofs_tests.cpp b/src/test/quorum_proofs_tests.cpp index 9d49c3657993..a4f733ca6d9b 100644 --- a/src/test/quorum_proofs_tests.cpp +++ b/src/test/quorum_proofs_tests.cpp @@ -217,6 +217,24 @@ struct QuorumProofGenerationSetup : TestingSetup { llmq::CFinalCommitmentTxPayload qc; qc.nHeight = height; qc.commitment = commitments.back(); + if (mine_nonstandard_handoff && height == handoff_height) { + // Decoys ahead of the real commitment: the same quorum hash under + // another type, and another quorum hash under the same type. The + // prover must still pick exactly the real one. + for (int decoy = 0; decoy < 2; ++decoy) { + CMutableTransaction other; + other.nVersion = 3; + other.nType = TRANSACTION_QUORUM_COMMITMENT; + auto payload = qc; + if (decoy == 0) { + payload.commitment.llmqType = Consensus::LLMQType::LLMQ_400_60; + } else { + payload.commitment.quorumHash = uint256::ONE; + } + SetTxPayload(other, payload); + block.vtx.push_back(MakeTransactionRef(other)); + } + } SetTxPayload(tx, qc); block.vtx.push_back(MakeTransactionRef(tx)); } @@ -275,7 +293,7 @@ BOOST_FIXTURE_TEST_CASE(generation_retries_retired_checkpoint_signer, QuorumProo *m_node.chainman, reader); const auto first = reader.Find(checkpoint_height + 1, chain.Height()); BOOST_REQUIRE(first); - BOOST_CHECK_EQUAL(first->clsig.getHeight(), checkpoint_height + 1); + BOOST_CHECK_EQUAL(first->height, checkpoint_height + 1); BOOST_CHECK(!builder.Build(checkpoint, first->Signed())); const auto result = Generate(checkpoint_height + 1); @@ -318,13 +336,15 @@ BOOST_FIXTURE_TEST_CASE(consensus_valid_envelopes_are_provable, QuorumProofGener CDataStream stream(proof->links[0].mining.transaction, SER_NETWORK, PROTOCOL_VERSION); CMutableTransaction mining; stream >> mining; + // Two decoy commitments were mined ahead of it in the same block. + BOOST_CHECK_EQUAL(proof->links[0].mining.path.index, 3U); BOOST_CHECK_EQUAL(mining.nVersion, 4); BOOST_CHECK_EQUAL(mining.nLockTime, 1U); BOOST_CHECK(!mining.vin.empty()); BOOST_CHECK(!mining.vout.empty()); const auto state = proof->Verify(llmq::QuorumProofBuilder::StateAt(checkpoint)); - BOOST_CHECK_EQUAL(state.height, uint32_t(entry->clsig.getHeight())); + BOOST_CHECK_EQUAL(state.height, uint32_t(entry->height)); BOOST_CHECK(state.blockHash == proof->target.header.GetHash()); } BOOST_AUTO_TEST_CASE(real_testnet_wire_and_crypto) { diff --git a/src/test/skiplist_tests.cpp b/src/test/skiplist_tests.cpp index 050033e43a9e..0122c0069bcd 100644 --- a/src/test/skiplist_tests.cpp +++ b/src/test/skiplist_tests.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include #include #include #include @@ -43,6 +44,47 @@ BOOST_AUTO_TEST_CASE(skiplist_test) } } +BOOST_AUTO_TEST_CASE(copyfrom_then_settip_matches_fresh_chain) +{ + // Main chain 0..99 and a fork from height 60 to 90. + std::vector main(100), fork(31); + std::vector main_hashes(100), fork_hashes(31); + for (int i = 0; i < 100; ++i) { + main_hashes[i] = ArithToUint256(i); + main[i].phashBlock = &main_hashes[i]; + main[i].nHeight = i; + main[i].pprev = i ? &main[i - 1] : nullptr; + main[i].BuildSkip(); + } + for (int i = 0; i < 31; ++i) { + fork_hashes[i] = ArithToUint256(1000 + i); + fork[i].phashBlock = &fork_hashes[i]; + fork[i].nHeight = 60 + i; + fork[i].pprev = i ? &fork[i - 1] : &main[59]; + fork[i].BuildSkip(); + } + auto same = [](const CChain& a, const CChain& b) { + if (a.Height() != b.Height()) return false; + for (int h = 0; h <= a.Height(); ++h) { + if (a[h] != b[h]) return false; + } + return true; + }; + for (CBlockIndex* target : {&main[99], &main[40], &fork[30], &fork[5], &main[99], &main[0]}) { + CChain previous; + previous.SetTip(main[80]); + CChain extended; + extended.CopyFrom(previous); + extended.SetTip(*target); + CChain fresh; + fresh.SetTip(*target); + BOOST_CHECK(same(extended, fresh)); + // The source is left untouched. + BOOST_CHECK_EQUAL(previous.Height(), 80); + BOOST_CHECK(previous.Tip() == &main[80]); + } +} + BOOST_AUTO_TEST_CASE(getlocator_test) { // Build a main chain 100000 blocks long. From 1988be27a3c2dff86300756266424b65284afe98 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 22 Sep 2026 20:38:23 -0500 Subject: [PATCH 4/5] perf(rpc): reuse active quorum sets and verify the finished proof once Per-phase timing of a warm mainnet request from a release checkpoint showed about 60% of the remaining time in reading each block's active quorum set: 88 commitments read from the evo database with their BLS points decoded, once for the checkpoint and again for the requested quorum's record at the target. Cache each block's sorted, root-checked active set by block hash, and sort by precomputed hashes instead of re-hashing inside the comparator. The finished proof was verified three times per request. Build() already verifies it and matches the result against the target block, so the RPC now reuses that state for the bootstrap encoding and the target field. EncodeBootstrap gains an overload that takes an already-verified state. Cache the target block's masternode list and entry hashes for EvoNode records. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/llmq/quorumproofs.cpp | 63 ++++++++++++++++++++++++++++---- src/llmq/quorumproofs.h | 7 ++++ src/rpc/quorums.cpp | 53 +++++++++++++++++++++++---- src/test/quorum_proofs_tests.cpp | 35 ++++++++++++++++++ 4 files changed, 144 insertions(+), 14 deletions(-) diff --git a/src/llmq/quorumproofs.cpp b/src/llmq/quorumproofs.cpp index 57d7e204198c..9faff615dc4e 100644 --- a/src/llmq/quorumproofs.cpp +++ b/src/llmq/quorumproofs.cpp @@ -48,6 +48,12 @@ class ProofCache LOCK(m_mutex); m_entries.insert(key, value); } + + void Clear() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex) + { + LOCK(m_mutex); + m_entries.clear(); + } }; static void Require(bool condition, const char* message) @@ -408,9 +414,20 @@ ProofState QuorumProofChain::Verify(const ProofState& trusted) const } std::vector EncodeBootstrap(const QuorumProofChain& proof, const std::vector& records) +{ + return EncodeBootstrap(proof, proof.Verify(proof.anchor), records); +} + +std::vector EncodeBootstrap(const QuorumProofChain& proof, const ProofState& state, + const std::vector& records) { Require(!records.empty() && records.size() <= 16, "projection count"); - const auto state = proof.Verify(proof.anchor); + // No BLS work: this re-reads only the target coinbase the proof carries. + const auto cb = Coinbase(proof.coinbase, proof.target); + Require(state.network == proof.anchor.network && state.height == proof.target.height && + state.blockHash == proof.target.header.GetHash() && state.masternodeRoot == cb.merkle_root_mn_list && + state.quorumRoot == cb.merkle_root_quorums, + "state does not match proof target"); CDataStream out(SER_NETWORK, PROTOCOL_VERSION); WriteBlob(out, proof.Encode()); out << uint8_t(records.size()); @@ -458,7 +475,13 @@ ProofState ProofState::FromJson(const UniValue& value) std::vector QuorumProofBuilder::ActiveCommitments(const CBlockIndex* index) const { Require(index != nullptr, "missing block index"); + // The active set at a block is a function of that block's history, and it is + // checked against the block's own quorum root below before it is kept. Every + // proof from one checkpoint needs the same set, as does every quorum record at + // one target. Reading it decodes each commitment's BLS key and signatures. + static ProofCache, 64> sets; std::vector result; + if (sets.Get(index->GetBlockHash(), result)) return result; { LOCK(cs_main); for (const auto& [type, indexes] : m_quorum_block_processor.GetMinedAndActiveCommitmentsUntilBlock(index)) { @@ -469,25 +492,51 @@ std::vector QuorumProofBuilder::ActiveCommitments(const CBlock } } } - std::sort(result.begin(), result.end(), [](const auto& a, const auto& b) { return SerializeHash(a) < SerializeHash(b); }); + std::vector> order; + order.reserve(result.size()); + for (size_t i = 0; i < result.size(); ++i) + order.emplace_back(SerializeHash(result[i]), i); + std::sort(order.begin(), order.end()); + std::vector sorted; std::vector hashes; + sorted.reserve(result.size()); hashes.reserve(result.size()); - for (const auto& commitment : result) hashes.push_back(SerializeHash(commitment)); - Require(ComputeMerkleRoot(hashes) == StateAt(index).quorumRoot, "active quorum root mismatch"); - return result; + for (const auto& [hash, i] : order) { + sorted.push_back(std::move(result[i])); + hashes.push_back(hash); + } + bool mutated{false}; + Require(ComputeMerkleRoot(hashes, &mutated) == StateAt(index).quorumRoot && !mutated, + "active quorum root mismatch"); + sets.Insert(index->GetBlockHash(), sorted); + return sorted; } +static ProofCache& StateCache() +{ + static ProofCache cache; + return cache; +} + +void ClearProofStateCacheForTesting() { StateCache().Clear(); } + ProofState QuorumProofBuilder::StateAt(const CBlockIndex* index) { Require(index != nullptr && index->nHeight > 0, "invalid checkpoint block"); const auto network = Params().NetworkIDString(); Require(network == "main" || network == "test", "proofs support mainnet/testnet"); + // A block hash fixes the coinbase, and the network is fixed for the process. + // Every request reads the checkpoint's state and the target's more than once. + ProofState cached; + if (StateCache().Get(index->GetBlockHash(), cached)) return cached; CBlock block; Require(node::ReadBlockFromDisk(block, index, Params().GetConsensus()) && !block.vtx.empty(), "historical block unavailable"); ProofCertificate cert{uint32_t(index->nHeight), block.GetBlockHeader(), {}}; auto cb = Coinbase(ProofTransaction::Build(block, 0), cert); - return {uint8_t(network == "main" ? 0 : 1), uint32_t(index->nHeight), index->GetBlockHash(), cb.merkle_root_mn_list, - cb.merkle_root_quorums}; + const ProofState state{uint8_t(network == "main" ? 0 : 1), uint32_t(index->nHeight), index->GetBlockHash(), + cb.merkle_root_mn_list, cb.merkle_root_quorums}; + StateCache().Insert(index->GetBlockHash(), state); + return state; } std::optional QuorumProofBuilder::DetermineChainlockSigningCommitment(int32_t height) const diff --git a/src/llmq/quorumproofs.h b/src/llmq/quorumproofs.h index f68129a8d03d..af214c85d4e9 100644 --- a/src/llmq/quorumproofs.h +++ b/src/llmq/quorumproofs.h @@ -56,6 +56,13 @@ struct ProofProjection { ProofMerklePath path; }; std::vector EncodeBootstrap(const QuorumProofChain& proof, const std::vector& records); +/** As above, for a proof whose Verify(proof.anchor) result the caller already holds. */ +std::vector EncodeBootstrap(const QuorumProofChain& proof, const ProofState& verified, + const std::vector& records); +/** Checkpoint and target states are memoized by block hash; a state read once + * stays available even if the block's data is later removed. */ +void ClearProofStateCacheForTesting(); + class QuorumProofBuilder { const CQuorumBlockProcessor& m_quorum_block_processor; diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index a9326a00c4ec..b1d3164c649f 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #include #include @@ -41,6 +42,7 @@ #include #include +#include #include #include #include @@ -72,6 +74,42 @@ std::shared_ptr GetProofChainSnapshot(const ChainstateManager& cha } return g_proof_chain_snapshot; } + +/** The simplified masternode list at a block, with its entry hashes: the leaves of + * that block's masternode root. Every request that opens EvoNode records at the + * same target needs them, and hashing every entry costs more than the lookup. */ +struct MasternodeLeaves { + std::shared_ptr sml; + std::vector leaves; +}; + +std::mutex g_mn_leaves_mutex; +std::map g_mn_leaves; // keyed by block hash; a few recent targets + +MasternodeLeaves GetMasternodeLeaves(CDeterministicMNManager& dmnman, const CBlockIndex* target, + const uint256& masternode_root) +{ + const uint256 key = target->GetBlockHash(); + { + std::lock_guard lock(g_mn_leaves_mutex); + if (const auto it = g_mn_leaves.find(key); it != g_mn_leaves.end()) return it->second; + } + const auto list = WITH_LOCK(cs_main, return dmnman.GetListForBlock(target)); + MasternodeLeaves result{list.to_sml(), {}}; + result.leaves.reserve(result.sml->mnList.size()); + for (const auto& entry : result.sml->mnList) + result.leaves.push_back(entry->CalcHash()); + // Keep only a list that is the one the block commits to. + bool mutated{false}; + if (ComputeMerkleRoot(result.leaves, &mutated) != masternode_root || mutated) { + throw std::runtime_error("Masternode list does not match the target block"); + } + std::lock_guard lock(g_mn_leaves_mutex); + // Only recent targets are requested; drop everything rather than track recency. + if (g_mn_leaves.size() >= 8) g_mn_leaves.clear(); + g_mn_leaves.emplace(key, result); + return result; +} } // namespace static RPCHelpMan quorum_list() @@ -1566,6 +1604,9 @@ static RPCHelpMan getquorumproofchain() next = int64_t(target_signature.getHeight()) + 1; } if (!proof) throw std::runtime_error("No bridge to snapshot within search budget"); + // Build() already verified the proof and matched the result against the + // chain; the target is exactly the state at the target block. + const auto state = llmq::QuorumProofBuilder::StateAt(target); std::vector records; if (!quorumText.empty()) { const auto hash = uint256S(quorumText); @@ -1587,11 +1628,8 @@ static RPCHelpMan getquorumproofchain() if (!found) throw std::runtime_error("Requested quorum is not in the target root"); } if (nodeCount > 0) { - const auto list = WITH_LOCK(cs_main, return CHECK_NONFATAL(node.dmnman)->GetListForBlock(target)); - const auto sml = list.to_sml(); - std::vector leaves; - for (const auto& entry : sml->mnList) - leaves.push_back(entry->CalcHash()); + const auto [sml, leaves] = GetMasternodeLeaves(*CHECK_NONFATAL(node.dmnman), target, + state.masternodeRoot); int included = 0; for (size_t i = 0; i < sml->mnList.size() && included < nodeCount; ++i) { const auto& entry = *sml->mnList[i]; @@ -1611,8 +1649,9 @@ static RPCHelpMan getquorumproofchain() } UniValue result(UniValue::VOBJ); result.pushKV("proof_hex", HexStr(proof->Encode())); - result.pushKV("bootstrap_hex", records.empty() ? "" : HexStr(llmq::EncodeBootstrap(*proof, records))); - result.pushKV("target", proof->Verify(proof->anchor).ToJson()); + result.pushKV("bootstrap_hex", + records.empty() ? "" : HexStr(llmq::EncodeBootstrap(*proof, state, records))); + result.pushKV("target", state.ToJson()); return result; } catch (const std::exception& e) { throw JSONRPCError(RPC_MISC_ERROR, e.what()); diff --git a/src/test/quorum_proofs_tests.cpp b/src/test/quorum_proofs_tests.cpp index a4f733ca6d9b..6b670eb2e2f9 100644 --- a/src/test/quorum_proofs_tests.cpp +++ b/src/test/quorum_proofs_tests.cpp @@ -310,6 +310,8 @@ BOOST_FIXTURE_TEST_CASE(generation_retries_retired_checkpoint_signer, QuorumProo WITH_LOCK(cs_main, chain.SetTip(*tip)); // Missing history is a hard error, not a reason to skip to another target. + // A state read once is memoized by block hash, so start from a cold cache. + llmq::ClearProofStateCacheForTesting(); WITH_LOCK(cs_main, chain[checkpoint_height]->nStatus &= ~BLOCK_HAVE_DATA); BOOST_CHECK_EXCEPTION(Generate(checkpoint_height + 1), UniValue, [](const UniValue& error) { return error["message"].get_str().find("historical block unavailable") != std::string::npos; @@ -345,6 +347,39 @@ BOOST_FIXTURE_TEST_CASE(consensus_valid_envelopes_are_provable, QuorumProofGener const auto state = proof->Verify(llmq::QuorumProofBuilder::StateAt(checkpoint)); BOOST_CHECK_EQUAL(state.height, uint32_t(entry->height)); + + // The RPC reuses the target block's own state instead of re-verifying the + // proof; Build() only returns a proof whose verified state is exactly that. + BOOST_CHECK(state == llmq::QuorumProofBuilder::StateAt(chain[state.height])); + + // The memoized active set is the committed set: its leaves are sorted and + // hash to the block's own quorum root. + const auto active = builder.ActiveCommitments(checkpoint); + std::vector active_leaves; + for (const auto& commitment : active) + active_leaves.push_back(::SerializeHash(commitment)); + BOOST_CHECK(std::is_sorted(active_leaves.begin(), active_leaves.end())); + BOOST_CHECK(ComputeMerkleRoot(active_leaves) == llmq::QuorumProofBuilder::StateAt(checkpoint).quorumRoot); + + // Encoding with the caller's verified state is byte-identical to verifying again, + // and a state that is not the proof's own target is refused. + const auto target_set = builder.ActiveCommitments(chain[state.height]); + BOOST_REQUIRE(!target_set.empty()); + std::vector leaves; + for (const auto& commitment : target_set) + leaves.push_back(::SerializeHash(commitment)); + CDataStream raw(SER_NETWORK, PROTOCOL_VERSION); + raw << target_set.front(); + const llmq::ProofProjection record{0, + {UCharCast(raw.data()), UCharCast(raw.data()) + raw.size()}, + llmq::ProofMerklePath::Build(leaves, 0)}; + BOOST_CHECK(llmq::EncodeBootstrap(*proof, {record}) == llmq::EncodeBootstrap(*proof, state, {record})); + auto wrong = state; + wrong.quorumRoot = uint256::ONE; + BOOST_CHECK_THROW(llmq::EncodeBootstrap(*proof, wrong, {record}), std::runtime_error); + wrong = state; + wrong.height -= 1; + BOOST_CHECK_THROW(llmq::EncodeBootstrap(*proof, wrong, {record}), std::runtime_error); BOOST_CHECK(state.blockHash == proof->target.header.GetHash()); } BOOST_AUTO_TEST_CASE(real_testnet_wire_and_crypto) { From 0363826af3b72c7f7970b25158bc18af52a611f7 Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 22 Sep 2026 19:31:50 -0500 Subject: [PATCH 5/5] doc: add release notes for proof generation caching Co-Authored-By: Claude Opus 5.5 (1M context) --- doc/release-notes-7727.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 doc/release-notes-7727.md diff --git a/doc/release-notes-7727.md b/doc/release-notes-7727.md new file mode 100644 index 000000000000..d0e0b7a0a8a1 --- /dev/null +++ b/doc/release-notes-7727.md @@ -0,0 +1,17 @@ +Performance +----------- + +- `getquorumproofchain` and `getchainlockbyheight` now reuse work across calls. + Historical ChainLocks, quorum mining transactions, active quorum sets, block + states, parsed proof payloads and signing-quorum selections are kept in + bounded in-memory caches keyed by block hash or content. A repeated or + overlapping mainnet proof from a recent checkpoint now takes about 0.1 + seconds instead of about two seconds. The first request after startup still + reads and decodes its history. Proof output is unchanged. +- Nodes serving these RPCs use up to about 50 MiB of additional memory for + these caches. +- Cached results outlive the block data they came from. On a node whose block + data was pruned or removed after a proof was generated, a later request can + still succeed from the cache, while the same request on a freshly started node + fails with a missing-data error. Nodes must still retain the required + historical blocks for reliable proof generation.