From 1485fd4e0a6b76f13e530bcc89063ef08aea36f6 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Thu, 2 Mar 2023 21:37:21 +0200 Subject: [PATCH 01/18] best CL sig in cbtx --- src/evo/cbtx.cpp | 58 +++++++++++++++++-- src/evo/cbtx.h | 16 +++++ src/evo/specialtxman.cpp | 7 ++- src/evo/specialtxman.h | 3 +- src/llmq/chainlocks.cpp | 10 +++- src/llmq/chainlocks.h | 4 +- src/llmq/context.cpp | 2 +- src/miner.cpp | 12 +++- src/validation.cpp | 4 +- test/functional/feature_llmq_chainlocks.py | 3 + test/functional/test_framework/messages.py | 17 +++++- .../test_framework/test_framework.py | 3 + 12 files changed, 125 insertions(+), 14 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 81fefe7853f9..c212c432690f 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -14,6 +15,7 @@ #include #include #include +#include bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state) { @@ -30,8 +32,16 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload"); } - if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CURRENT_VERSION) { - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); + if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CB_CL_SIG_VERSION) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); + } + + if (pindexPrev && !llmq::utils::IsV20Active(pindexPrev) && cbTx.nVersion == CCbTx::CB_CL_SIG_VERSION) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); + } + + if (pindexPrev && llmq::utils::IsV20Active(pindexPrev) && cbTx.nVersion != CCbTx::CB_CL_SIG_VERSION) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); } if (pindexPrev && pindexPrev->nHeight + 1 != cbTx.nHeight) { @@ -314,8 +324,48 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre return true; } +bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, CValidationState& state) +{ + if (block.vtx[0]->nType != TRANSACTION_COINBASE) { + return true; + } + + CCbTx cbTx; + if (!GetTxPayload(*block.vtx[0], cbTx)) { + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-payload"); + } + + if (cbTx.nVersion >= CCbTx::CB_CL_SIG_VERSION) { + if (cbTx.bestCLSignature.IsValid()) { + int bestChainLockedHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + uint256 bestChainLockedHash = ::ChainActive()[bestChainLockedHeight]->GetBlockHash(); + if (!chainlock_handler.VerifyChainLock(bestChainLockedHeight, bestChainLockedHash, cbTx.bestCLSignature )){ + return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-clsig"); + } + } + + } + + return true; +} + +bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) +{ + if (chainlock_handler.GetBestChainLock().IsNull()) { + bestCLHeightDiff = 0; + bestCLSignature.Reset(); + return false; + } + + bestCLHeightDiff = (static_cast(nHeight) - chainlock_handler.GetBestChainLock().getHeight() - 1); + bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); + + return true; +} + + std::string CCbTx::ToString() const { - return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s)", - nVersion, nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString()); + return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s)", + nVersion, nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString()); } diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 1b64ef2036a5..8a295f26fcb2 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_EVO_CBTX_H #define BITCOIN_EVO_CBTX_H +#include #include #include @@ -16,6 +17,7 @@ class TxValidationState; namespace llmq { class CQuorumBlockProcessor; +class CChainLocksHandler; }// namespace llmq // coinbase transaction @@ -24,11 +26,14 @@ class CCbTx public: static constexpr auto SPECIALTX_TYPE = TRANSACTION_COINBASE; static constexpr uint16_t CURRENT_VERSION = 2; + static constexpr uint16_t CB_CL_SIG_VERSION = 3; uint16_t nVersion{CURRENT_VERSION}; int32_t nHeight{0}; uint256 merkleRootMNList; uint256 merkleRootQuorums; + uint32_t bestCLHeightDiff; + CBLSSignature bestCLSignature; SERIALIZE_METHODS(CCbTx, obj) { @@ -36,6 +41,10 @@ class CCbTx if (obj.nVersion >= 2) { READWRITE(obj.merkleRootQuorums); + if (obj.nVersion >= CB_CL_SIG_VERSION) { + READWRITE(COMPACTSIZE(obj.bestCLHeightDiff)); + READWRITE(obj.bestCLSignature); + } } } @@ -50,6 +59,10 @@ class CCbTx obj.pushKV("merkleRootMNList", merkleRootMNList.ToString()); if (nVersion >= 2) { obj.pushKV("merkleRootQuorums", merkleRootQuorums.ToString()); + if (nVersion >= CB_CL_SIG_VERSION) { + obj.pushKV("bestCLHeightDiff", static_cast(bestCLHeightDiff)); + obj.pushKV("bestCLSignature", bestCLSignature.ToString()); + } } } }; @@ -59,5 +72,8 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, const llmq::CQuorumBlockProcessor& quorum_block_processor, BlockValidationState& state, const CCoinsViewCache& view); bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet, BlockValidationState& state, const CCoinsViewCache& view); bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); +bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state); + +bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature); #endif // BITCOIN_EVO_CBTX_H diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index e3837c50f0fe..2dd01ad3bd37 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -98,7 +98,7 @@ bool UndoSpecialTx(const CTransaction& tx, const CBlockIndex* pindex) return false; } -bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, +bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots) { AssertLockHeld(cs_main); @@ -154,6 +154,11 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll return false; } + if (!CheckCbTxBestChainlock(block, pindex, chainlock_handler, state)) { + // pass the state returned by the function above + return false; + } + int64_t nTime5 = GetTimeMicros(); nTimeMerkle += nTime5 - nTime4; LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index ed8d4b5c3140..318dbb5784c8 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -16,12 +16,13 @@ class CCoinsViewCache; class TxValidationState; namespace llmq { class CQuorumBlockProcessor; +class CChainLocksHandler; } // namespace llmq extern CCriticalSection cs_main; bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, +bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor) EXCLUSIVE_LOCKS_REQUIRED(cs_main); diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index 7a73939daf0b..9cd84cb067e4 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -24,12 +24,13 @@ namespace llmq { std::unique_ptr chainLocksHandler; -CChainLocksHandler::CChainLocksHandler(CTxMemPool& _mempool, CConnman& _connman, CSporkManager& sporkManager, CSigningManager& _sigman, CSigSharesManager& _shareman, const std::unique_ptr& mn_sync) : +CChainLocksHandler::CChainLocksHandler(CTxMemPool& _mempool, CConnman& _connman, CSporkManager& sporkManager, CSigningManager& _sigman, CSigSharesManager& _shareman, CQuorumManager& _qman, const std::unique_ptr& mn_sync) : connman(_connman), mempool(_mempool), spork_manager(sporkManager), sigman(_sigman), shareman(_shareman), + qman(_qman), m_mn_sync(mn_sync), scheduler(std::make_unique()), scheduler_thread(std::make_unique([&] { TraceThread("cl-schdlr", [&] { scheduler->serviceQueue(); }); })) @@ -557,6 +558,13 @@ bool CChainLocksHandler::HasChainLock(int nHeight, const uint256& blockHash) con return InternalHasChainLock(nHeight, blockHash); } +bool CChainLocksHandler::VerifyChainLock(int nHeight, const uint256& blockHash, const CBLSSignature& sig) const +{ + const auto llmqType = Params().GetConsensus().llmqTypeChainLocks; + const uint256 nRequestId = ::SerializeHash(std::make_pair(llmq::CLSIG_REQUESTID_PREFIX, nHeight)); + return llmq::CSigningManager::VerifyRecoveredSig(llmqType, qman, nHeight, nRequestId, blockHash, sig); +} + bool CChainLocksHandler::InternalHasChainLock(int nHeight, const uint256& blockHash) const { AssertLockHeld(cs); diff --git a/src/llmq/chainlocks.h b/src/llmq/chainlocks.h index 61d4c92ad9b1..7da2c72b57bd 100644 --- a/src/llmq/chainlocks.h +++ b/src/llmq/chainlocks.h @@ -46,6 +46,7 @@ class CChainLocksHandler : public CRecoveredSigsListener CSporkManager& spork_manager; CSigningManager& sigman; CSigSharesManager& shareman; + CQuorumManager& qman; const std::unique_ptr& m_mn_sync; std::unique_ptr scheduler; std::unique_ptr scheduler_thread; @@ -79,7 +80,7 @@ class CChainLocksHandler : public CRecoveredSigsListener int64_t lastCleanupTime GUARDED_BY(cs) {0}; public: - explicit CChainLocksHandler(CTxMemPool& _mempool, CConnman& _connman, CSporkManager& sporkManager, CSigningManager& _sigman, CSigSharesManager& _shareman, const std::unique_ptr& mn_sync); + explicit CChainLocksHandler(CTxMemPool& _mempool, CConnman& _connman, CSporkManager& sporkManager, CSigningManager& _sigman, CSigSharesManager& _shareman, CQuorumManager& _qman, const std::unique_ptr& mn_sync); ~CChainLocksHandler(); void Start(); @@ -103,6 +104,7 @@ class CChainLocksHandler : public CRecoveredSigsListener bool HasChainLock(int nHeight, const uint256& blockHash) const LOCKS_EXCLUDED(cs); bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const LOCKS_EXCLUDED(cs); + bool VerifyChainLock(int nHeight, const uint256& blockHash, const CBLSSignature& sig) const; bool IsTxSafeForMining(const CInstantSendManager& isman, const uint256& txid) const LOCKS_EXCLUDED(cs); diff --git a/src/llmq/context.cpp b/src/llmq/context.cpp index 27f92e43fae5..b188aac29470 100644 --- a/src/llmq/context.cpp +++ b/src/llmq/context.cpp @@ -47,7 +47,7 @@ void LLMQContext::Create(CEvoDB& evoDb, CTxMemPool& mempool, CConnman& connman, llmq::quorumManager = std::make_unique(evoDb, connman, *bls_worker, *llmq::quorumBlockProcessor, *qdkgsman, ::masternodeSync); sigman = std::make_unique(connman, *llmq::quorumManager, unitTests, fWipe); shareman = std::make_unique(connman, *llmq::quorumManager, *sigman); - llmq::chainLocksHandler = std::make_unique(mempool, connman, sporkManager, *sigman, *shareman, ::masternodeSync); + llmq::chainLocksHandler = std::make_unique(mempool, connman, sporkManager, *sigman, *shareman, *llmq::quorumManager, ::masternodeSync); llmq::quorumInstantSendManager = std::make_unique(mempool, connman, sporkManager, *llmq::quorumManager, *sigman, *shareman, *llmq::chainLocksHandler, ::masternodeSync, unitTests, fWipe); // NOTE: we use this only to wipe the old db, do NOT use it for anything else diff --git a/src/miner.cpp b/src/miner.cpp index 9673c8a2bc26..a1ea16832f87 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -197,7 +197,9 @@ std::unique_ptr BlockAssembler::CreateNewBlock(CChainState& chai CCbTx cbTx; - if (fDIP0008Active_context) { + if (llmq::utils::IsV20Active(pindexPrev)) { + cbTx.nVersion = CCbTx::CB_CL_SIG_VERSION; + } else if (fDIP0008Active_context) { cbTx.nVersion = 2; } else { cbTx.nVersion = 1; @@ -213,6 +215,14 @@ std::unique_ptr BlockAssembler::CreateNewBlock(CChainState& chai if (!CalcCbTxMerkleRootQuorums(*pblock, pindexPrev, quorum_block_processor, cbTx.merkleRootQuorums, state)) { throw std::runtime_error(strprintf("%s: CalcCbTxMerkleRootQuorums failed: %s", __func__, FormatStateMessage(state))); } + if (llmq::utils::IsV20Active(pindexPrev)) { + if (!EmplaceBestChainlock(m_clhandler, nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { + LogPrintf("CreateNewBlock() height[%d] CBTx failed to find best CL.\n", nHeight); + } + else { + LogPrintf("CreateNewBlock() height[%d] CBTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); + } + } } SetTxPayload(coinbaseTx, cbTx); diff --git a/src/validation.cpp b/src/validation.cpp index f48f81cb9c2b..7b0c7e410988 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2258,7 +2258,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, bool fDIP0001Active_context = pindex->nHeight >= Params().GetConsensus().DIP0001Height; // MUST process special txes before updating UTXO to ensure consistency between mempool and block processing - if (!ProcessSpecialTxsInBlock(block, pindex, *m_quorum_block_processor, state, view, fJustCheck, fScriptChecks)) { + if (!ProcessSpecialTxsInBlock(block, pindex, *m_quorum_block_processor, *m_clhandler, state, view, fJustCheck, fScriptChecks)) { return error("ConnectBlock(DASH): ProcessSpecialTxsInBlock for block %s failed with %s", pindex->GetBlockHash().ToString(), FormatStateMessage(state)); } @@ -4888,7 +4888,7 @@ bool CChainState::RollforwardBlock(const CBlockIndex* pindex, CCoinsViewCache& i // MUST process special txes before updating UTXO to ensure consistency between mempool and block processing BlockValidationState state; - if (!ProcessSpecialTxsInBlock(block, pindex, *m_quorum_block_processor, state, inputs, false /*fJustCheck*/, false /*fScriptChecks*/)) { + if (!ProcessSpecialTxsInBlock(block, pindex, *m_quorum_block_processor, *m_clhandler, state, inputs, false /*fJustCheck*/, false /*fScriptChecks*/)) { return error("RollforwardBlock(DASH): ProcessSpecialTxsInBlock for block %s failed with %s", pindex->GetBlockHash().ToString(), FormatStateMessage(state)); } diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index b3b1fc822a72..a456f4d45585 100755 --- a/test/functional/feature_llmq_chainlocks.py +++ b/test/functional/feature_llmq_chainlocks.py @@ -34,6 +34,9 @@ def run_test(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) self.wait_for_sporks_same() + self.activate_by_name('v20', expected_activation_height=904) + self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount())) + self.log.info("Mining 4 quorums") for i in range(4): self.mine_quorum() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 055b9c44e181..5fc2df1a6c12 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1046,9 +1046,9 @@ def __repr__(self): class CCbTx: - __slots__ = ("version", "height", "merkleRootMNList", "merkleRootQuorums") + __slots__ = ("version", "height", "merkleRootMNList", "merkleRootQuorums", "bestCLHeight", "bestCLSignature") - def __init__(self, version=None, height=None, merkleRootMNList=None, merkleRootQuorums=None): + def __init__(self, version=None, height=None, merkleRootMNList=None, merkleRootQuorums=None, bestCLHeight=None, bestCLSignature=None): self.set_null() if version is not None: self.version = version @@ -1058,11 +1058,17 @@ def __init__(self, version=None, height=None, merkleRootMNList=None, merkleRootQ self.merkleRootMNList = merkleRootMNList if merkleRootQuorums is not None: self.merkleRootQuorums = merkleRootQuorums + if bestCLHeight is not None: + self.bestCLHeight = bestCLHeight + if bestCLSignature is not None: + self.bestCLSignature = bestCLSignature def set_null(self): self.version = 0 self.height = 0 self.merkleRootMNList = None + self.bestCLHeight = 0 + self.bestCLSignature = b'\x00' * 96 def deserialize(self, f): self.version = struct.unpack("= 2: self.merkleRootQuorums = deser_uint256(f) + if self.version >= 3: + self.bestCLHeight = deser_compact_size(f) + self.bestCLSignature = f.read(96) + def serialize(self): r = b"" @@ -1078,6 +1088,9 @@ def serialize(self): r += ser_uint256(self.merkleRootMNList) if self.version >= 2: r += ser_uint256(self.merkleRootQuorums) + if self.version >= 3: + r += ser_compact_size(self.bestCLHeight) + r += self.bestCLSignature return r diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8741b59f0050..ab8a8d3d14e2 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1072,6 +1072,9 @@ def activate_dip0024(self, expected_activation_height=None): def activate_v19(self, expected_activation_height=None): self.activate_by_name('v19', expected_activation_height) + def activate_v20(self, expected_activation_height=None): + self.activate_by_name('v20', expected_activation_height) + def set_dash_llmq_test_params(self, llmq_size, llmq_threshold): self.llmq_size = llmq_size self.llmq_threshold = llmq_threshold From 531c7c58e28272206aa4253794503ab5dc04e6c9 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Thu, 2 Mar 2023 21:55:55 +0200 Subject: [PATCH 02/18] const ref --- src/evo/specialtxman.cpp | 2 +- src/evo/specialtxman.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 2dd01ad3bd37..b33e8e9517c4 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -98,7 +98,7 @@ bool UndoSpecialTx(const CTransaction& tx, const CBlockIndex* pindex) return false; } -bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, llmq::CChainLocksHandler& chainlock_handler, +bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots) { AssertLockHeld(cs_main); diff --git a/src/evo/specialtxman.h b/src/evo/specialtxman.h index 318dbb5784c8..011bc4c5c7f8 100644 --- a/src/evo/specialtxman.h +++ b/src/evo/specialtxman.h @@ -22,7 +22,7 @@ class CChainLocksHandler; extern CCriticalSection cs_main; bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state, const CCoinsViewCache& view, bool check_sigs) EXCLUSIVE_LOCKS_REQUIRED(cs_main); -bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, llmq::CChainLocksHandler& chainlock_handler, +bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state, const CCoinsViewCache& view, bool fJustCheck, bool fCheckCbTxMerleRoots) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, llmq::CQuorumBlockProcessor& quorum_block_processor) EXCLUSIVE_LOCKS_REQUIRED(cs_main); From 5ceb71b2c5b7a402482bbcc776f90cff113a0b95 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Mon, 17 Apr 2023 13:10:06 +0300 Subject: [PATCH 03/18] refactoring --- src/evo/cbtx.cpp | 10 ++++------ src/evo/cbtx.h | 4 ++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index c212c432690f..1bfa87a8ba33 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -36,12 +36,10 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); } - if (pindexPrev && !llmq::utils::IsV20Active(pindexPrev) && cbTx.nVersion == CCbTx::CB_CL_SIG_VERSION) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); - } - - if (pindexPrev && llmq::utils::IsV20Active(pindexPrev) && cbTx.nVersion != CCbTx::CB_CL_SIG_VERSION) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); + if (pindexPrev) { + bool isV20 = llmq::utils::IsV20Active(pindexPrev); + bool isCbV20 = cbTx.nVersion != CCbTx::CB_CL_SIG_VERSION; + if (isV20 != isCbV20) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); } if (pindexPrev && pindexPrev->nHeight + 1 != cbTx.nHeight) { diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 8a295f26fcb2..24b49d027ccb 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -25,10 +25,10 @@ class CCbTx { public: static constexpr auto SPECIALTX_TYPE = TRANSACTION_COINBASE; - static constexpr uint16_t CURRENT_VERSION = 2; + static constexpr uint16_t CB_V19_VERSION = 2; static constexpr uint16_t CB_CL_SIG_VERSION = 3; - uint16_t nVersion{CURRENT_VERSION}; + uint16_t nVersion{CB_V19_VERSION}; int32_t nHeight{0}; uint256 merkleRootMNList; uint256 merkleRootQuorums; From 7e002b639abadd4f901f14526f27c1ef7b8c5255 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Mon, 17 Apr 2023 13:49:25 +0300 Subject: [PATCH 04/18] fix --- src/evo/cbtx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 1bfa87a8ba33..292fd67ca05a 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -38,7 +38,7 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati if (pindexPrev) { bool isV20 = llmq::utils::IsV20Active(pindexPrev); - bool isCbV20 = cbTx.nVersion != CCbTx::CB_CL_SIG_VERSION; + bool isCbV20 = cbTx.nVersion == CCbTx::CB_CL_SIG_VERSION; if (isV20 != isCbV20) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); } From cce11841fb4a99d33e7011ec3975db666a0ffe7b Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Mon, 17 Apr 2023 23:48:33 +0300 Subject: [PATCH 05/18] Rebase adjustements --- src/evo/cbtx.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 292fd67ca05a..c630c3488108 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -33,13 +33,13 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati } if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CB_CL_SIG_VERSION) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } if (pindexPrev) { bool isV20 = llmq::utils::IsV20Active(pindexPrev); bool isCbV20 = cbTx.nVersion == CCbTx::CB_CL_SIG_VERSION; - if (isV20 != isCbV20) return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-version"); + if (isV20 != isCbV20) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } if (pindexPrev && pindexPrev->nHeight + 1 != cbTx.nHeight) { @@ -322,7 +322,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre return true; } -bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, CValidationState& state) +bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state) { if (block.vtx[0]->nType != TRANSACTION_COINBASE) { return true; @@ -330,7 +330,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, CCbTx cbTx; if (!GetTxPayload(*block.vtx[0], cbTx)) { - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-payload"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } if (cbTx.nVersion >= CCbTx::CB_CL_SIG_VERSION) { @@ -338,7 +338,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, int bestChainLockedHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; uint256 bestChainLockedHash = ::ChainActive()[bestChainLockedHeight]->GetBlockHash(); if (!chainlock_handler.VerifyChainLock(bestChainLockedHeight, bestChainLockedHash, cbTx.bestCLSignature )){ - return state.Invalid(ValidationInvalidReason::CONSENSUS, false, REJECT_INVALID, "bad-cbtx-clsig"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } } From 9a5648ce06bea0ff14a9c54eb02d668e4ad47380 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Wed, 19 Apr 2023 17:26:23 +0300 Subject: [PATCH 06/18] tests and refactoring --- src/evo/cbtx.cpp | 12 ++++++--- src/evo/cbtx.h | 10 +++---- src/miner.cpp | 4 +-- test/functional/feature_llmq_chainlocks.py | 31 +++++++++++++++++++++- test/functional/test_framework/messages.py | 14 +++++----- 5 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index c630c3488108..0cf4510ae6f4 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -32,13 +32,13 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-payload"); } - if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CB_CL_SIG_VERSION) { + if (cbTx.nVersion == 0 || cbTx.nVersion > CCbTx::CB_V20_VERSION) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } if (pindexPrev) { bool isV20 = llmq::utils::IsV20Active(pindexPrev); - bool isCbV20 = cbTx.nVersion == CCbTx::CB_CL_SIG_VERSION; + bool isCbV20 = cbTx.nVersion == CCbTx::CB_V20_VERSION; if (isV20 != isCbV20) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } @@ -333,7 +333,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } - if (cbTx.nVersion >= CCbTx::CB_CL_SIG_VERSION) { + if (cbTx.nVersion >= CCbTx::CB_V20_VERSION) { if (cbTx.bestCLSignature.IsValid()) { int bestChainLockedHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; uint256 bestChainLockedHash = ::ChainActive()[bestChainLockedHeight]->GetBlockHash(); @@ -341,6 +341,12 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } } + else { + if (cbTx.bestCLHeightDiff != 0) { + // Null bestCLSignature (IsNull() doesn't exist: we assume that a non valid BLS sig is null) is allowed only when bestCLHeightDiff is 0 + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); + } + } } diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 24b49d027ccb..8c5b1c62820a 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -26,7 +26,7 @@ class CCbTx public: static constexpr auto SPECIALTX_TYPE = TRANSACTION_COINBASE; static constexpr uint16_t CB_V19_VERSION = 2; - static constexpr uint16_t CB_CL_SIG_VERSION = 3; + static constexpr uint16_t CB_V20_VERSION = 3; uint16_t nVersion{CB_V19_VERSION}; int32_t nHeight{0}; @@ -39,9 +39,9 @@ class CCbTx { READWRITE(obj.nVersion, obj.nHeight, obj.merkleRootMNList); - if (obj.nVersion >= 2) { + if (obj.nVersion >= CB_V19_VERSION) { READWRITE(obj.merkleRootQuorums); - if (obj.nVersion >= CB_CL_SIG_VERSION) { + if (obj.nVersion >= CB_V20_VERSION) { READWRITE(COMPACTSIZE(obj.bestCLHeightDiff)); READWRITE(obj.bestCLSignature); } @@ -57,9 +57,9 @@ class CCbTx obj.pushKV("version", (int)nVersion); obj.pushKV("height", nHeight); obj.pushKV("merkleRootMNList", merkleRootMNList.ToString()); - if (nVersion >= 2) { + if (nVersion >= CB_V19_VERSION) { obj.pushKV("merkleRootQuorums", merkleRootQuorums.ToString()); - if (nVersion >= CB_CL_SIG_VERSION) { + if (nVersion >= CB_V20_VERSION) { obj.pushKV("bestCLHeightDiff", static_cast(bestCLHeightDiff)); obj.pushKV("bestCLSignature", bestCLSignature.ToString()); } diff --git a/src/miner.cpp b/src/miner.cpp index a1ea16832f87..b52b251e5cd7 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -198,9 +198,9 @@ std::unique_ptr BlockAssembler::CreateNewBlock(CChainState& chai CCbTx cbTx; if (llmq::utils::IsV20Active(pindexPrev)) { - cbTx.nVersion = CCbTx::CB_CL_SIG_VERSION; + cbTx.nVersion = CCbTx::CB_V20_VERSION; } else if (fDIP0008Active_context) { - cbTx.nVersion = 2; + cbTx.nVersion = CCbTx::CB_V19_VERSION; } else { cbTx.nVersion = 1; } diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index a456f4d45585..8a931e86b400 100755 --- a/test/functional/feature_llmq_chainlocks.py +++ b/test/functional/feature_llmq_chainlocks.py @@ -13,7 +13,7 @@ import time from test_framework.test_framework import DashTestFramework -from test_framework.util import force_finish_mnsync +from test_framework.util import force_finish_mnsync, assert_equal class LLMQChainLocksTest(DashTestFramework): @@ -34,6 +34,8 @@ def run_test(self): self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) self.wait_for_sporks_same() + self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False) + self.activate_by_name('v20', expected_activation_height=904) self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount())) @@ -44,11 +46,13 @@ def run_test(self): self.log.info("Mine single block, wait for chainlock") self.nodes[0].generate(1) self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash()) + self.test_coinbase_best_cl(self.nodes[0]) self.log.info("Mine many blocks, wait for chainlock") self.nodes[0].generate(20) # We need more time here due to 20 blocks being generated at once self.wait_for_chainlocked_block_all_nodes(self.nodes[0].getbestblockhash(), timeout=30) + self.test_coinbase_best_cl(self.nodes[0]) self.log.info("Assert that all blocks up until the tip are chainlocked") for h in range(1, self.nodes[0].getblockcount()): @@ -61,10 +65,12 @@ def run_test(self): node0_tip = self.nodes[0].getbestblockhash() self.nodes[1].generatetoaddress(5, node0_mining_addr) self.wait_for_chainlocked_block(self.nodes[1], self.nodes[1].getbestblockhash()) + self.test_coinbase_best_cl(self.nodes[0]) assert self.nodes[0].getbestblockhash() == node0_tip self.reconnect_isolated_node(0, 1) self.nodes[1].generatetoaddress(1, node0_mining_addr) self.wait_for_chainlocked_block_all_nodes(self.nodes[1].getbestblockhash()) + self.test_coinbase_best_cl(self.nodes[0]) self.log.info("Isolate node, mine on both parts of the network, and reconnect") self.isolate_node(0) @@ -76,6 +82,7 @@ def run_test(self): self.reconnect_isolated_node(0, 1) self.nodes[1].generatetoaddress(1, node0_mining_addr) self.wait_for_chainlocked_block_all_nodes(self.nodes[1].getbestblockhash()) + self.test_coinbase_best_cl(self.nodes[0]) assert self.nodes[0].getblock(self.nodes[0].getbestblockhash())["previousblockhash"] == good_tip assert self.nodes[1].getblock(self.nodes[1].getbestblockhash())["previousblockhash"] == good_tip @@ -110,6 +117,7 @@ def run_test(self): good_fork = good_tip good_tip = self.nodes[1].generatetoaddress(1, node0_mining_addr)[-1] # this should mark bad_tip as conflicting self.wait_for_chainlocked_block_all_nodes(good_tip) + self.test_coinbase_best_cl(self.nodes[0]) assert self.nodes[0].getbestblockhash() == good_tip found = False for tip in self.nodes[0].getchaintips(2): @@ -176,6 +184,27 @@ def create_chained_txs(self, node, amount): return [txid, rawtxid] + def test_coinbase_best_cl(self, node, expected_cl_in_cb=True): + block_hash = node.getbestblockhash() + block = node.getblock(block_hash, 2) + cbtx = block["cbTx"] + assert_equal(int(cbtx["version"]) > 2, expected_cl_in_cb) + self.log.info("cbtx: "+str(cbtx)) + if expected_cl_in_cb: + cb_height = int(cbtx["height"]) + best_cl_height_diff = int(cbtx["bestCLHeightDiff"]) + best_cl_signature = cbtx["bestCLSignature"] + if int(best_cl_signature, 16) == 0: + # Null bestCLSignature is allowed. + # bestCLHeightDiff must be 0 if bestCLSignature is null + assert_equal(best_cl_height_diff, 0) + # Returning as no more tests can be conducted + return + best_cl_height = cb_height - best_cl_height_diff - 1 + target_block_hash = node.getblockhash(best_cl_height) + # Verify CL signature + assert(node.verifychainlock(target_block_hash, best_cl_signature, best_cl_height)) + if __name__ == '__main__': LLMQChainLocksTest().main() diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 5fc2df1a6c12..fbe32a8b6607 100755 --- a/test/functional/test_framework/messages.py +++ b/test/functional/test_framework/messages.py @@ -1046,9 +1046,9 @@ def __repr__(self): class CCbTx: - __slots__ = ("version", "height", "merkleRootMNList", "merkleRootQuorums", "bestCLHeight", "bestCLSignature") + __slots__ = ("version", "height", "merkleRootMNList", "merkleRootQuorums", "bestCLHeightDiff", "bestCLSignature") - def __init__(self, version=None, height=None, merkleRootMNList=None, merkleRootQuorums=None, bestCLHeight=None, bestCLSignature=None): + def __init__(self, version=None, height=None, merkleRootMNList=None, merkleRootQuorums=None, bestCLHeightDiff=None, bestCLSignature=None): self.set_null() if version is not None: self.version = version @@ -1058,8 +1058,8 @@ def __init__(self, version=None, height=None, merkleRootMNList=None, merkleRootQ self.merkleRootMNList = merkleRootMNList if merkleRootQuorums is not None: self.merkleRootQuorums = merkleRootQuorums - if bestCLHeight is not None: - self.bestCLHeight = bestCLHeight + if bestCLHeightDiff is not None: + self.bestCLHeightDiff = bestCLHeightDiff if bestCLSignature is not None: self.bestCLSignature = bestCLSignature @@ -1067,7 +1067,7 @@ def set_null(self): self.version = 0 self.height = 0 self.merkleRootMNList = None - self.bestCLHeight = 0 + self.bestCLHeightDiff = 0 self.bestCLSignature = b'\x00' * 96 def deserialize(self, f): @@ -1077,7 +1077,7 @@ def deserialize(self, f): if self.version >= 2: self.merkleRootQuorums = deser_uint256(f) if self.version >= 3: - self.bestCLHeight = deser_compact_size(f) + self.bestCLHeightDiff = deser_compact_size(f) self.bestCLSignature = f.read(96) @@ -1089,7 +1089,7 @@ def serialize(self): if self.version >= 2: r += ser_uint256(self.merkleRootQuorums) if self.version >= 3: - r += ser_compact_size(self.bestCLHeight) + r += ser_compact_size(self.bestCLHeightDiff) r += self.bestCLSignature return r From 2a502268f28f989c6ac842baef7a0f6b5fe1869e Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Thu, 20 Apr 2023 18:51:43 +0300 Subject: [PATCH 07/18] Added expected_null_cl --- test/functional/feature_llmq_chainlocks.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index 8a931e86b400..eb5164b78c72 100755 --- a/test/functional/feature_llmq_chainlocks.py +++ b/test/functional/feature_llmq_chainlocks.py @@ -39,6 +39,8 @@ def run_test(self): self.activate_by_name('v20', expected_activation_height=904) self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount())) + self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=True, expected_null_cl=True) + self.log.info("Mining 4 quorums") for i in range(4): self.mine_quorum() @@ -184,16 +186,16 @@ def create_chained_txs(self, node, amount): return [txid, rawtxid] - def test_coinbase_best_cl(self, node, expected_cl_in_cb=True): + def test_coinbase_best_cl(self, node, expected_cl_in_cb=True, expected_null_cl=False): block_hash = node.getbestblockhash() block = node.getblock(block_hash, 2) cbtx = block["cbTx"] assert_equal(int(cbtx["version"]) > 2, expected_cl_in_cb) - self.log.info("cbtx: "+str(cbtx)) if expected_cl_in_cb: cb_height = int(cbtx["height"]) best_cl_height_diff = int(cbtx["bestCLHeightDiff"]) best_cl_signature = cbtx["bestCLSignature"] + assert_equal(expected_null_cl, int(best_cl_signature, 16) == 0) if int(best_cl_signature, 16) == 0: # Null bestCLSignature is allowed. # bestCLHeightDiff must be 0 if bestCLSignature is null From 60994a1097f265a67e24ec932b32387ccf27b807 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Sun, 23 Apr 2023 13:05:02 +0300 Subject: [PATCH 08/18] adjustements --- src/evo/cbtx.cpp | 117 ++++++++++++++++++++++++++++++++++++++++------- src/evo/cbtx.h | 4 +- src/miner.cpp | 7 +-- 3 files changed, 108 insertions(+), 20 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 0cf4510ae6f4..099643f472a5 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -334,37 +334,81 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, } if (cbTx.nVersion >= CCbTx::CB_V20_VERSION) { - if (cbTx.bestCLSignature.IsValid()) { - int bestChainLockedHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; - uint256 bestChainLockedHash = ::ChainActive()[bestChainLockedHeight]->GetBlockHash(); - if (!chainlock_handler.VerifyChainLock(bestChainLockedHeight, bestChainLockedHash, cbTx.bestCLSignature )){ + auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev); + // 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-clsig"); + } + int prevBlockCoinbaseCLHeight = pindexPrev->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; + int curBlockCoinbaseCLHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + if (curBlockCoinbaseCLHeight < prevBlockCoinbaseCLHeight) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } } - else { - if (cbTx.bestCLHeightDiff != 0) { - // Null bestCLSignature (IsNull() doesn't exist: we assume that a non valid BLS sig is null) is allowed only when bestCLHeightDiff is 0 - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); + + // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null + if (cbTx.bestCLSignature.IsValid()) { + int curBlockCoinbaseCLHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + uint256 curBlockCoinbaseCLBlockHash = ::ChainActive()[curBlockCoinbaseCLHeight]->GetBlockHash(); + if (!chainlock_handler.VerifyChainLock(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature )){ + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } } + 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 EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) +bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, std::optional> prevBlockCoinbaseChainlock, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) { - if (chainlock_handler.GetBestChainLock().IsNull()) { - bestCLHeightDiff = 0; - bestCLSignature.Reset(); - return false; + if (prevBlockCoinbaseChainlock.has_value()) { + // Previous block Coinbase contains a non-null CL: We must insert the same sig or a better (newest) one + if (chainlock_handler.GetBestChainLock().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 + auto curCLHeight = chainlock_handler.GetBestChainLock().getHeight(); + auto prevCLHeight = (static_cast(nHeight - 1) - 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 = (static_cast(nHeight) - chainlock_handler.GetBestChainLock().getHeight() - 1); + bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); + } + + return true; } + else { + // Previous block Coinbase has no CL. We can either insert null or any valid CL + if (chainlock_handler.GetBestChainLock().IsNull()) { + // We don't know any CL, therefore inserting a null CL + bestCLHeightDiff = 0; + bestCLSignature.Reset(); + return false; + } - bestCLHeightDiff = (static_cast(nHeight) - chainlock_handler.GetBestChainLock().getHeight() - 1); - bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); + // Inserting our best CL + bestCLHeightDiff = (static_cast(nHeight) - chainlock_handler.GetBestChainLock().getHeight() - 1); + bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); - return true; + return true; + } } @@ -373,3 +417,44 @@ std::string CCbTx::ToString() const return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s)", nVersion, nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString()); } + +std::optional GetCoinbaseTx(const CBlockIndex* pindex) +{ + if (pindex == nullptr) { + return std::nullopt; + } + + CBlock block; + if (!ReadBlockFromDisk(block, pindex, Params().GetConsensus())) { + return std::nullopt; + } + + CTransactionRef cbTx = block.vtx[0]; + CCbTx cbTxPayload; + if (!GetTxPayload(*cbTx, cbTxPayload)) { + return std::nullopt; + } + + return cbTxPayload; +} + +std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex) +{ + auto opt_cbtx = GetCoinbaseTx(pindex); + + if (!opt_cbtx.has_value()) { + return std::nullopt; + } + + CCbTx& cbtx = opt_cbtx.value(); + + if (cbtx.nVersion < CCbTx::CB_V20_VERSION) { + return std::nullopt; + } + + if (!cbtx.bestCLSignature.IsValid()) { + return std::nullopt; + } + + return std::make_pair(cbtx.bestCLSignature, cbtx.bestCLHeightDiff); +} diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 8c5b1c62820a..505a30930de9 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -74,6 +74,8 @@ bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state); -bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature); +bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, std::optional> prevBlockCoinbaseChainlock, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature); +std::optional GetCoinbaseTx(const CBlockIndex* pindex); +std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex); #endif // BITCOIN_EVO_CBTX_H diff --git a/src/miner.cpp b/src/miner.cpp index b52b251e5cd7..a2d35496a2fe 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -216,11 +216,12 @@ std::unique_ptr BlockAssembler::CreateNewBlock(CChainState& chai throw std::runtime_error(strprintf("%s: CalcCbTxMerkleRootQuorums failed: %s", __func__, FormatStateMessage(state))); } if (llmq::utils::IsV20Active(pindexPrev)) { - if (!EmplaceBestChainlock(m_clhandler, nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { - LogPrintf("CreateNewBlock() height[%d] CBTx failed to find best CL.\n", nHeight); + auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev); + if (!EmplaceBestChainlock(m_clhandler, nHeight, prevBlockCoinbaseChainlock, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { + LogPrintf("CreateNewBlock() h[%d] CbTx failed to find best CL. Inserting null CL\n", nHeight); } else { - LogPrintf("CreateNewBlock() height[%d] CBTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); + LogPrintf("CreateNewBlock() h[%d] CbTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); } } } From 3fffb2fa0d60cf34d9e78ac1b59641aba3fd928e Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Tue, 25 Apr 2023 11:55:28 +0300 Subject: [PATCH 09/18] refactor --- src/evo/cbtx.cpp | 30 ++++++++++++++++-------------- src/evo/cbtx.h | 4 ++-- src/miner.cpp | 8 ++++---- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 099643f472a5..1232dde1e65e 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -322,7 +322,7 @@ bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPre return true; } -bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state) +bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state) { if (block.vtx[0]->nType != TRANSACTION_COINBASE) { return true; @@ -334,7 +334,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, } if (cbTx.nVersion >= CCbTx::CB_V20_VERSION) { - auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev); + auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex); // 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 @@ -342,8 +342,8 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, // IsNull() doesn't exist for CBLSSignature: we assume that a non valid BLS sig is null return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } - int prevBlockCoinbaseCLHeight = pindexPrev->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; - int curBlockCoinbaseCLHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + int prevBlockCoinbaseCLHeight = pindex->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; if (curBlockCoinbaseCLHeight < prevBlockCoinbaseCLHeight) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } @@ -351,9 +351,9 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, // IsNull() doesn't exist for CBLSSignature: we assume that a valid BLS sig is non-null if (cbTx.bestCLSignature.IsValid()) { - int curBlockCoinbaseCLHeight = pindexPrev->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; uint256 curBlockCoinbaseCLBlockHash = ::ChainActive()[curBlockCoinbaseCLHeight]->GetBlockHash(); - if (!chainlock_handler.VerifyChainLock(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature )){ + if (!chainlock_handler.VerifyChainLock(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } } @@ -367,11 +367,13 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, return true; } -bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, std::optional> prevBlockCoinbaseChainlock, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) +bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const CBlockIndex* pindexPrev, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature) { + auto best_clsig = chainlock_handler.GetBestChainLock(); + 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 (chainlock_handler.GetBestChainLock().IsNull()) { + 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; @@ -379,8 +381,8 @@ bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, con } // We check if our best CL is newer than the one from previous block Coinbase - auto curCLHeight = chainlock_handler.GetBestChainLock().getHeight(); - auto prevCLHeight = (static_cast(nHeight - 1) - prevBlockCoinbaseChainlock->second - 1); + auto curCLHeight = best_clsig.getHeight(); + auto prevCLHeight = static_cast(pindexPrev->nHeight) - prevBlockCoinbaseChainlock->second - 1; if (curCLHeight < prevCLHeight) { // Our best CL isn't newer: inserting CL from previous block bestCLHeightDiff = prevBlockCoinbaseChainlock->second + 1; @@ -388,15 +390,15 @@ bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, con } else { // Our best CL is newer - bestCLHeightDiff = (static_cast(nHeight) - chainlock_handler.GetBestChainLock().getHeight() - 1); - bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); + bestCLHeightDiff = static_cast(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 (chainlock_handler.GetBestChainLock().IsNull()) { + if (best_clsig.IsNull()) { // We don't know any CL, therefore inserting a null CL bestCLHeightDiff = 0; bestCLSignature.Reset(); @@ -404,7 +406,7 @@ bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, con } // Inserting our best CL - bestCLHeightDiff = (static_cast(nHeight) - chainlock_handler.GetBestChainLock().getHeight() - 1); + bestCLHeightDiff = static_cast(pindexPrev->nHeight) - best_clsig.getHeight(); bestCLSignature = chainlock_handler.GetBestChainLock().getSig(); return true; diff --git a/src/evo/cbtx.h b/src/evo/cbtx.h index 505a30930de9..f3141464f5fc 100644 --- a/src/evo/cbtx.h +++ b/src/evo/cbtx.h @@ -72,9 +72,9 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati bool CheckCbTxMerkleRoots(const CBlock& block, const CBlockIndex* pindex, const llmq::CQuorumBlockProcessor& quorum_block_processor, BlockValidationState& state, const CCoinsViewCache& view); bool CalcCbTxMerkleRootMNList(const CBlock& block, const CBlockIndex* pindexPrev, uint256& merkleRootRet, BlockValidationState& state, const CCoinsViewCache& view); bool CalcCbTxMerkleRootQuorums(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CQuorumBlockProcessor& quorum_block_processor, uint256& merkleRootRet, BlockValidationState& state); -bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindexPrev, const llmq::CChainLocksHandler& chainlock_handler, BlockValidationState& state); -bool EmplaceBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, const int nHeight, std::optional> prevBlockCoinbaseChainlock, uint32_t& bestCLHeightDiff, CBLSSignature& bestCLSignature); +bool CheckCbTxBestChainlock(const CBlock& block, 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 GetCoinbaseTx(const CBlockIndex* pindex); std::optional> GetNonNullCoinbaseChainlock(const CBlockIndex* pindex); diff --git a/src/miner.cpp b/src/miner.cpp index a2d35496a2fe..c5b5e34c23f0 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -216,12 +216,12 @@ std::unique_ptr BlockAssembler::CreateNewBlock(CChainState& chai throw std::runtime_error(strprintf("%s: CalcCbTxMerkleRootQuorums failed: %s", __func__, FormatStateMessage(state))); } if (llmq::utils::IsV20Active(pindexPrev)) { - auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindexPrev); - if (!EmplaceBestChainlock(m_clhandler, nHeight, prevBlockCoinbaseChainlock, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { - LogPrintf("CreateNewBlock() h[%d] CbTx failed to find best CL. Inserting null CL\n", nHeight); + if (CalcCbTxBestChainlock(m_clhandler, pindexPrev, cbTx.bestCLHeightDiff, cbTx.bestCLSignature)) { + LogPrintf("CreateNewBlock() h[%d] CbTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); } else { - LogPrintf("CreateNewBlock() h[%d] CbTx bestCLHeightDiff[%d] CLSig[%s]\n", nHeight, cbTx.bestCLHeightDiff, cbTx.bestCLSignature.ToString()); + // not an error + LogPrintf("CreateNewBlock() h[%d] CbTx failed to find best CL. Inserting null CL\n", nHeight); } } } From b2cbaabcae13b63b3afe9aea424fe929f52c0074 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 24 Apr 2023 14:09:49 +0300 Subject: [PATCH 10/18] use best known cl as a shortcut in few places --- src/evo/cbtx.cpp | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 1232dde1e65e..eb43afd81e6d 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -334,6 +334,12 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons } if (cbTx.nVersion >= CCbTx::CB_V20_VERSION) { + 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 + return true; + } + auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex); // If std::optional prevBlockCoinbaseChainlock is empty, then up to the previous block, coinbase Chainlock is null. if (prevBlockCoinbaseChainlock.has_value()) { @@ -352,6 +358,10 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons // 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(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 + return true; + } uint256 curBlockCoinbaseCLBlockHash = ::ChainActive()[curBlockCoinbaseCLHeight]->GetBlockHash(); if (!chainlock_handler.VerifyChainLock(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); @@ -370,6 +380,13 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons 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() == 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 From 3fb85eebf8c4bb3aad8a120291c6966cc7b22723 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 24 Apr 2023 14:11:23 +0300 Subject: [PATCH 11/18] fix bench --- src/evo/specialtxman.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index b33e8e9517c4..10bb87eecec5 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -108,6 +108,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll static int64_t nTimeQuorum = 0; static int64_t nTimeDMN = 0; static int64_t nTimeMerkle = 0; + static int64_t nTimeCbTxCL = 0; int64_t nTime1 = GetTimeMicros(); @@ -154,14 +155,18 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll return false; } + int64_t nTime5 = GetTimeMicros(); + nTimeMerkle += nTime5 - nTime4; + LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); + if (!CheckCbTxBestChainlock(block, pindex, chainlock_handler, state)) { // pass the state returned by the function above return false; } - int64_t nTime5 = GetTimeMicros(); - nTimeMerkle += nTime5 - nTime4; - LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); + int64_t nTime6 = GetTimeMicros(); + nTimeCbTxCL += nTime6 - nTime5; + LogPrint(BCLog::BENCHMARK, " - CheckCbTxBestChainlock: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeCbTxCL * 0.000001); } catch (const std::exception& e) { LogPrintf("%s -- failed: %s\n", __func__, e.what()); return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "failed-procspectxsinblock"); From 032cfb16cfcdbbbac5b1b41f56629f665c5e8b1f Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 24 Apr 2023 14:11:41 +0300 Subject: [PATCH 12/18] feat: skip CheckCbTxBestChainlock for assumevalid blocks --- src/evo/specialtxman.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evo/specialtxman.cpp b/src/evo/specialtxman.cpp index 10bb87eecec5..429e9db2d506 100644 --- a/src/evo/specialtxman.cpp +++ b/src/evo/specialtxman.cpp @@ -159,7 +159,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll nTimeMerkle += nTime5 - nTime4; LogPrint(BCLog::BENCHMARK, " - CheckCbTxMerkleRoots: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeMerkle * 0.000001); - if (!CheckCbTxBestChainlock(block, pindex, chainlock_handler, state)) { + if (fCheckCbTxMerleRoots && !CheckCbTxBestChainlock(block, pindex, chainlock_handler, state)) { // pass the state returned by the function above return false; } From 5d094a9dd3f801f112445948f908515966e33c74 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Mon, 24 Apr 2023 14:12:10 +0300 Subject: [PATCH 13/18] refactor VerifyChainLock and use it in more places --- src/llmq/chainlocks.cpp | 9 ++++----- src/llmq/chainlocks.h | 2 +- src/rpc/quorums.cpp | 9 ++++----- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/src/llmq/chainlocks.cpp b/src/llmq/chainlocks.cpp index 9cd84cb067e4..370a1ffb36bf 100644 --- a/src/llmq/chainlocks.cpp +++ b/src/llmq/chainlocks.cpp @@ -122,8 +122,7 @@ void CChainLocksHandler::ProcessNewChainLock(const NodeId from, const llmq::CCha } } - const uint256 requestId = ::SerializeHash(std::make_pair(CLSIG_REQUESTID_PREFIX, clsig.getHeight())); - if (!llmq::CSigningManager::VerifyRecoveredSig(Params().GetConsensus().llmqTypeChainLocks, *llmq::quorumManager, clsig.getHeight(), requestId, clsig.getBlockHash(), clsig.getSig())) { + if (!VerifyChainLock(clsig)) { LogPrint(BCLog::CHAINLOCKS, "CChainLocksHandler::%s -- invalid CLSIG (%s), peer=%d\n", __func__, clsig.ToString(), from); if (from != -1) { Misbehaving(from, 10); @@ -558,11 +557,11 @@ bool CChainLocksHandler::HasChainLock(int nHeight, const uint256& blockHash) con return InternalHasChainLock(nHeight, blockHash); } -bool CChainLocksHandler::VerifyChainLock(int nHeight, const uint256& blockHash, const CBLSSignature& sig) const +bool CChainLocksHandler::VerifyChainLock(const CChainLockSig& clsig) const { const auto llmqType = Params().GetConsensus().llmqTypeChainLocks; - const uint256 nRequestId = ::SerializeHash(std::make_pair(llmq::CLSIG_REQUESTID_PREFIX, nHeight)); - return llmq::CSigningManager::VerifyRecoveredSig(llmqType, qman, nHeight, nRequestId, blockHash, sig); + const uint256 nRequestId = ::SerializeHash(std::make_pair(llmq::CLSIG_REQUESTID_PREFIX, clsig.getHeight())); + return llmq::CSigningManager::VerifyRecoveredSig(llmqType, qman, clsig.getHeight(), nRequestId, clsig.getBlockHash(), clsig.getSig()); } bool CChainLocksHandler::InternalHasChainLock(int nHeight, const uint256& blockHash) const diff --git a/src/llmq/chainlocks.h b/src/llmq/chainlocks.h index 7da2c72b57bd..27687da2f51e 100644 --- a/src/llmq/chainlocks.h +++ b/src/llmq/chainlocks.h @@ -104,7 +104,7 @@ class CChainLocksHandler : public CRecoveredSigsListener bool HasChainLock(int nHeight, const uint256& blockHash) const LOCKS_EXCLUDED(cs); bool HasConflictingChainLock(int nHeight, const uint256& blockHash) const LOCKS_EXCLUDED(cs); - bool VerifyChainLock(int nHeight, const uint256& blockHash, const CBLSSignature& sig) const; + bool VerifyChainLock(const CChainLockSig& clsig) const; bool IsTxSafeForMining(const CInstantSendManager& isman, const uint256& txid) const LOCKS_EXCLUDED(cs); diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 28bd9efa8716..c4ea9ed76902 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include @@ -873,8 +874,8 @@ static UniValue verifychainlock(const JSONRPCRequest& request) const uint256 nBlockHash(ParseHashV(request.params[0], "blockHash")); - CBLSSignature chainLockSig; - if (!chainLockSig.SetHexStr(request.params[1].get_str())) { + CBLSSignature sig; + if (!sig.SetHexStr(request.params[1].get_str())) { throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid signature format"); } @@ -891,9 +892,7 @@ static UniValue verifychainlock(const JSONRPCRequest& request) LLMQContext& llmq_ctx = EnsureLLMQContext(request.context); - const auto llmqType = Params().GetConsensus().llmqTypeChainLocks; - const uint256 nRequestId = ::SerializeHash(std::make_pair(llmq::CLSIG_REQUESTID_PREFIX, nBlockHeight)); - return llmq::CSigningManager::VerifyRecoveredSig(llmqType, *llmq_ctx.qman, nBlockHeight, nRequestId, nBlockHash, chainLockSig); + return llmq_ctx.clhandler->VerifyChainLock(llmq::CChainLockSig(nBlockHeight, nBlockHash, sig)); } static void verifyislock_help(const JSONRPCRequest& request) From 380e9e072eb085ef941eb4a8ae85c5cf1db9d25c Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Tue, 25 Apr 2023 12:07:29 +0300 Subject: [PATCH 14/18] adjustement --- src/evo/cbtx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index eb43afd81e6d..59ef867e61c5 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -363,7 +363,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons return true; } uint256 curBlockCoinbaseCLBlockHash = ::ChainActive()[curBlockCoinbaseCLHeight]->GetBlockHash(); - if (!chainlock_handler.VerifyChainLock(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature)) { + if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature))) { return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); } } From 04d1986c109f9057359f66b1103fceb6d48628c6 Mon Sep 17 00:00:00 2001 From: Odysseas Gabrielides Date: Tue, 25 Apr 2023 12:49:36 +0300 Subject: [PATCH 15/18] suggestions --- src/evo/cbtx.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 59ef867e61c5..f36ec7cb05c3 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -38,8 +38,7 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati if (pindexPrev) { bool isV20 = llmq::utils::IsV20Active(pindexPrev); - bool isCbV20 = cbTx.nVersion == CCbTx::CB_V20_VERSION; - if (isV20 != isCbV20) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); + if (isV20 && cbTx.nVersion != CCbTx::CB_V20_VERSION) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } if (pindexPrev && pindexPrev->nHeight + 1 != cbTx.nHeight) { @@ -333,7 +332,7 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } - if (cbTx.nVersion >= CCbTx::CB_V20_VERSION) { + if (cbTx.nVersion == CCbTx::CB_V20_VERSION) { 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 @@ -346,12 +345,12 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons // 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-clsig"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-null-clsig"); } int prevBlockCoinbaseCLHeight = pindex->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; if (curBlockCoinbaseCLHeight < prevBlockCoinbaseCLHeight) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig"); } } @@ -362,9 +361,9 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons // matches our best (but outdated) clsig, no need to verify it again return true; } - uint256 curBlockCoinbaseCLBlockHash = ::ChainActive()[curBlockCoinbaseCLHeight]->GetBlockHash(); + uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature))) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-clsig"); + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); } } else if (cbTx.bestCLHeightDiff != 0) { From 39997394f497619190ce3bb139ff0a4ca6d19692 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 26 Apr 2023 22:59:13 +0300 Subject: [PATCH 16/18] fix: no cb_v20 prior to v20 activation, only cb_v20 after v20 activation --- src/evo/cbtx.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index f36ec7cb05c3..1f40ce215422 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -36,18 +36,18 @@ bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidati return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } - if (pindexPrev) { - bool isV20 = llmq::utils::IsV20Active(pindexPrev); - if (isV20 && cbTx.nVersion != CCbTx::CB_V20_VERSION) return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); - } - if (pindexPrev && pindexPrev->nHeight + 1 != cbTx.nHeight) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-height"); } if (pindexPrev) { bool fDIP0008Active = pindexPrev->nHeight >= Params().GetConsensus().DIP0008Height; - if (fDIP0008Active && cbTx.nVersion < 2) { + if (fDIP0008Active && cbTx.nVersion < CCbTx::CB_V19_VERSION) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); + } + + bool isV20 = llmq::utils::IsV20Active(pindexPrev); + if ((isV20 && cbTx.nVersion < CCbTx::CB_V20_VERSION) || (!isV20 && cbTx.nVersion >= CCbTx::CB_V20_VERSION)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-cbtx-version"); } } From 29b0c301f052b2000cb99b905aeec078df27ea44 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 26 Apr 2023 23:03:04 +0300 Subject: [PATCH 17/18] fix: post-cb_v20 should use cb_v20 rules too, bail out early on pre-cb_v20 in CheckCbTxBestChainlock --- src/evo/cbtx.cpp | 68 ++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/evo/cbtx.cpp b/src/evo/cbtx.cpp index 1f40ce215422..f93b20eaffda 100644 --- a/src/evo/cbtx.cpp +++ b/src/evo/cbtx.cpp @@ -332,45 +332,45 @@ bool CheckCbTxBestChainlock(const CBlock& block, const CBlockIndex* pindex, cons return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-payload"); } - if (cbTx.nVersion == CCbTx::CB_V20_VERSION) { - 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 - return true; - } + if (cbTx.nVersion < CCbTx::CB_V20_VERSION) { + return true; + } - auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex); - // 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"); - } - int prevBlockCoinbaseCLHeight = pindex->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; - int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; - if (curBlockCoinbaseCLHeight < prevBlockCoinbaseCLHeight) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-older-clsig"); - } - } + 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 + return true; + } - // 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(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 - return true; - } - uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); - if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature))) { - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); - } + auto prevBlockCoinbaseChainlock = GetNonNullCoinbaseChainlock(pindex); + // 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"); } - else if (cbTx.bestCLHeightDiff != 0) { - // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 - return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); + int prevBlockCoinbaseCLHeight = pindex->nHeight - static_cast(prevBlockCoinbaseChainlock.value().second) - 1; + int curBlockCoinbaseCLHeight = pindex->nHeight - static_cast(cbTx.bestCLHeightDiff) - 1; + if (curBlockCoinbaseCLHeight < prevBlockCoinbaseCLHeight) { + 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(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 + return true; + } + uint256 curBlockCoinbaseCLBlockHash = pindex->GetAncestor(curBlockCoinbaseCLHeight)->GetBlockHash(); + if (!chainlock_handler.VerifyChainLock(llmq::CChainLockSig(curBlockCoinbaseCLHeight, curBlockCoinbaseCLBlockHash, cbTx.bestCLSignature))) { + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-invalid-clsig"); + } + } else if (cbTx.bestCLHeightDiff != 0) { + // Null bestCLSignature is allowed only with bestCLHeightDiff = 0 + return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, "bad-cbtx-cldiff"); } return true; From c5ee6ed978782400c1181d569cbd3b59a7fec5fe Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Wed, 26 Apr 2023 23:59:03 +0300 Subject: [PATCH 18/18] fix: tweak test --- test/functional/feature_llmq_chainlocks.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/functional/feature_llmq_chainlocks.py b/test/functional/feature_llmq_chainlocks.py index eb5164b78c72..e6428da555b5 100755 --- a/test/functional/feature_llmq_chainlocks.py +++ b/test/functional/feature_llmq_chainlocks.py @@ -31,16 +31,17 @@ def run_test(self): self.activate_dip8() - self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) - self.wait_for_sporks_same() - self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=False) - self.activate_by_name('v20', expected_activation_height=904) + self.activate_v20(expected_activation_height=904) self.log.info("Activated v20 at height:" + str(self.nodes[0].getblockcount())) + # no quorums, no CLs - null CL in CbTx self.test_coinbase_best_cl(self.nodes[0], expected_cl_in_cb=True, expected_null_cl=True) + self.nodes[0].sporkupdate("SPORK_17_QUORUM_DKG_ENABLED", 0) + self.wait_for_sporks_same() + self.log.info("Mining 4 quorums") for i in range(4): self.mine_quorum() @@ -196,7 +197,7 @@ def test_coinbase_best_cl(self, node, expected_cl_in_cb=True, expected_null_cl=F best_cl_height_diff = int(cbtx["bestCLHeightDiff"]) best_cl_signature = cbtx["bestCLSignature"] assert_equal(expected_null_cl, int(best_cl_signature, 16) == 0) - if int(best_cl_signature, 16) == 0: + if expected_null_cl: # Null bestCLSignature is allowed. # bestCLHeightDiff must be 0 if bestCLSignature is null assert_equal(best_cl_height_diff, 0) @@ -205,7 +206,9 @@ def test_coinbase_best_cl(self, node, expected_cl_in_cb=True, expected_null_cl=F best_cl_height = cb_height - best_cl_height_diff - 1 target_block_hash = node.getblockhash(best_cl_height) # Verify CL signature - assert(node.verifychainlock(target_block_hash, best_cl_signature, best_cl_height)) + assert node.verifychainlock(target_block_hash, best_cl_signature, best_cl_height) + else: + assert "bestCLHeightDiff" not in cbtx and "bestCLSignature" not in cbtx if __name__ == '__main__':