diff --git a/src/chainlock/chainlock.cpp b/src/chainlock/chainlock.cpp index 39154d30d2d2..97a8ec155734 100644 --- a/src/chainlock/chainlock.cpp +++ b/src/chainlock/chainlock.cpp @@ -71,6 +71,7 @@ void CChainLocksHandler::Start(const llmq::CInstantSendManager& isman) [&]() { auto signer = m_signer.load(std::memory_order_acquire); CheckActiveState(); + ProcessPendingCoinbaseChainLocks(); EnforceBestChainLock(); Cleanup(); // regularly retry signing the current chaintip as it might have failed before due to missing islocks @@ -490,4 +491,72 @@ void CChainLocksHandler::Cleanup() } } } + +void CChainLocksHandler::QueueCoinbaseChainLock(const chainlock::ChainLockSig& clsig) +{ + LOCK(cs); + + if (!IsEnabled()) { + return; + } + + // Only queue if it's potentially newer than what we have + if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) { + return; + } + + // Check if we've already seen this chainlock + const uint256 hash = ::SerializeHash(clsig); + if (seenChainLocks.count(hash) != 0) { + return; + } + + pendingCoinbaseChainLocks.push_back(clsig); +} + +void CChainLocksHandler::ProcessPendingCoinbaseChainLocks() +{ + AssertLockNotHeld(cs); + AssertLockNotHeld(cs_main); + + if (!IsEnabled()) { + return; + } + + std::vector toProcess; + { + LOCK(cs); + if (pendingCoinbaseChainLocks.empty()) { + return; + } + + // Swap to avoid copying - O(1) operation + toProcess.swap(pendingCoinbaseChainLocks); + } + + // Process in LIFO order (newest first) to minimize wasted work during reindex + // Once a newer chainlock is accepted, older ones will fail the height check early + for (auto it = toProcess.rbegin(); it != toProcess.rend(); ++it) { + const auto& clsig = *it; + + // Check height first (cheap), then hash and check if seen (if height passed) + uint256 hash; + { + LOCK(cs); + // Fast height check to skip old chainlocks without hashing + if (!bestChainLock.IsNull() && clsig.getHeight() <= bestChainLock.getHeight()) { + continue; + } + // Only compute hash if height check passed + hash = ::SerializeHash(clsig); + if (seenChainLocks.count(hash) != 0) { + continue; + } + } + + // Process as if it came from a coinbase (from = -1 means internal) + // Ignore return value as we're processing internally from coinbase + (void)ProcessNewChainLock(-1, clsig, hash); + } +} } // namespace llmq diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h index c999d7f0f81c..fb6d2ad66c64 100644 --- a/src/chainlock/chainlock.h +++ b/src/chainlock/chainlock.h @@ -24,6 +24,7 @@ #include #include #include +#include class CBlock; class CBlockIndex; @@ -69,6 +70,9 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent std::atomic lastCleanupTime{0s}; + // Queue for coinbase chainlocks to be processed asynchronously + std::vector pendingCoinbaseChainLocks GUARDED_BY(cs); + public: CChainLocksHandler() = delete; CChainLocksHandler(const CChainLocksHandler&) = delete; @@ -126,9 +130,14 @@ class CChainLocksHandler final : public chainlock::ChainLockSignerParent EXCLUSIVE_LOCKS_REQUIRED(!cs); [[nodiscard]] bool IsEnabled() const override { return isEnabled; } + // Queue a coinbase chainlock for asynchronous processing + // This is called during block validation to avoid blocking + void QueueCoinbaseChainLock(const chainlock::ChainLockSig& clsig) EXCLUSIVE_LOCKS_REQUIRED(!cs); + private: void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs); + void ProcessPendingCoinbaseChainLocks() EXCLUSIVE_LOCKS_REQUIRED(!cs); }; bool AreChainLocksEnabled(const CSporkManager& sporkman); diff --git a/src/evo/chainhelper.cpp b/src/evo/chainhelper.cpp index 9c11fc4e6a3c..230a607603db 100644 --- a/src/evo/chainhelper.cpp +++ b/src/evo/chainhelper.cpp @@ -17,7 +17,7 @@ CChainstateHelper::CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministi llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync, const CSporkManager& sporkman, - const llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman) : + llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman) : isman{isman}, clhandler{clhandler}, mn_payments{std::make_unique(dmnman, govman, chainman, consensus_params, mn_sync, sporkman)}, diff --git a/src/evo/chainhelper.h b/src/evo/chainhelper.h index 66bee994652f..5f1a4f93db4e 100644 --- a/src/evo/chainhelper.h +++ b/src/evo/chainhelper.h @@ -33,7 +33,7 @@ class CChainstateHelper { private: llmq::CInstantSendManager& isman; - const llmq::CChainLocksHandler& clhandler; + llmq::CChainLocksHandler& clhandler; public: CChainstateHelper() = delete; @@ -44,7 +44,7 @@ class CChainstateHelper llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync, const CSporkManager& sporkman, - const llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman); + llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman); ~CChainstateHelper(); /** Passthrough functions to CChainLocksHandler */ diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 35d3fe14bc43..ace812215532 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -645,6 +645,18 @@ bool CSpecialTxProcessor::ProcessSpecialTxsInBlock(const CBlock& block, const CB return false; } + // Queue the coinbase chainlock for asynchronous processing if it's valid + if (opt_cbTx->bestCLSignature.IsValid() && !fJustCheck) { + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(opt_cbTx->bestCLHeightDiff) - 1; + const CBlockIndex* pindexCL = pindex->GetAncestor(curBlockCoinbaseCLHeight); + if (pindexCL) { + uint256 curBlockCoinbaseCLBlockHash = pindexCL->GetBlockHash(); + chainlock::ChainLockSig clsig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, + opt_cbTx->bestCLSignature); + m_clhandler.QueueCoinbaseChainLock(clsig); + } + } + int64_t nTime6_3 = GetTimeMicros(); nTimeCbTxCL += nTime6_3 - nTime6_2; LogPrint(BCLog::BENCHMARK, " - CheckCbTxBestChainlock: %.2fms [%.2fs]\n", diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 84f74cd06e85..5610a0eb16d8 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -45,14 +45,14 @@ class CSpecialTxProcessor llmq::CQuorumSnapshotManager& m_qsnapman; const ChainstateManager& m_chainman; const Consensus::Params& m_consensus_params; - const llmq::CChainLocksHandler& m_clhandler; + llmq::CChainLocksHandler& m_clhandler; const llmq::CQuorumManager& m_qman; public: explicit CSpecialTxProcessor(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman, CMNHFManager& mnhfman, llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const Consensus::Params& consensus_params, - const llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman) : + llmq::CChainLocksHandler& clhandler, const llmq::CQuorumManager& qman) : m_cpoolman(cpoolman), m_dmnman{dmnman}, m_mnhfman{mnhfman}, diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index ea070fb6e867..a77a212424b3 100755 --- a/test/functional/feature_llmq_chainlocks.py +++ b/test/functional/feature_llmq_chainlocks.py @@ -14,7 +14,7 @@ from test_framework.messages import CBlock, CCbTx from test_framework.test_framework import DashTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, force_finish_mnsync +from test_framework.util import assert_equal, assert_greater_than, assert_raises_rpc_error, force_finish_mnsync import time @@ -251,6 +251,9 @@ def test_cb(self): self.log.info("Test bestCLHeightDiff restrictions") self.test_bestCLHeightDiff() + self.log.info("Test coinbase chainlock recovery") + self.test_coinbase_chainlock_recovery() + def create_chained_txs(self, node, amount): txid = node.sendtoaddress(node.getnewaddress(), amount) tx = node.getrawtransaction(txid, 1) @@ -354,6 +357,73 @@ def test_bestCLHeightDiff(self): self.reconnect_isolated_node(1, 0) self.sync_all() + def test_coinbase_chainlock_recovery(self): + """ + Test that nodes can learn about chainlocks from coinbase transactions + when they miss the P2P broadcast. + + This verifies the async chainlock queueing and processing mechanism. + """ + self.log.info("Testing coinbase chainlock recovery from submitted blocks...") + + # Isolate node4 before creating a chainlock + self.isolate_node(4) + + # Mine one block on nodes 0-3 and wait for it to be chainlocked + cl_block_hash = self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks(self.nodes[0:4]))[0] + self.wait_for_chainlocked_block(self.nodes[0], cl_block_hash, timeout=15) + cl_height = self.nodes[0].getblockcount() + + # Mine another block - its coinbase should contain the chainlock + new_block_hash = self.generate(self.nodes[0], 1, sync_fun=lambda: self.sync_blocks(self.nodes[0:4]))[0] + + # Verify the new block's coinbase contains the chainlock for cl_block_hash + cbtx = self.nodes[0].getblock(new_block_hash, 2)["cbTx"] + # CbTx should include chainlock fields + assert_greater_than(int(cbtx["version"]), 2) + # CbTx should reference immediately previous block + assert_equal(int(cbtx["bestCLHeightDiff"]), 0) + + # Verify the chainlock in coinbase matches our saved block + cb_cl_height = int(cbtx["height"]) - int(cbtx["bestCLHeightDiff"]) - 1 + assert_equal(cb_cl_height, cl_height) + cb_cl_block_hash = self.nodes[0].getblockhash(cb_cl_height) + assert_equal(cb_cl_block_hash, cl_block_hash) + + # Now submit both blocks to isolated node4 via submitblock (NOT via P2P) + # This way node4 gets the blocks but NOT the chainlock P2P message + cl_block_hex = self.nodes[0].getblock(cl_block_hash, 0) + self.nodes[4].submitblock(cl_block_hex) + + new_block_hex = self.nodes[0].getblock(new_block_hash, 0) + result = self.nodes[4].submitblock(new_block_hex) + assert_equal(result, None) + assert_equal(self.nodes[4].getbestblockhash(), new_block_hash) + + # Verify node4 has the blocks but NOT the chainlock (missed P2P message) + node4_block = self.nodes[4].getblock(cl_block_hash) + assert not node4_block["chainlock"], "Node4 should not have chainlock yet (no P2P)" + + # At this point: + # - Node4 has both blocks + # - Node4 has NOT received chainlock via P2P + # - Node4 HAS seen the chainlock in the coinbase of new_block_hash + # - The chainlock should be queued for async processing + + # Trigger scheduler to process pending coinbase chainlocks + # The scheduler runs every 5 seconds, so advancing by 6 seconds ensures it runs + self.log.info("Triggering async chainlock processing from coinbase...") + self.nodes[4].mockscheduler(6) + + # Verify node4 learned about the chainlock from the coinbase + self.wait_for_chainlocked_block(self.nodes[4], cl_block_hash, timeout=5) + + self.log.info("Node successfully recovered chainlock from coinbase (not P2P)") + + # Reconnect and verify everything is consistent + self.reconnect_isolated_node(4, 0) + self.sync_blocks() + if __name__ == '__main__': LLMQChainLocksTest().main()