From 93be20423719c8a96b654b34bc01619b6b02ff7a Mon Sep 17 00:00:00 2001 From: PastaClaw Date: Thu, 25 Jun 2026 15:27:19 -0500 Subject: [PATCH] fix(consensus): require multiplicity-correct MN payment matching after v24 CMNPaymentsProcessor::IsTransactionValid previously checked each expected masternode coinbase output with std::ranges::any_of, which made matching existence-only: a single actual output could satisfy multiple identical expected outputs. With multi-payout reward shares, expected outputs can legitimately include duplicates, so this mismatch is consensus-relevant. Gate the stricter matching behind DEPLOYMENT_V24 to avoid tightening historical validation. After v24 activation, every expected output must be matched by a distinct actual output; pre-v24 retains the legacy existence-only behaviour. Extract the matching into FindUnmatchedMasternodePayment so it can be unit-tested directly, and add focused regression coverage in masternode_payments_tests, including the duplicate-expected case that motivates the fix. --- src/Makefile.test.include | 1 + src/masternode/payments.cpp | 57 +++++++++++++---- src/masternode/payments.h | 19 ++++++ src/test/masternode_payments_tests.cpp | 88 ++++++++++++++++++++++++++ 4 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 src/test/masternode_payments_tests.cpp diff --git a/src/Makefile.test.include b/src/Makefile.test.include index b7d7cc5cde14..04c920aabf4b 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -140,6 +140,7 @@ BITCOIN_TESTS =\ test/llmq_snapshot_tests.cpp \ test/llmq_utils_tests.cpp \ test/logging_tests.cpp \ + test/masternode_payments_tests.cpp \ test/dbwrapper_tests.cpp \ test/validation_tests.cpp \ test/mempool_tests.cpp \ diff --git a/src/masternode/payments.cpp b/src/masternode/payments.cpp index eb6ef85470d4..d69e7a67edeb 100644 --- a/src/masternode/payments.cpp +++ b/src/masternode/payments.cpp @@ -22,6 +22,36 @@ #include #include +int FindUnmatchedMasternodePayment(const std::vector& expected, + const std::vector& actual, + bool strict_multiplicity) +{ + if (!strict_multiplicity) { + for (size_t i = 0; i < expected.size(); ++i) { + const auto& txout = expected[i]; + if (!std::ranges::any_of(actual, [&txout](const auto& txout2) { return txout == txout2; })) { + return static_cast(i); + } + } + return -1; + } + + std::vector consumed(actual.size(), false); + for (size_t i = 0; i < expected.size(); ++i) { + const auto& txout = expected[i]; + bool found = false; + for (size_t j = 0; j < actual.size(); ++j) { + if (!consumed[j] && actual[j] == txout) { + consumed[j] = true; + found = true; + break; + } + } + if (!found) return static_cast(i); + } + return -1; +} + CAmount PlatformShare(const CAmount reward) { const CAmount platformReward = reward * 375 / 1000; @@ -131,19 +161,22 @@ CAmount PlatformShare(const CAmount reward) return true; } - for (const auto& txout : voutMasternodePayments) { - bool found = std::ranges::any_of(txNew.vout, [&txout](const auto& txout2) { return txout == txout2; }); - if (!found) { - std::string str_payout; - if (CTxDestination dest; ExtractDestination(txout.scriptPubKey, dest)) { - str_payout = "address=" + EncodeDestination(dest); - } else { - str_payout = "scriptPubKey=" + HexStr(txout.scriptPubKey); - } - LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Failed to find expected payee %s amount=%lld height=%d\n", - __func__, str_payout, txout.nValue, nBlockHeight); - return false; + // After v24 activation each expected payment must be matched by a distinct coinbase output: + // duplicate expected outputs require duplicate coinbase outputs. Pre-v24 retains the legacy + // existence-only check to avoid tightening historical validation. + const bool strict_match{DeploymentActiveAfter(pindexPrev, m_chainman, Consensus::DEPLOYMENT_V24)}; + const int unmatched_idx = FindUnmatchedMasternodePayment(voutMasternodePayments, txNew.vout, strict_match); + if (unmatched_idx >= 0) { + const auto& txout = voutMasternodePayments[unmatched_idx]; + std::string str_payout; + if (CTxDestination dest; ExtractDestination(txout.scriptPubKey, dest)) { + str_payout = "address=" + EncodeDestination(dest); + } else { + str_payout = "scriptPubKey=" + HexStr(txout.scriptPubKey); } + LogPrintf("CMNPaymentsProcessor::%s -- ERROR! Failed to find expected payee %s amount=%lld height=%d\n", + __func__, str_payout, txout.nValue, nBlockHeight); + return false; } return true; } diff --git a/src/masternode/payments.h b/src/masternode/payments.h index e1d7b9c151af..3eb8c5c32fac 100644 --- a/src/masternode/payments.h +++ b/src/masternode/payments.h @@ -17,6 +17,25 @@ class ChainstateManager; class CTransaction; class CTxOut; +/** + * Match the list of expected masternode payment outputs against the coinbase + * outputs. + * + * When @p strict_multiplicity is true, every expected output must be matched + * by a distinct actual output (multiplicity-correct matching): two identical + * expected outputs require two identical actual outputs. + * + * When false, the legacy behaviour is used where each expected output only + * has to appear at least once in the actual outputs. This is preserved for + * pre-v24 historical validation. + * + * @return -1 if every expected output is matched, otherwise the index in + * @p expected of the first output that could not be matched. + */ +int FindUnmatchedMasternodePayment(const std::vector& expected, + const std::vector& actual, + bool strict_multiplicity); + struct CMutableTransaction; namespace governance { diff --git a/src/test/masternode_payments_tests.cpp b/src/test/masternode_payments_tests.cpp new file mode 100644 index 000000000000..d85d51bae606 --- /dev/null +++ b/src/test/masternode_payments_tests.cpp @@ -0,0 +1,88 @@ +// Copyright (c) 2026 The Dash Core developers +// Distributed under the MIT/X11 software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include