From 00268d811c2f38a8dda5a40764b586d24af5d30d Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 20 Oct 2026 03:12:02 +0700 Subject: [PATCH 1/2] perf: deserialize CbTx bestCLSignature lazily Every parse of a CbTx special tx's playload after v20+ activation coinbase payload decompressed the chainlock signature's G2 point, ran the subgroup check and recompressed it for the malleability check, and the payload is parsed three to four times per connected block (CheckBlock, IsBlockMutated, special tx processing, the credit pool's read of the previous block). Only CheckCbTxBestChainlock and the chainlock handler use the signature. It consumes on mainnet during IBD 10-15% of CPU. Keep the bytes and decode on first use. The payload parser rejected bytes that do not decode to a valid signature. CheckCbTx now rejects them with the same reject reason. One corner differs: bytes that only decode under the other BLS scheme were treated as a null signature and are now rejected too. Both outcomes reject any block whose previous coinbase carried a chainlock, which is every mainnet block since the first one did. A block with a malformed signature and a wrong asset unlock root is now reported as mutated by CheckMerkleRoot instead of as consensus-invalid by the payload parser; it is rejected either way. --- src/chainlock/handler.cpp | 4 ++-- src/evo/cbtx.cpp | 21 +++++++++++++++++++-- src/evo/cbtx.h | 2 +- src/evo/specialtxman.cpp | 17 +++++++++-------- src/node/miner.cpp | 5 ++++- src/test/evo_cbtx_tests.cpp | 18 +++++++++++++++++- src/test/llmq_chainlock_tests.cpp | 8 ++++---- src/test/quorum_proofs_tests.cpp | 11 ++++++----- 8 files changed, 62 insertions(+), 24 deletions(-) diff --git a/src/chainlock/handler.cpp b/src/chainlock/handler.cpp index 7dacdd7b0fa5..0c71f072e674 100644 --- a/src/chainlock/handler.cpp +++ b/src/chainlock/handler.cpp @@ -110,7 +110,7 @@ MessageProcessingResult ChainlockHandler::ProcessCoinbaseChainLock(const CBlock& } const auto cbtx = GetTxPayload(*block.vtx.front(), /*assert_type=*/false); - if (!cbtx || cbtx->nVersion < CCbTx::Version::CLSIG_AND_BALANCE || !cbtx->bestCLSignature.IsValid()) { + if (!cbtx || cbtx->nVersion < CCbTx::Version::CLSIG_AND_BALANCE || !cbtx->bestCLSignature.Get().IsValid()) { return {}; } @@ -126,7 +126,7 @@ MessageProcessingResult ChainlockHandler::ProcessCoinbaseChainLock(const CBlock& if (ancestor == nullptr) { return {}; } - const ChainLockSig clsig{height, ancestor->GetBlockHash(), cbtx->bestCLSignature}; + const ChainLockSig clsig{height, ancestor->GetBlockHash(), cbtx->bestCLSignature.Get()}; return ProcessNewChainLock(/*from=*/-1, clsig, qman, ::SerializeHash(clsig)); } diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 10f23bf6deb1..e67afdddc4c5 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -51,6 +51,23 @@ bool CheckCbTx(const CCbTx& cbTx, const CBlockIndex* pindexPrev, bool is_v24_act } } + // The signature is deserialized lazily. Get() decodes it in the active scheme only and + // resets the wrapper on failure, which is what the eager parser did with bytes encoded + // in the opposite scheme; bytes that decode in neither scheme made it throw, so they + // are rejected here. + if (cbTx.nVersion >= CCbTx::Version::CLSIG_AND_BALANCE) { + CDataStream ss(SER_NETWORK, PROTOCOL_VERSION); + ss << cbTx.bestCLSignature; + if (!cbTx.bestCLSignature.Get().IsValid()) { + try { + CBLSSignature sig; + ss >> sig; + } catch (const std::ios_base::failure&) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload"); + } + } + } + return true; } @@ -203,8 +220,8 @@ std::optional> GetNonNullCoinbaseChainlock(co } const auto payload = GetTxPayload(*block.vtx[0]); if (!payload || payload->nVersion < CCbTx::Version::CLSIG_AND_BALANCE || payload->nHeight != height || - !payload->bestCLSignature.IsValid() || payload->bestCLHeightDiff >= uint32_t(height)) { + !payload->bestCLSignature.Get().IsValid() || payload->bestCLHeightDiff >= uint32_t(height)) { return std::nullopt; } - return std::make_pair(payload->bestCLSignature, payload->bestCLHeightDiff); + return std::make_pair(payload->bestCLSignature.Get(), payload->bestCLHeightDiff); } diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 2a4eca48658e..45ddee56b310 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -44,7 +44,7 @@ class CCbTx uint256 merkleRootMNList; uint256 merkleRootQuorums; uint32_t bestCLHeightDiff{0}; - CBLSSignature bestCLSignature; + CBLSLazySignature bestCLSignature; CAmount creditPoolBalance{0}; /** Merkle root over the instance hashes of the block's version 2+ asset unlock transactions * (block order; null when there are none). Their txids exclude the quorum signing info, so diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 4085b40d454e..02bc3025155e 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -163,6 +163,7 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) { return true; } + const CBLSSignature& cbtx_sig{cbTx.bestCLSignature.Get()}; static Mutex cached_mutex; static const CBlockIndex* cached_pindex GUARDED_BY(cached_mutex){nullptr}; @@ -170,10 +171,10 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const auto best_clsig = chainlocks.GetBestChainLock(); if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && - cbTx.bestCLSignature == best_clsig.getSig()) { + cbtx_sig == best_clsig.getSig()) { // matches our best clsig which still hold values for the previous block LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); + cached_chainlock = std::make_pair(cbtx_sig, cbTx.bestCLHeightDiff); cached_pindex = pindex; return true; } @@ -188,7 +189,7 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const // If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null. if (prevBlockCoinbaseChainlock.has_value()) { // Previous block Coinbase has a non-null Chainlock: current block's Chainlock must be non-null and at least as new as the previous one - if (!cbTx.bestCLSignature.IsValid()) { + if (!cbtx_sig.IsValid()) { // IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig"); } @@ -198,16 +199,16 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const } // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null - if (cbTx.bestCLSignature.IsValid()) { + if (cbtx_sig.IsValid()) { // Reject out-of-range bestCLHeightDiff that would yield a pre-genesis ancestor height. if (cbTx.bestCLHeightDiff >= static_cast(pindex->nHeight)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); } int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; - if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) { + if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbtx_sig) { // matches our best (but outdated) clsig, no need to verify it again LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); + cached_chainlock = std::make_pair(cbtx_sig, cbTx.bestCLHeightDiff); cached_pindex = pindex; return true; } @@ -218,13 +219,13 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff-ancestor"); } uint256 curBlockCoinbaseCLBlockHash = pAncestor->GetBlockHash(); - chainlock::ChainLockSig clsig{curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature}; + chainlock::ChainLockSig clsig{curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbtx_sig}; llmq::VerifyRecSigStatus ret = chainlock::VerifyChainLock(consensus_params, chain, qman, clsig); if (ret != llmq::VerifyRecSigStatus::Valid) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); } LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff); + cached_chainlock = std::make_pair(cbtx_sig, cbTx.bestCLHeightDiff); cached_pindex = pindex; } else if (cbTx.bestCLHeightDiff != 0) { // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 4e06d4d84bb4..93b93c28811a 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -354,7 +354,10 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc throw std::runtime_error(strprintf("%s: CalcCbTxMerkleRootQuorums failed: %s", __func__, state.ToString())); } if (fV20Active_context) { - if (CalcCbTxBestChainlock(m_chainlocks, pindexPrev, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { + CBLSSignature best_cl_sig; + const bool has_best_cl{CalcCbTxBestChainlock(m_chainlocks, pindexPrev, cbTx.bestCLHeightDiff, best_cl_sig)}; + cbTx.bestCLSignature.Set(best_cl_sig, bls::bls_legacy_scheme.load()); + if (has_best_cl) { LogPrintf("CreateNewBlock() h[%d] CbTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); } else { // not an error diff --git a/src/test/evo_cbtx_tests.cpp b/src/test/evo_cbtx_tests.cpp index ff9f42cc40c2..090ac9de96b2 100644 --- a/src/test/evo_cbtx_tests.cpp +++ b/src/test/evo_cbtx_tests.cpp @@ -22,9 +22,11 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -66,7 +68,7 @@ BOOST_FIXTURE_TEST_CASE(check_cbtx_best_chainlock_rejects_excessive_height_diff, CCbTx cbTx; cbTx.nVersion = CCbTx::Version::CLSIG_AND_BALANCE; - cbTx.bestCLSignature = valid_sig; + cbTx.bestCLSignature.Set(valid_sig, legacy_scheme); // bestCLHeightDiff == nHeight: lower boundary of the rejected range. cbTx.bestCLHeightDiff = static_cast(pindex.nHeight); @@ -79,6 +81,20 @@ BOOST_FIXTURE_TEST_CASE(check_cbtx_best_chainlock_rejects_excessive_height_diff, BlockValidationState state_big; BOOST_CHECK(!CheckCbTxBestChainlock(cbTx, &pindex, consensus_params, chain, qman, chainlocks, state_big)); BOOST_CHECK_EQUAL(state_big.GetRejectReason(), "bad-cbtx-cldiff"); + + // The signature is deserialized lazily, so the payload parser accepts any bytes in its + // place and CheckCbTx rejects an encoding that is not a signature; the valid signature + // above and the null encoding pass. + TxValidationState tx_state; + BOOST_CHECK(CheckCbTx(cbTx, /*pindexPrev=*/nullptr, /*is_v24_active=*/false, tx_state)); + std::vector sig_bytes(CBLSSignature::SerSize, 0xff); + CDataStream(sig_bytes, SER_NETWORK, PROTOCOL_VERSION) >> cbTx.bestCLSignature; + BOOST_CHECK(!CheckCbTx(cbTx, /*pindexPrev=*/nullptr, /*is_v24_active=*/false, tx_state)); + BOOST_CHECK_EQUAL(tx_state.GetRejectReason(), "bad-cbtx-payload"); + std::fill(sig_bytes.begin(), sig_bytes.end(), 0); + CDataStream(sig_bytes, SER_NETWORK, PROTOCOL_VERSION) >> cbTx.bestCLSignature; + TxValidationState null_state; + BOOST_CHECK(CheckCbTx(cbTx, /*pindexPrev=*/nullptr, /*is_v24_active=*/false, null_state)); } namespace { diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index 8fcc541d82a8..e3aed1100e00 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -71,7 +71,7 @@ BOOST_FIXTURE_TEST_CASE(historical_coinbase_lookup_from_disk, RegTestingSetup) } else { certified = activation + 16999; } - payload.bestCLSignature = signature; + payload.bestCLSignature.Set(signature, bls::bls_legacy_scheme.load()); payload.bestCLHeightDiff = height - certified - 1; } CMutableTransaction coinbase; @@ -415,7 +415,7 @@ BOOST_FIXTURE_TEST_CASE(coinbase_chainlock_processing, RegTestingSetup) CCbTx cbtx; cbtx.nVersion = CCbTx::Version::CLSIG_AND_BALANCE; - cbtx.bestCLSignature = CreateRandomBLSSignature(); + cbtx.bestCLSignature.Set(CreateRandomBLSSignature(), bls::bls_legacy_scheme.load()); CMutableTransaction tx; tx.nVersion = CTransaction::SPECIAL_VERSION; tx.nType = TRANSACTION_COINBASE; @@ -439,7 +439,7 @@ BOOST_FIXTURE_TEST_CASE(coinbase_chainlock_processing, RegTestingSetup) set_payload(); process(&indexes.back()); const int32_t height = 499 - offset; - const ChainLockSig expected{height, hashes[height], cbtx.bestCLSignature}; + const ChainLockSig expected{height, hashes[height], cbtx.bestCLSignature.Get()}; BOOST_CHECK(m_node.clhandler->AlreadyHave(CInv{MSG_CLSIG, ::SerializeHash(expected)})); } const auto seen = m_node.clhandler->SeenChainLockCacheSizeForTesting(); @@ -464,7 +464,7 @@ BOOST_FIXTURE_TEST_CASE(coinbase_chainlock_processing, RegTestingSetup) process(&indexes.back()); BOOST_REQUIRE(m_node.sporkman->UpdateSpork(SPORK_19_CHAINLOCKS_ENABLED, 0).has_value()); - cbtx.bestCLSignature = {}; + cbtx.bestCLSignature = CBLSLazySignature{}; set_payload(); process(&indexes.back()); cbtx.nVersion = CCbTx::Version::MERKLE_ROOT_QUORUMS; diff --git a/src/test/quorum_proofs_tests.cpp b/src/test/quorum_proofs_tests.cpp index f79e4a463992..582f884fde54 100644 --- a/src/test/quorum_proofs_tests.cpp +++ b/src/test/quorum_proofs_tests.cpp @@ -163,11 +163,12 @@ struct QuorumProofGenerationSetup : TestingSetup { }); BOOST_REQUIRE(match != commitments.end()); const size_t key = size_t(match - commitments.begin()); - payload.bestCLSignature = keys[key].Sign(llmq::SignHash{params.type, signer->quorumHash, - chainlock::GenSigRequestId(height - 1), - previous->GetBlockHash()} - .Get(), - false); + payload.bestCLSignature.Set(keys[key].Sign(llmq::SignHash{params.type, signer->quorumHash, + chainlock::GenSigRequestId(height - 1), + previous->GetBlockHash()} + .Get(), + false), + bls::bls_legacy_scheme.load()); } const bool mining = height == checkpoint_height - params.dkgInterval || height == checkpoint_height || (mine_nonstandard_handoff && height == handoff_height); From 78c28ede46935d471379597554f18c16f7973c56 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 21 Oct 2026 00:17:56 +0700 Subject: [PATCH 2/2] perf: do not re-verify a coinbase chainlock carried over from the previous block Verification by CheckCbTxBestChainlock requires one pairing on the connecting thread and it takes around ~14% of overall time in main validation thread. When no newer ChainLock exists at mining time the coinbase repeats the previous block's best chainlock signature with the height offset grown by one: the same height, block hash and signature. The function already caches the previous block's coinbase chainlock once it verified it. If any verification could be skipped it impoves sync time --- src/evo/specialtxman.cpp | 37 +++++++++++++++--------- src/kernel/chainparams.cpp | 3 ++ test/functional/feature_protx_version.py | 2 ++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 02bc3025155e..078de7ee8da2 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -165,23 +165,30 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const } const CBLSSignature& cbtx_sig{cbTx.bestCLSignature.Get()}; + struct CachedChainlock { + const CBlockIndex* pindex{nullptr}; + CBLSSignature sig; + uint32_t height_diff{0}; + //! Passed VerifyChainLock in this process (or was carried over from such a block) rather than + //! merely matched the chainlock handler's best lock, whose verification scheme is unknown here + bool verified_here{false}; + }; static Mutex cached_mutex; - static const CBlockIndex* cached_pindex GUARDED_BY(cached_mutex){nullptr}; - static std::optional> cached_chainlock GUARDED_BY(cached_mutex){std::nullopt}; + static std::optional cached GUARDED_BY(cached_mutex); auto best_clsig = chainlocks.GetBestChainLock(); if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && cbtx_sig == best_clsig.getSig()) { // matches our best clsig which still hold values for the previous block - LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbtx_sig, cbTx.bestCLHeightDiff); - cached_pindex = pindex; + WITH_LOCK(cached_mutex, cached.emplace(pindex, cbtx_sig, cbTx.bestCLHeightDiff, /*verified_here=*/false)); return true; } std::optional> prevBlockCoinbaseChainlock{std::nullopt}; - if (LOCK(cached_mutex); cached_pindex == pindex->pprev) { - prevBlockCoinbaseChainlock = cached_chainlock; + bool prev_verified_here{false}; + if (LOCK(cached_mutex); cached && cached->pindex == pindex->pprev) { + prevBlockCoinbaseChainlock = std::make_pair(cached->sig, cached->height_diff); + prev_verified_here = cached->verified_here; } if (!prevBlockCoinbaseChainlock.has_value()) { prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev); @@ -207,9 +214,15 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbtx_sig) { // matches our best (but outdated) clsig, no need to verify it again - LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbtx_sig, cbTx.bestCLHeightDiff); - cached_pindex = pindex; + WITH_LOCK(cached_mutex, cached.emplace(pindex, cbtx_sig, cbTx.bestCLHeightDiff, /*verified_here=*/false)); + return true; + } + // The previous block's coinbase chainlock was verified by this process; carried into this + // block with the offset grown by one it names the same height, block and signature. + if (prev_verified_here && prevBlockCoinbaseChainlock.has_value() && + cbTx.bestCLHeightDiff == prevBlockCoinbaseChainlock->second + 1 && + cbtx_sig == prevBlockCoinbaseChainlock->first) { + WITH_LOCK(cached_mutex, cached.emplace(pindex, cbtx_sig, cbTx.bestCLHeightDiff, /*verified_here=*/true)); return true; } const CBlockIndex* pAncestor = pindex->GetAncestor(curBlockCoinbaseCLHeight); @@ -224,9 +237,7 @@ bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const if (ret != llmq::VerifyRecSigStatus::Valid) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); } - LOCK(cached_mutex); - cached_chainlock = std::make_pair(cbtx_sig, cbTx.bestCLHeightDiff); - cached_pindex = pindex; + WITH_LOCK(cached_mutex, cached.emplace(pindex, cbtx_sig, cbTx.bestCLHeightDiff, /*verified_here=*/true)); } else if (cbTx.bestCLHeightDiff != 0) { // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); diff --git a/src/kernel/chainparams.cpp b/src/kernel/chainparams.cpp index 225268291879..ba1721cd8489 100644 --- a/src/kernel/chainparams.cpp +++ b/src/kernel/chainparams.cpp @@ -1031,6 +1031,9 @@ class CRegTestParams : public CChainParams { assert(consensus.V20Height >= consensus.DIP0003Height); // MN_RR reallocate part of reward to CreditPool which exits since V20 assert(consensus.MN_RRHeight >= consensus.V20Height); + // Coinbase chainlocks (V20) are verified once and reused by the next block, so the BLS + // scheme must not change after V20 activation + assert(consensus.V20Height >= consensus.V19Height); } private: diff --git a/test/functional/feature_protx_version.py b/test/functional/feature_protx_version.py index 5af72f1719d7..f6d7adaf15ef 100755 --- a/test/functional/feature_protx_version.py +++ b/test/functional/feature_protx_version.py @@ -38,6 +38,8 @@ def set_test_params(self): f'-vbparams=v24:{self.mocktime}:999999999999:350:10:8:6:5:0', ]] * 2 self.set_dash_test_params(2, 1, evo_count=2, extra_args=self.extra_args) + # regtest requires v20 not earlier than v19 + self.delay_v20_and_mn_rr(200) def get_peer_ids(self, node_idx): return {peer['id'] for peer in self.nodes[node_idx].getpeerinfo()}