From e9a2560961c33f3a760346cf0e7d29e6d4134c53 Mon Sep 17 00:00:00 2001
From: PastaClaw <thepastaclaw@users.noreply.github.com>
Date: Sat, 13 Jun 2026 08:23:12 -0500
Subject: [PATCH 1/3] fix: require distinct coinbase outputs for duplicate
 superblock payments

CSuperblock::IsValid matched expected payments against txNew.vout using a
forward scan that re-started at the previously matched index. Two adjacent
expected payments with identical scriptPubKey and amount could both match
the same coinbase output, so a superblock could shortchange payees by
emitting only one of two duplicate payouts.

Initialize the cursor to -1 and scan from nVoutIndex + 1 so each expected
payment consumes a distinct output. Adds a unit-test regression covering
both the missing-output and well-formed cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 src/Makefile.test.include                |  1 +
 src/governance/superblock.cpp            |  7 +-
 src/test/governance_superblock_tests.cpp | 83 ++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 src/test/governance_superblock_tests.cpp

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..bafa13fb2776 100644
--- a/src/governance/superblock.cpp
+++ b/src/governance/superblock.cpp
@@ -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,10 @@ bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew,
 
         bool fPaymentMatch = false;
 
-        for (int j = nVoutIndex; j < nOutputs; j++) {
+        // 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).
+        for (int j = nVoutIndex + 1; j < nOutputs; j++) {
             // Find superblock payment
             fPaymentMatch = ((payment.script == txNew.vout[j].scriptPubKey) &&
                              (payment.nAmount == txNew.vout[j].nValue));
diff --git a/src/test/governance_superblock_tests.cpp b/src/test/governance_superblock_tests.cpp
new file mode 100644
index 000000000000..1fafc305eee2
--- /dev/null
+++ b/src/test/governance_superblock_tests.cpp
@@ -0,0 +1,83 @@
+// 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): 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.
+    // After the fix, 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));
+    }
+
+    // Case 2: 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));
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()

From e5895ab9399e4d8e10ab12a1b5302d725ed16fa2 Mon Sep 17 00:00:00 2001
From: UdjinM6 <UdjinM6@users.noreply.github.com>
Date: Sun, 14 Jun 2026 10:07:39 +0300
Subject: [PATCH 2/3] fix: gate distinct superblock output check behind v24

The stricter rule requiring each expected superblock payment to consume a
distinct coinbase output tightens consensus validation. Apply it only once
DEPLOYMENT_V24 is active so all nodes flip the rule at the same height and
already-validated history keeps the legacy (reuse-allowed) scan, avoiding a
chain split during rollout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/governance/superblock.cpp            | 17 ++++++++++-------
 src/governance/superblock.h              |  4 ++--
 src/masternode/payments.cpp              |  9 ++++++---
 src/masternode/payments.h                |  2 +-
 src/test/governance_superblock_tests.cpp | 21 ++++++++++++++++-----
 src/validation.cpp                       |  2 +-
 6 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/src/governance/superblock.cpp b/src/governance/superblock.cpp
index bafa13fb2776..d4dcf9979481 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
@@ -303,10 +303,13 @@ bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew,
 
         bool fPaymentMatch = false;
 
-        // 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).
-        for (int j = nVoutIndex + 1; 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));
@@ -614,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;
 }
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<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);
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<CTxOut>& voutMasternodePaymentsRet, std::vector<CTxOut>& voutSuperblockPaymentsRet);
diff --git a/src/test/governance_superblock_tests.cpp b/src/test/governance_superblock_tests.cpp
index 1fafc305eee2..759bc6e9485c 100644
--- a/src/test/governance_superblock_tests.cpp
+++ b/src/test/governance_superblock_tests.cpp
@@ -56,27 +56,38 @@ BOOST_AUTO_TEST_CASE(isvalid_duplicate_payments_require_distinct_outputs)
     const CAmount blockReward = 500 * COIN;
     CChain dummy_chain;
 
-    // Case 1 (regression): coinbase carries only ONE output matching the
+    // 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.
-    // After the fix, the second expected payment must find a distinct output
+    // 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));
+        BOOST_CHECK(!sb.IsValid(dummy_chain, CTransaction(txNew), nBlockHeight, blockReward, /*is_v24=*/true));
     }
 
-    // Case 2: coinbase carries TWO outputs matching the duplicate expected
+    // 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));
+        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));
     }
 }
 
diff --git a/src/validation.cpp b/src/validation.cpp
index 06d45ca61bcb..b7528e9b41f3 100644
--- a/src/validation.cpp
+++ b/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");

From 0d27678e3830e166eb197c762aee48954594a77d Mon Sep 17 00:00:00 2001
From: PastaClaw <thepastaclaw@users.noreply.github.com>
Date: Mon, 15 Jun 2026 07:03:14 -0500
Subject: [PATCH 3/3] doc: note future superblock scan simplification

---
 src/governance/superblock.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/governance/superblock.cpp b/src/governance/superblock.cpp
index d4dcf9979481..781596017055 100644
--- a/src/governance/superblock.cpp
+++ b/src/governance/superblock.cpp
@@ -308,6 +308,8 @@ bool CSuperblock::IsValid(const CChain& active_chain, const CTransaction& txNew,
         // 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