refactor: move CL code validation to break circular dependencies over CbTx

Author: knst

src/evo/cbtx.cpp

@@ -6,7 +6,6 @@
 #include <evo/cbtx.h>
 #include <evo/specialtx.h>
 #include <llmq/blockprocessor.h>
-#include <llmq/chainlocks.h>
 #include <llmq/commitment.h>
 #include <llmq/options.h>
 #include <llmq/quorums.h>
@@ -217,129 +216,6 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
     return true;
 }
 
-bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex,
-                            const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state)
-{
-    if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
-        return true;
-    }
-
-    static Mutex cached_mutex;
-    static const CBlockIndex* cached_pindex GUARDED_BY(cached_mutex){nullptr};
-    static std::optional<std::pair<CBLSSignature, uint32_t>> cached_chainlock GUARDED_BY(cached_mutex){std::nullopt};
-
-    auto best_clsig = chainlock_handler.GetBestChainLock();
-    if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 && cbTx.bestCLSignature == 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_pindex = pindex;
-        return true;
-    }
-
-    std::optional<std::pair<CBLSSignature, uint32_t>> prevBlockCoinbaseChainlock{std::nullopt};
-    if (LOCK(cached_mutex); cached_pindex == pindex->pprev) {
-        prevBlockCoinbaseChainlock = cached_chainlock;
-    }
-    if (!prevBlockCoinbaseChainlock.has_value()) {
-        prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev);
-    }
-    // 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()) {
-            // 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");
-        }
-        if (cbTx.bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) {
-            return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig");
-        }
-    }
-
-    // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null
-    if (cbTx.bestCLSignature.IsValid()) {
-        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
-            LOCK(cached_mutex);
-            cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff);
-            cached_pindex = pindex;
-            return true;
-        }
-        uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash();
-        if (chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) != 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_pindex = pindex;
-    } else if (cbTx.bestCLHeightDiff != 0) {
-        // Null bestCLSignature is allowed only with bestCLHeightDiff = 0
-        return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff");
-    }
-
-    return true;
-}
-
-bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
-                           uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature)
-{
-    auto best_clsig = chainlock_handler.GetBestChainLock();
-    if (best_clsig.getHeight() < Params().GetConsensus().DeploymentHeight(Consensus::DEPLOYMENT_V19)) {
-        // We don't want legacy BLS ChainLocks in CbTx (can happen on regtest/devenets)
-        best_clsig = llmq::CChainLockSig{};
-    }
-    if (best_clsig.getHeight() == pindexPrev->nHeight) {
-        // Our best CL is the newest one possible
-        bestCLHeightDiff = 0;
-        bestCLSignature = best_clsig.getSig();
-        return true;
-    }
-
-    auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev);
-    if (prevBlockCoinbaseChainlock.has_value()) {
-        // Previous block Coinbase contains a non-null CL: We must insert the same sig or a better (newest) one
-        if (best_clsig.IsNull()) {
-            // We don't know any CL, therefore inserting the CL of the previous block
-            bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1;
-            bestCLSignature = prevBlockCoinbaseChainlock->first;
-            return true;
-        }
-
-        // We check if our best CL is newer than the one from previous block Coinbase
-        int curCLHeight = best_clsig.getHeight();
-        int prevCLHeight = pindexPrev->nHeight - static_cast<int>(prevBlockCoinbaseChainlock->second) - 1;
-        if (curCLHeight < prevCLHeight) {
-            // Our best CL isn't newer: inserting CL from previous block
-            bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1;
-            bestCLSignature = prevBlockCoinbaseChainlock->first;
-        }
-        else {
-            // Our best CL is newer
-            bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight();
-            bestCLSignature = best_clsig.getSig();
-        }
-
-        return true;
-    }
-    else {
-        // Previous block Coinbase has no CL. We can either insert null or any valid CL
-        if (best_clsig.IsNull()) {
-            // We don't know any CL, therefore inserting a null CL
-            bestCLHeightDiff = 0;
-            bestCLSignature.Reset();
-            return false;
-        }
-
-        // Inserting our best CL
-        bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight();
-        bestCLSignature = chainlock_handler.GetBestChainLock().getSig();
-
-        return true;
-    }
-}
-
-
 std::string CCbTx::ToString() const
 {
     return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d)",

src/evo/cbtx.h

@@ -16,7 +16,6 @@ class CBlock;
 class CBlockIndex;
 class TxValidationState;
 namespace llmq {
-class CChainLocksHandler;
 class CQuorumBlockProcessor;
 }// namespace llmq
 
@@ -71,11 +70,6 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre
                                const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet,
                                BlockValidationState& state);
 
-bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindexPrev,
-                            const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state);
-bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
-                           uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature);
-
 std::optional<std::pair<CBLSSignature, uint32_t>> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex);
 
 #endif // BITCOIN_EVO_CBTX_H

src/evo/specialtxman.cpp

@@ -17,10 +17,80 @@
 #include <evo/simplifiedmns.h>
 #include <hash.h>
 #include <llmq/blockprocessor.h>
+#include <llmq/chainlocks.h>
+#include <llmq/clsig.h>
 #include <llmq/commitment.h>
+#include <llmq/quorums.h>
 #include <primitives/block.h>
 #include <validation.h>
 
+static bool CheckCbTxBestChainlock(const CCbTx& cbTx, const CBlockIndex* pindex,
+                                   const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state)
+{
+    if (cbTx.nVersion < CCbTx::Version::CLSIG_AND_BALANCE) {
+        return true;
+    }
+
+    static Mutex cached_mutex;
+    static const CBlockIndex* cached_pindex GUARDED_BY(cached_mutex){nullptr};
+    static std::optional<std::pair<CBLSSignature, uint32_t>> cached_chainlock GUARDED_BY(cached_mutex){std::nullopt};
+
+    auto best_clsig = chainlock_handler.GetBestChainLock();
+    if (best_clsig.getHeight() == pindex->nHeight - 1 && cbTx.bestCLHeightDiff == 0 &&
+        cbTx.bestCLSignature == 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_pindex = pindex;
+        return true;
+    }
+
+    std::optional<std::pair<CBLSSignature, uint32_t>> prevBlockCoinbaseChainlock{std::nullopt};
+    if (LOCK(cached_mutex); cached_pindex == pindex->pprev) {
+        prevBlockCoinbaseChainlock = cached_chainlock;
+    }
+    if (!prevBlockCoinbaseChainlock.has_value()) {
+        prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex->pprev);
+    }
+    // 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()) {
+            // 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");
+        }
+        if (cbTx.bestCLHeightDiff > prevBlockCoinbaseChainlock.value().second + 1) {
+            return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig");
+        }
+    }
+
+    // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null
+    if (cbTx.bestCLSignature.IsValid()) {
+        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
+            LOCK(cached_mutex);
+            cached_chainlock = std::make_pair(cbTx.bestCLSignature, cbTx.bestCLHeightDiff);
+            cached_pindex = pindex;
+            return true;
+        }
+        uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash();
+        if (chainlock_handler.VerifyChainLock(
+                llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) !=
+            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_pindex = pindex;
+    } else if (cbTx.bestCLHeightDiff != 0) {
+        // Null bestCLSignature is allowed only with bestCLHeightDiff = 0
+        return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff");
+    }
+
+    return true;
+}
+
 static bool CheckSpecialTxInner(CDeterministicMNManager& dmnman, llmq::CQuorumSnapshotManager& qsnapman,
                                 const ChainstateManager& chainman, const llmq::CQuorumManager& qman,
                                 const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache& view,

src/node/miner.cpp

@@ -121,6 +121,66 @@ void BlockAssembler::resetBlock()
     nFees = 0;
 }
 
+// Helper to calculate best chainlock
+static bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev,
+                                  uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature)
+{
+    auto best_clsig = chainlock_handler.GetBestChainLock();
+    if (best_clsig.getHeight() < Params().GetConsensus().DeploymentHeight(Consensus::DEPLOYMENT_V19)) {
+        // We don't want legacy BLS ChainLocks in CbTx (can happen on regtest/devenets)
+        best_clsig = llmq::CChainLockSig{};
+    }
+    if (best_clsig.getHeight() == pindexPrev->nHeight) {
+        // Our best CL is the newest one possible
+        bestCLHeightDiff = 0;
+        bestCLSignature = best_clsig.getSig();
+        return true;
+    }
+
+    auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev);
+    if (prevBlockCoinbaseChainlock.has_value()) {
+        // Previous block Coinbase contains a non-null CL: We must insert the same sig or a better (newest) one
+        if (best_clsig.IsNull()) {
+            // We don't know any CL, therefore inserting the CL of the previous block
+            bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1;
+            bestCLSignature = prevBlockCoinbaseChainlock->first;
+            return true;
+        }
+
+        // We check if our best CL is newer than the one from previous block Coinbase
+        int curCLHeight = best_clsig.getHeight();
+        int prevCLHeight = pindexPrev->nHeight - static_cast<int>(prevBlockCoinbaseChainlock->second) - 1;
+        if (curCLHeight < prevCLHeight) {
+            // Our best CL isn't newer: inserting CL from previous block
+            bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1;
+            bestCLSignature = prevBlockCoinbaseChainlock->first;
+        }
+        else {
+            // Our best CL is newer
+            bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight();
+            bestCLSignature = best_clsig.getSig();
+        }
+
+        return true;
+    }
+    else {
+        // Previous block Coinbase has no CL. We can either insert null or any valid CL
+        if (best_clsig.IsNull()) {
+            // We don't know any CL, therefore inserting a null CL
+            bestCLHeightDiff = 0;
+            bestCLSignature.Reset();
+            return false;
+        }
+
+        // Inserting our best CL
+        bestCLHeightDiff = pindexPrev->nHeight - best_clsig.getHeight();
+        bestCLSignature = chainlock_handler.GetBestChainLock().getSig();
+
+        return true;
+    }
+}
+
+
 std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn)
 {
     int64_t nTimeStart = GetTimeMicros();

test/lint/lint-circular-dependencies.py

@@ -47,7 +47,6 @@
     "evo/netinfo -> evo/providertx -> evo/netinfo",
     "evo/simplifiedmns -> llmq/blockprocessor -> llmq/utils -> llmq/snapshot -> evo/simplifiedmns",
     "core_io -> evo/assetlocktx -> llmq/signing -> net_processing -> evo/simplifiedmns -> core_io",
-    "evo/cbtx -> llmq/chainlocks -> llmq/instantsend -> net_processing -> evo/simplifiedmns -> evo/cbtx",
     "evo/specialtxman -> validation -> evo/specialtxman",
     "governance/governance -> governance/object -> governance/governance",
     "governance/governance -> masternode/sync -> governance/governance",