diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 375c9940d059..b7d7cc5cde14 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -119,6 +119,7 @@ BITCOIN_TESTS =\ test/flatfile_tests.cpp \ test/fs_tests.cpp \ test/getarg_tests.cpp \ + test/governance_superblock_tests.cpp \ test/governance_validators_tests.cpp \ test/coinjoin_inouts_tests.cpp \ test/coinjoin_dstxmanager_tests.cpp \ diff --git a/src/governance/superblock.cpp b/src/governance/superblock.cpp index f50538877257..781596017055 100644 --- a/src/governance/superblock.cpp +++ b/src/governance/superblock.cpp @@ -245,7 +245,7 @@ CAmount CSuperblock::GetPaymentsTotalAmount() * - Does this transaction match the superblock? */ -bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward) +bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward, bool is_v24) { // TODO : LOCK(cs); // No reason for a lock here now since this method only accesses data @@ -292,7 +292,7 @@ bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew, return false; } - int nVoutIndex = 0; + int nVoutIndex = -1; for (int i = 0; i < nPayments; i++) { CGovernancePayment payment; if (!GetPayment(i, payment)) { @@ -303,7 +303,15 @@ bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew, bool fPaymentMatch = false; - for (int j = nVoutIndex; j < nOutputs; j++) { + // From V24 on, start past the previously matched output so each expected + // payment consumes a distinct vout (two adjacent payments with the same + // script and amount must match two separate outputs, not the same one + // twice). Before V24 the scan restarted at the previously matched index + // (inclusive), which is kept for backwards compatibility. + // TODO: After V24 is hardened/finalized so historical duplicate-output + // blocks cannot be encountered, simplify this path to the V24 scan only. + const int nVoutStart = is_v24 ? nVoutIndex + 1 : std::max(nVoutIndex, 0); + for (int j = nVoutStart; j < nOutputs; j++) { // Find superblock payment fPaymentMatch = ((payment.script == txNew.vout[j].scriptPubKey) && (payment.nAmount == txNew.vout[j].nValue)); @@ -611,12 +619,12 @@ bool SuperblockManager::IsSuperblockTriggered(const CDeterministicMNList& tip_mn } bool SuperblockManager::IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, - const CTransaction& txNew, int nBlockHeight, CAmount blockReward) const + const CTransaction& txNew, int nBlockHeight, CAmount blockReward, bool is_v24) const { LOCK(cs_sb); CSuperblock_sptr pSuperblock; if (GetBestSuperblockInternal(tip_mn_list, pSuperblock, nBlockHeight)) { - return pSuperblock->IsValid(active_chain, txNew, nBlockHeight, blockReward); + return pSuperblock->IsValid(active_chain, txNew, nBlockHeight, blockReward, is_v24); } return false; } diff --git a/src/governance/superblock.h b/src/governance/superblock.h index 1b24bec323d2..b0204d8544d6 100644 --- a/src/governance/superblock.h +++ b/src/governance/superblock.h @@ -107,7 +107,7 @@ class CSuperblock : public CGovernanceObject bool GetPayment(int nPaymentIndex, CGovernancePayment& paymentRet); CAmount GetPaymentsTotalAmount(); - bool IsValid(const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward); + bool IsValid(const CChain& active_chain, const CTransaction& txNew, int nBlockHeight, CAmount blockReward, bool is_v24); bool IsExpired(int heightToTest) const; std::vector GetProposalHashes() const; @@ -158,7 +158,7 @@ class SuperblockManager bool IsSuperblockTriggered(const CDeterministicMNList& tip_mn_list, int nBlockHeight) EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); bool IsValidSuperblock(const CChain& active_chain, const CDeterministicMNList& tip_mn_list, const CTransaction& txNew, - int nBlockHeight, CAmount blockReward) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); + int nBlockHeight, CAmount blockReward, bool is_v24) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); bool GetSuperblockPayments(const CDeterministicMNList& tip_mn_list, int nBlockHeight, std::vector& voutSuperblockRet) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb); diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index be5635d15a27..2577a6e40513 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -184,8 +184,9 @@ CAmount PlatformShare(const CAmount reward) * - Other blocks are 10% lower in outgoing value, so in total, no extra coins are created * - When non-superblocks are detected, the normal schedule should be maintained */ -bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock) +bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const CBlockIndex* pindexPrev, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock) { + const int nBlockHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1; bool isBlockRewardValueMet = (block.vtx[0]->GetValueOut() <= blockReward); strErrorRet = ""; @@ -249,7 +250,8 @@ bool CMNPaymentsProcessor::IsBlockValueValid(const CBlock& block, const int nBlo } // this actually also checks for correct payees and not only amount - if (!m_superblocks.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, *block.vtx[0], nBlockHeight, blockReward)) { + const bool is_v24{DeploymentActiveAfter(pindexPrev, m_chainman, Consensus::DEPLOYMENT_V24)}; + if (!m_superblocks.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, *block.vtx[0], nBlockHeight, blockReward, is_v24)) { // triggered but invalid? that's weird LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Invalid superblock detected at height %d: %s", __func__, nBlockHeight, block.vtx[0]->ToString()); /* Continued */ // should NOT allow invalid superblocks, when superblocks are enabled @@ -293,9 +295,10 @@ bool CMNPaymentsProcessor::IsBlockPayeeValid(const CTransaction& txNew, const CB if (!check_superblock) return true; const auto tip_mn_list = m_dmnman.GetListAtChainTip(); + const bool is_v24{DeploymentActiveAfter(pindexPrev, m_chainman, Consensus::DEPLOYMENT_V24)}; if (m_superblocks.IsSuperblockTriggered(tip_mn_list, nBlockHeight)) { if (m_superblocks.IsValidSuperblock(m_chainman.ActiveChain(), tip_mn_list, txNew, nBlockHeight, - blockSubsidy + feeReward)) { + blockSubsidy + feeReward, is_v24)) { LogPrint(BCLog::GOBJECT, "CMNPaymentsProcessor::%s -- Valid superblock at height %d: %s", /* Continued */ __func__, nBlockHeight, txNew.ToString()); // continue validation, should also pay MN diff --git a/src/masternode/payments.h b/src/masternode/payments.h index d11da4dd3e87..e1d7b9c151af 100644 --- a/src/masternode/payments.h +++ b/src/masternode/payments.h @@ -57,7 +57,7 @@ class CMNPaymentsProcessor { } - bool IsBlockValueValid(const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock); + bool IsBlockValueValid(const CBlock& block, const CBlockIndex* pindexPrev, const CAmount blockReward, std::string& strErrorRet, const bool check_superblock); bool IsBlockPayeeValid(const CTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, const bool check_superblock); void FillBlockPayments(CMutableTransaction& txNew, const CBlockIndex* pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, std::vector& voutMasternodePaymentsRet, std::vector& voutSuperblockPaymentsRet); diff --git a/src/test/governance_superblock_tests.cpp b/src/test/governance_superblock_tests.cpp new file mode 100644 index 000000000000..759bc6e9485c --- /dev/null +++ b/src/test/governance_superblock_tests.cpp @@ -0,0 +1,94 @@ +// Copyright (c) 2026 The Dash Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include +#include