Merge #7363: fix: validate CbTx chainlock height diff

c93049f test: clarify CbTx chainlock diff ancestor guard (PastaClaw)
c892349 fix: validate bestCLHeightDiff range in CheckCbTxBestChainlock (PastaClaw)

Pull request description:

  ## Issue being fixed or feature implemented

  `CheckCbTxBestChainlock` derived an ancestor height from `bestCLHeightDiff`
  without first ensuring the derived height was in range. This PR adds an
  explicit consensus rejection for out-of-range CbTx chainlock height diffs before
  ancestor lookup.

  ## What was done?

  - Validate `bestCLHeightDiff` before deriving and using the referenced chainlock
    ancestor height.
  - Return `bad-cbtx-cldiff` for out-of-range values.
  - Add a defensive null check around the ancestor lookup.
  - Add focused unit coverage for boundary values in a new `evo_cbtx_tests` suite.

  ## How Has This Been Tested?

  Tested locally on macOS arm64:

  ```text
  cd src && make -j6 test/test_dash
  ./test/test_dash --run_test=evo_cbtx_tests
  ./test/test_dash '--run_test=evo_*'
  ```

  Results:

  ```text
  make -j6 test/test_dash: pass
  ./test/test_dash --run_test=evo_cbtx_tests: 1/1 pass
  ./test/test_dash '--run_test=evo_*': 38/38 pass
  ```

  Pre-PR review gate:

  ```text
  code-review dashpay/dash upstream/develop review/fix-cbtx-chainlock-height-bound "Pre-PR review: validate CbTx bestCLHeightDiff range before ancestor lookup and add focused regression tests"
  ```

  Result: `ship`.

  ## Breaking Changes

  None expected. This only rejects malformed CbTx chainlock metadata that
  references an invalid ancestor height.

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [ ] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK c93049f

Tree-SHA512: 83ce8c3f262930001684a96face20df623f6123f1a9ad169c8c4f0a019a971b54452d19b1b13f3939371186cecd52091b6e20cbde024df088abbc47b34dbb40a

Author: PastaPastaPasta

src/Makefile.test.include

@@ -109,6 +109,7 @@ BITCOIN_TESTS =\
   test/descriptor_tests.cpp \
   test/dynamic_activation_thresholds_tests.cpp \
   test/evo_assetlocks_tests.cpp \
+  test/evo_cbtx_tests.cpp \
   test/evo_deterministicmns_tests.cpp \
   test/evo_islock_tests.cpp \
   test/evo_mnhf_tests.cpp \

src/evo/specialtxman.cpp

@@ -93,9 +93,9 @@ static bool SetStateVersion(CDeterministicMNState& state_mn, uint16_t nVersion,
     return true;
 }
 
-static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const Consensus::Params& consensus_params,
-                                   const CChain& chain, const llmq::CQuorumManager& qman,
-                                   const chainlock::Chainlocks& chainlocks, BlockValidationState& state)
+bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const Consensus::Params& consensus_params,
+                            const CChain& chain, const llmq::CQuorumManager& qman,
+                            const chainlock::Chainlocks& chainlocks, BlockValidationState& state)
 {
     if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
         return true;
@@ -136,6 +136,10 @@ static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex,
 
     // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null
     if (cbTx.bestCLSignature.IsValid()) {
+        // Reject out-of-range bestCLHeightDiff that would yield a pre-genesis ancestor height.
+        if (cbTx.bestCLHeightDiff >= static_cast<uint32_t>(pindex->nHeight)) {
+            return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff");
+        }
         int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast<int>(cbTx.bestCLHeightDiff) - 1;
         if (best_clsig.getHeight() == curBlockCoinbaseCLHeight && best_clsig.getSig() == cbTx.bestCLSignature) {
             // matches our best (but outdated) clsig, no need to verify it again
@@ -144,7 +148,13 @@ static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex,
             cached_pindex = pindex;
             return true;
         }
-        uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash();
+        const CBlockIndex* pAncestor = pindex->GetAncestor(curBlockCoinbaseCLHeight);
+        if (pAncestor == nullptr) {
+            // Defense-in-depth: the range check above keeps curBlockCoinbaseCLHeight in
+            // [0, pindex->nHeight - 1], so GetAncestor() should never return nullptr here.
+            return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff-ancestor");
+        }
+        uint256 curBlockCoinbaseCLBlockHash = pAncestor->GetBlockHash();
         chainlock::ChainLockSig clsig{curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature};
         llmq::VerifyRecSigStatus ret = chainlock::VerifyChainLock(consensus_params, chain, qman, clsig);
         if (ret != llmq::VerifyRecSigStatus::Valid) {

src/evo/specialtxman.h

@@ -15,6 +15,7 @@ class BlockValidationState;
 class CBlock;
 class CBlockIndex;
 class CCbTx;
+class CChain;
 class CCoinsViewCache;
 class CCreditPoolManager;
 class CDeterministicMNList;
@@ -93,6 +94,11 @@ class CSpecialTxProcessor
 };
 
 
+/** Validates the bestCLSignature / bestCLHeightDiff fields embedded in a CbTx payload. */
+bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex, const Consensus::Params& consensus_params,
+                            const CChain& chain, const llmq::CQuorumManager& qman,
+                            const chainlock::Chainlocks& chainlocks, BlockValidationState& state);
+
 bool CheckProRegTx(const CTransaction& tx, gsl::not_null<const CBlockIndex*> pindexPrev,
                    CDeterministicMNManager& dmnman, const CCoinsViewCache& view, const ChainstateManager& chainman,
                    TxValidationState& state, bool check_sigs);

src/test/evo_cbtx_tests.cpp

@@ -0,0 +1,70 @@
+// Copyright (c) 2026 The Dash Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <test/util/setup_common.h>
+
+#include <bls/bls.h>
+#include <chain.h>
+#include <chainlock/chainlock.h>
+#include <chainparams.h>
+#include <consensus/validation.h>
+#include <evo/cbtx.h>
+#include <evo/specialtxman.h>
+#include <llmq/context.h>
+#include <llmq/quorumsman.h>
+#include <uint256.h>
+#include <validation.h>
+
+#include <cstdint>
+#include <limits>
+
+#include <boost/test/unit_test.hpp>
+
+BOOST_AUTO_TEST_SUITE(evo_cbtx_tests)
+
+// Out-of-range bestCLHeightDiff (>= pindex->nHeight) must be rejected with
+// "bad-cbtx-cldiff" so that the subsequent GetAncestor() call sees a valid height.
+//
+// The defensive nullptr branch after GetAncestor() returns "bad-cbtx-cldiff-ancestor".
+// That branch is unreachable in practice (the range check guarantees the requested
+// ancestor height is in [0, pindex->nHeight - 1], for which GetAncestor() never returns
+// nullptr) and cannot be exercised from a unit test: a fake CBlockIndex with no pprev
+// would trip GetAncestor()'s `assert(pprev)` while walking, not return nullptr.
+BOOST_FIXTURE_TEST_CASE(check_cbtx_best_chainlock_rejects_excessive_height_diff, RegTestingSetup)
+{
+    const auto& consensus_params = Params().GetConsensus();
+    const auto& chain = m_node.chainman->ActiveChain();
+    auto& qman = *Assert(m_node.llmq_ctx)->qman;
+    auto& chainlocks = *Assert(m_node.chainlocks);
+
+    // Standalone fake block index with no predecessor, so the prevBlockCoinbaseChainlock
+    // branch is skipped and the validation path under test is reached directly.
+    CBlockIndex pindex;
+    pindex.nHeight = 5;
+
+    // A structurally-valid BLS signature is required for the IsValid() guard.
+    CBLSSecretKey sk;
+    sk.MakeNewKey();
+    const bool legacy_scheme = bls::bls_legacy_scheme.load();
+    CBLSSignature valid_sig = sk.Sign(uint256::ONE, legacy_scheme);
+    BOOST_REQUIRE(valid_sig.IsValid());
+
+    CCbTx cbTx;
+    cbTx.nVersion = CCbTx::Version::CLSIG_AND_BALANCE;
+    cbTx.bestCLSignature = valid_sig;
+
+    // bestCLHeightDiff == nHeight: lower boundary of the rejected range.
+    cbTx.bestCLHeightDiff = static_cast<uint32_t>(pindex.nHeight);
+    BlockValidationState state;
+    BOOST_CHECK(!CheckCbTxBestChainlock(cbTx, &pindex, consensus_params, chain, qman, chainlocks, state));
+    BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-cbtx-cldiff");
+
+    // Upper boundary: uint32_t max.
+    cbTx.bestCLHeightDiff = std::numeric_limits<uint32_t>::max();
+    BlockValidationState state_big;
+    BOOST_CHECK(!CheckCbTxBestChainlock(cbTx, &pindex, consensus_params, chain, qman, chainlocks, state_big));
+    BOOST_CHECK_EQUAL(state_big.GetRejectReason(), "bad-cbtx-cldiff");
+}
+
+BOOST_AUTO_TEST_SUITE_END()