fix: require distinct outputs for duplicate superblock payments

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 \

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,13 @@ 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.
+        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 +617,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;
 }

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<uint256> 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<CTxOut>& voutSuperblockRet) const EXCLUSIVE_LOCKS_REQUIRED(!cs_sb);

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

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<CTxOut>& voutMasternodePaymentsRet, std::vector<CTxOut>& voutSuperblockPaymentsRet);

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 <chain.h>
+#include <chainparams.h>
+#include <consensus/amount.h>
+#include <governance/superblock.h>
+#include <key.h>
+#include <primitives/transaction.h>
+#include <pubkey.h>
+#include <script/script.h>
+#include <script/standard.h>
+#include <uint256.h>
+
+#include <test/util/setup_common.h>
+
+#include <boost/test/unit_test.hpp>
+
+#include <vector>
+
+struct SuperblockRegtestSetup : public BasicTestingSetup {
+    SuperblockRegtestSetup() : BasicTestingSetup(CBaseChainParams::REGTEST) {}
+};
+
+BOOST_FIXTURE_TEST_SUITE(governance_superblock_tests, SuperblockRegtestSetup)
+
+// Regression test for: CSuperblock::IsValid was matching expected payments
+// against coinbase outputs using a forward scan that re-started at the
+// previously matched index, which allowed two adjacent expected payments
+// with identical scriptPubKey and amount to both match the same coinbase
+// output. Each expected payment must consume a distinct output.
+BOOST_AUTO_TEST_CASE(isvalid_duplicate_payments_require_distinct_outputs)
+{
+    const auto& consensus = Params().GetConsensus();
+    const int nBlockHeight = consensus.nSuperblockStartBlock + consensus.nSuperblockCycle;
+    BOOST_REQUIRE(CSuperblock::IsValidBlockHeight(nBlockHeight));
+
+    CKey key;
+    key.MakeNewKey(/*fCompressed=*/true);
+    const CTxDestination dest{PKHash(key.GetPubKey())};
+    const CScript scriptPayee = GetScriptForDestination(dest);
+    const CAmount nPayAmount = 1 * COIN;
+
+    // Two identical expected payments (same script, same amount).
+    std::vector<CGovernancePayment> payments;
+    payments.emplace_back(dest, nPayAmount, /*proposalHash=*/uint256());
+    payments.emplace_back(dest, nPayAmount, /*proposalHash=*/uint256::ONE);
+    BOOST_REQUIRE(payments[0].IsValid());
+    BOOST_REQUIRE(payments[1].IsValid());
+
+    CSuperblock sb(nBlockHeight, payments);
+    BOOST_REQUIRE_EQUAL(sb.CountPayments(), 2);
+
+    const CScript scriptMinerOrMN = CScript() << OP_RETURN;
+    const CAmount blockReward = 500 * COIN;
+    CChain dummy_chain;
+
+    // Case 1 (regression, V24): coinbase carries only ONE output matching the
+    // duplicate expected payment. With the buggy forward scan that restarted
+    // at the previously matched index, both expected payments would match the
+    // single matching vout and IsValid would (incorrectly) return true.
+    // From V24 on, the second expected payment must find a distinct output
+    // and validation must fail.
+    {
+        CMutableTransaction txNew;
+        txNew.vout.emplace_back(blockReward - nPayAmount, scriptMinerOrMN);
+        txNew.vout.emplace_back(nPayAmount, scriptPayee); // single matching output
+        BOOST_CHECK(!sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true));
+    }
+
+    // Case 2 (V24): coinbase carries TWO outputs matching the duplicate expected
+    // payments. The fix must still accept this legitimate case.
+    {
+        CMutableTransaction txNew;
+        txNew.vout.emplace_back(blockReward - 2 * nPayAmount, scriptMinerOrMN);
+        txNew.vout.emplace_back(nPayAmount, scriptPayee);
+        txNew.vout.emplace_back(nPayAmount, scriptPayee);
+        BOOST_CHECK(sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true));
+    }
+
+    // Case 3 (pre-V24): the stricter distinct-output rule is gated behind V24.
+    // Before activation the legacy scan is preserved, so the single-output
+    // coinbase from Case 1 must still be accepted to avoid changing consensus
+    // for already-validated history.
+    {
+        CMutableTransaction txNew;
+        txNew.vout.emplace_back(blockReward - nPayAmount, scriptMinerOrMN);
+        txNew.vout.emplace_back(nPayAmount, scriptPayee); // single matching output
+        BOOST_CHECK(sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/false));
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

@@ -2490,7 +2490,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
 
     const bool check_superblock = m_chain_helper->IsSuperblockValidationRequired(pindex);
 
-    if (!m_chain_helper->mn_payments->IsBlockValueValid(block, pindex->nHeight, blockSubsidy + feeReward, strError, check_superblock)) {
+    if (!m_chain_helper->mn_payments->IsBlockValueValid(block, pindex->pprev, blockSubsidy + feeReward, strError, check_superblock)) {
         // NOTE: Do not punish, the node might be missing governance data
         LogPrintf("ERROR: ConnectBlock(DASH): %s\n", strError);
         return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-amount");