diff --git a/src/chainlock/chainlock.cpp b/src/chainlock/chainlock.cpp index 1b376e90cea8..ed95c41a658f 100644 --- a/src/chainlock/chainlock.cpp +++ b/src/chainlock/chainlock.cpp @@ -171,4 +171,30 @@ void Chainlocks::AcceptedBlockHeader(gsl::not_null pindexNew bestChainLockBlockIndex = pindexNew; } } + +void Chainlocks::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; + } + + pendingCoinbaseChainLocks.push_back(clsig); +} + +std::vector Chainlocks::DrainPendingCoinbaseChainLocks() +{ + LOCK(cs); + // O(1) zero-copy swap rather than per-element copy. + std::vector drained; + drained.swap(pendingCoinbaseChainLocks); + return drained; +} + } // namespace chainlock diff --git a/src/chainlock/chainlock.h b/src/chainlock/chainlock.h index 3ee131264ad2..6f8e14688ecd 100644 --- a/src/chainlock/chainlock.h +++ b/src/chainlock/chainlock.h @@ -11,6 +11,8 @@ #include #include +#include + class CBlockIndex; class CSporkManager; class uint256; @@ -57,6 +59,9 @@ class Chainlocks chainlock::ChainLockSig bestChainLockWithKnownBlock GUARDED_BY(cs); + // Queue for coinbase chainlocks pending asynchronous processing by ChainlockHandler + std::vector pendingCoinbaseChainLocks GUARDED_BY(cs); + public: Chainlocks(const CSporkManager& sporkman); @@ -82,6 +87,13 @@ class Chainlocks void AcceptedBlockHeader(gsl::not_null pindexNew) EXCLUSIVE_LOCKS_REQUIRED(!cs); void ResetChainlock() EXCLUSIVE_LOCKS_REQUIRED(!cs); + + // Queue a coinbase chainlock for asynchronous processing by the ChainlockHandler. + // Called during block validation to avoid blocking the main validation flow. + void QueueCoinbaseChainLock(const chainlock::ChainLockSig& clsig) EXCLUSIVE_LOCKS_REQUIRED(!cs); + + // Drain pending coinbase chainlocks for processing by ChainlockHandler. + std::vector DrainPendingCoinbaseChainLocks() EXCLUSIVE_LOCKS_REQUIRED(!cs); }; } // namespace chainlock diff --git a/src/chainlock/handler.cpp b/src/chainlock/handler.cpp index b62a92b3d39c..1ea5e27ac5ac 100644 --- a/src/chainlock/handler.cpp +++ b/src/chainlock/handler.cpp @@ -56,17 +56,46 @@ ChainlockHandler::~ChainlockHandler() scheduler_thread->join(); } -void ChainlockHandler::Start() +void ChainlockHandler::Start(const llmq::CQuorumManager& qman) { scheduler->scheduleEvery( [&]() { CheckActiveState(); + ProcessPendingCoinbaseChainLocks(qman); EnforceBestChainLock(); Cleanup(); }, std::chrono::seconds{5}); } +void ChainlockHandler::ProcessPendingCoinbaseChainLocks(const llmq::CQuorumManager& qman) +{ + AssertLockNotHeld(cs); + AssertLockNotHeld(cs_main); + + if (!isEnabled) { + return; + } + + auto pending = m_chainlocks.DrainPendingCoinbaseChainLocks(); + // Process newest first (LIFO): once a newer chainlock is accepted, older + // ones short-circuit on the cheap height check below. + for (auto it = pending.rbegin(); it != pending.rend(); ++it) { + const auto& clsig = *it; + // Cheap height check before computing a hash; during reindex this + // skips ~all queued entries without paying for SHA256. + if (clsig.getHeight() <= m_chainlocks.GetBestChainLockHeight()) { + continue; + } + const uint256 hash = ::SerializeHash(clsig); + if (WITH_LOCK(cs, return seenChainLocks.count(hash) != 0)) { + continue; + } + // Process as if we discovered it locally (from = -1 means internal/coinbase). + (void)ProcessNewChainLock(-1, clsig, qman, hash); + } +} + void ChainlockHandler::Stop() { scheduler->stop(); } bool ChainlockHandler::AlreadyHave(const CInv& inv) const diff --git a/src/chainlock/handler.h b/src/chainlock/handler.h index 140325c007c6..5bbed0c26346 100644 --- a/src/chainlock/handler.h +++ b/src/chainlock/handler.h @@ -68,7 +68,7 @@ class ChainlockHandler final : public CValidationInterface const CMasternodeSync& mn_sync); ~ChainlockHandler(); - void Start(); + void Start(const llmq::CQuorumManager& qman); void Stop(); bool AlreadyHave(const CInv& inv) const EXCLUSIVE_LOCKS_REQUIRED(!cs); @@ -100,6 +100,7 @@ class ChainlockHandler final : public CValidationInterface private: void Cleanup() EXCLUSIVE_LOCKS_REQUIRED(!cs); + void ProcessPendingCoinbaseChainLocks(const llmq::CQuorumManager& qman) EXCLUSIVE_LOCKS_REQUIRED(!cs); }; } // namespace chainlock diff --git a/src/evo/chainhelper.cpp b/src/evo/chainhelper.cpp index 63c6fdafd2a7..2694ca80a6f6 100644 --- a/src/evo/chainhelper.cpp +++ b/src/evo/chainhelper.cpp @@ -18,7 +18,7 @@ CChainstateHelper::CChainstateHelper(CEvoDB& evodb, CDeterministicMNManager& dmn llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync, - const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) : + chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) : isman{isman}, credit_pool_manager{std::make_unique(evodb, chainman)}, m_chainlocks{chainlocks}, diff --git a/src/evo/chainhelper.h b/src/evo/chainhelper.h index ec129d30df2b..7a9e4c2ccf00 100644 --- a/src/evo/chainhelper.h +++ b/src/evo/chainhelper.h @@ -41,7 +41,7 @@ class CChainstateHelper public: const std::unique_ptr credit_pool_manager; - const chainlock::Chainlocks& m_chainlocks; + chainlock::Chainlocks& m_chainlocks; const std::unique_ptr ehf_manager; const std::unique_ptr mn_payments; const std::unique_ptr special_tx; @@ -54,7 +54,7 @@ class CChainstateHelper llmq::CInstantSendManager& isman, llmq::CQuorumBlockProcessor& qblockman, llmq::CQuorumSnapshotManager& qsnapman, const ChainstateManager& chainman, const Consensus::Params& consensus_params, const CMasternodeSync& mn_sync, - const chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman); + chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman); ~CChainstateHelper(); /** Passthrough functions to chainlock::Chainlocks */ diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 65b1e6c4e2c6..27417113158a 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -667,6 +667,17 @@ 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_chainlocks.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 2d5ab32096bb..56279e9e371a 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -47,14 +47,14 @@ class CSpecialTxProcessor llmq::CQuorumSnapshotManager& m_qsnapman; const ChainstateManager& m_chainman; const Consensus::Params& m_consensus_params; - const chainlock::Chainlocks& m_chainlocks; + chainlock::Chainlocks& m_chainlocks; 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 chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) : + chainlock::Chainlocks& chainlocks, const llmq::CQuorumManager& qman) : m_cpoolman(cpoolman), m_dmnman{dmnman}, m_mnhfman{mnhfman}, diff --git a/src/init.cpp b/src/init.cpp index 113bd4c9f025..04af96a07d3d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2338,7 +2338,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 10a: schedule Dash-specific tasks node.peerman->StartHandlers(); - node.clhandler->Start(); + node.clhandler->Start(*node.llmq_ctx->qman); node.scheduler->scheduleEvery(std::bind(&CNetFulfilledRequestManager::DoMaintenance, std::ref(*node.netfulfilledman)), std::chrono::minutes{1}); node.scheduler->scheduleEvery(std::bind(&CMasternodeUtils::DoMaintenance, std::ref(*node.connman), std::ref(*node.dmnman), std::ref(*node.mn_sync), node.cj_walletman.get()), std::chrono::minutes{1}); diff --git a/src/test/llmq_chainlock_tests.cpp b/src/test/llmq_chainlock_tests.cpp index b718cba9a342..69857f193b8b 100644 --- a/src/test/llmq_chainlock_tests.cpp +++ b/src/test/llmq_chainlock_tests.cpp @@ -8,7 +8,7 @@ #include #include -#include +#include #include diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index 44861b622dff..84dee9ca4b79 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 @@ -254,6 +254,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) @@ -357,6 +360,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()