feat!: exclude fees when calculating platformReward (#5612)

## Issue being fixed or feature implemented
Calculation of `platformReward` should ignore fees and rely only on
Block subsidy.

cc @QuantumExplorer 

## What was done?
From now on, the following formula is applied:
```
blockReward = blockSubsidy + feeReward
masternodeReward = masternodeShare(blockSubsidy)
platformReward = platformShare(masternodeReward)
masternodeReward += masternodeShare(feeReward)
```


## How Has This Been Tested?


## Breaking Changes
`plaftormReward` differs in networks where `mn_rr` is already active

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <[email protected]>

src/evo/creditpool.cpp

@@ -228,7 +228,7 @@ void CCreditPoolDiff::AddRewardRealloced(const CAmount reward) {
     platformReward += reward;
 }
 
-bool CCreditPoolDiff::SetTarget(const CTransaction& tx, const CAmount blockReward, TxValidationState& state)
+bool CCreditPoolDiff::SetTarget(const CTransaction& tx, const CAmount blockSubsidy, TxValidationState& state)
 {
     CCbTx cbTx;
     if (!GetTxPayload(tx, cbTx)) {
@@ -241,7 +241,7 @@ bool CCreditPoolDiff::SetTarget(const CTransaction& tx, const CAmount blockRewar
 
     if (!llmq::utils::IsMNRewardReallocationActive(pindex)) return true;
 
-    platformReward = MasternodePayments::PlatformShare(GetMasternodePayment(cbTx.nHeight, blockReward, /* reward_reallocation= */ true));
+    platformReward = MasternodePayments::PlatformShare(GetMasternodePayment(cbTx.nHeight, blockSubsidy, /* reward_reallocation= */ true));
     LogPrintf("CreditPool: set target to %lld with MN reward %lld\n", *targetBalance, platformReward);
 
     return true;
@@ -287,13 +287,13 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state)
     return true;
 }
 
-bool CCreditPoolDiff::ProcessCoinbaseTransaction(const CTransaction& tx, const CAmount blockReward, TxValidationState& state)
+bool CCreditPoolDiff::ProcessCoinbaseTransaction(const CTransaction& tx, const CAmount blockSubsidy, TxValidationState& state)
 {
     if (tx.nVersion != 3) return true;
 
     assert(tx.nType == TRANSACTION_COINBASE);
 
-    return SetTarget(tx, blockReward, state);
+    return SetTarget(tx, blockSubsidy, state);
 }
 
 bool CCreditPoolDiff::ProcessLockUnlockTransaction(const CTransaction& tx, TxValidationState& state)

src/evo/creditpool.h

@@ -82,7 +82,7 @@ class CCreditPoolDiff {
      * to change amount of credit pool
      * coinbase transaction's Payload must be valid if nVersion of coinbase transaction equals 3
      */
-    bool ProcessCoinbaseTransaction(const CTransaction& tx, const CAmount blockReward, TxValidationState& state);
+    bool ProcessCoinbaseTransaction(const CTransaction& tx, const CAmount blockSubsidy, TxValidationState& state);
 
     /**
      * This function should be called for each Asset Lock/Unlock tx
@@ -109,7 +109,7 @@ class CCreditPoolDiff {
     }
 
 private:
-    bool SetTarget(const CTransaction& tx, const CAmount blockReward, TxValidationState& state);
+    bool SetTarget(const CTransaction& tx, const CAmount blockSubsidy, TxValidationState& state);
     bool Lock(const CTransaction& tx, TxValidationState& state);
     bool Unlock(const CTransaction& tx, TxValidationState& state);
 };

src/evo/specialtxman.cpp

@@ -265,7 +265,7 @@ bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, CMNHF
 }
 
 bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams,
-                                const CAmount blockReward, BlockValidationState& state)
+                                const CAmount blockSubsidy, BlockValidationState& state)
 {
     AssertLockHeld(cs_main);
 
@@ -280,7 +280,7 @@ bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex,
         for (const auto& ptr_tx : block.vtx) {
             TxValidationState tx_state;
             if (ptr_tx->IsCoinBase()) {
-                if (!creditPoolDiff.ProcessCoinbaseTransaction(*ptr_tx, blockReward, tx_state)) {
+                if (!creditPoolDiff.ProcessCoinbaseTransaction(*ptr_tx, blockSubsidy, tx_state)) {
                     assert(tx_state.GetResult() == TxValidationResult::TX_CONSENSUS);
                     return state.Invalid(BlockValidationResult::BLOCK_CONSENSUS, tx_state.GetRejectReason(),
                                      strprintf("Process Coinbase Transaction failed at Credit Pool (tx hash %s) %s", ptr_tx->GetHash().ToString(), tx_state.GetDebugMessage()));

src/evo/specialtxman.h

@@ -34,6 +34,6 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, CM
 bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, CMNHFManager& mnhfManager,
                            llmq::CQuorumBlockProcessor& quorum_block_processor) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 bool CheckCreditPoolDiffForBlock(const CBlock& block, const CBlockIndex* pindex, const Consensus::Params& consensusParams,
-                                const CAmount blockReward, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
+                                const CAmount blockSubsidy, BlockValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 
 #endif // BITCOIN_EVO_SPECIALTXMAN_H

src/masternode/payments.cpp

@@ -24,17 +24,22 @@
 #include <cassert>
 #include <string>
 
-[[nodiscard]] static bool GetBlockTxOuts(const CBlockIndex* const pindexPrev, const CAmount blockReward, std::vector<CTxOut>& voutMasternodePaymentsRet)
+[[nodiscard]] static bool GetBlockTxOuts(const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, std::vector<CTxOut>& voutMasternodePaymentsRet)
 {
     voutMasternodePaymentsRet.clear();
 
     const int nBlockHeight = pindexPrev  == nullptr ? 0 : pindexPrev->nHeight + 1;
 
     bool fMNRewardReallocated =  llmq::utils::IsMNRewardReallocationActive(pindexPrev);
 
-    CAmount masternodeReward = GetMasternodePayment(nBlockHeight, blockReward, fMNRewardReallocated);
+    CAmount masternodeReward = GetMasternodePayment(nBlockHeight, blockSubsidy + feeReward, fMNRewardReallocated);
     if (fMNRewardReallocated) {
-        const CAmount platformReward = MasternodePayments::PlatformShare(masternodeReward);
+        CAmount masternodeSubsidyReward = GetMasternodePayment(nBlockHeight, blockSubsidy, fMNRewardReallocated);
+        // TODO remove this when we re-organize testnet
+        if (Params().NetworkIDString() == CBaseChainParams::TESTNET) {
+            masternodeSubsidyReward = masternodeReward;
+        }
+        const CAmount platformReward = MasternodePayments::PlatformShare(masternodeSubsidyReward);
         masternodeReward -= platformReward;
 
         assert(MoneyRange(masternodeReward));
@@ -73,12 +78,12 @@
 *
 *   Get masternode payment tx outputs
 */
-[[nodiscard]] static bool GetMasternodeTxOuts(const CBlockIndex* const pindexPrev, const CAmount blockReward, std::vector<CTxOut>& voutMasternodePaymentsRet)
+[[nodiscard]] static bool GetMasternodeTxOuts(const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward, std::vector<CTxOut>& voutMasternodePaymentsRet)
 {
     // make sure it's not filled yet
     voutMasternodePaymentsRet.clear();
 
-    if(!GetBlockTxOuts(pindexPrev, blockReward, voutMasternodePaymentsRet)) {
+    if(!GetBlockTxOuts(pindexPrev, blockSubsidy, feeReward, voutMasternodePaymentsRet)) {
         LogPrintf("MasternodePayments::%s -- no payee (deterministic masternode list empty)\n", __func__);
         return false;
     }
@@ -93,7 +98,7 @@
     return true;
 }
 
-[[nodiscard]] static bool IsTransactionValid(const CTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockReward)
+[[nodiscard]] static bool IsTransactionValid(const CTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward)
 {
     const int nBlockHeight = pindexPrev  == nullptr ? 0 : pindexPrev->nHeight + 1;
     if (!deterministicMNManager->IsDIP3Enforced(nBlockHeight)) {
@@ -102,7 +107,7 @@
     }
 
     std::vector<CTxOut> voutMasternodePayments;
-    if (!GetBlockTxOuts(pindexPrev, blockReward, voutMasternodePayments)) {
+    if (!GetBlockTxOuts(pindexPrev, blockSubsidy, feeReward, voutMasternodePayments)) {
         LogPrintf("MasternodePayments::%s -- ERROR failed to get payees for block at height %s\n", __func__, nBlockHeight);
         return true;
     }
@@ -261,7 +266,7 @@ bool IsBlockValueValid(const CSporkManager& sporkManager, CGovernanceManager& go
 }
 
 bool IsBlockPayeeValid(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
-                       const CTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockReward)
+                       const CTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward)
 {
     if(fDisableGovernance) {
         //there is no budget data to use to check anything, let's just accept the longest chain
@@ -288,7 +293,7 @@ bool IsBlockPayeeValid(const CSporkManager& sporkManager, CGovernanceManager& go
 
     if(AreSuperblocksEnabled(sporkManager)) {
         if(CSuperblockManager::IsSuperblockTriggered(governanceManager, nBlockHeight)) {
-            if(CSuperblockManager::IsValid(governanceManager, txNew, nBlockHeight, blockReward)) {
+            if(CSuperblockManager::IsValid(governanceManager, txNew, nBlockHeight, blockSubsidy + feeReward)) {
                 LogPrint(BCLog::GOBJECT, "%s -- Valid superblock at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */
                 // continue validation, should also pay MN
             } else {
@@ -305,7 +310,7 @@ bool IsBlockPayeeValid(const CSporkManager& sporkManager, CGovernanceManager& go
     }
 
     // Check for correct masternode payment
-    if(IsTransactionValid(txNew, pindexPrev, blockReward)) {
+    if(IsTransactionValid(txNew, pindexPrev, blockSubsidy, feeReward)) {
         LogPrint(BCLog::MNPAYMENTS, "%s -- Valid masternode payment at height %d: %s", __func__, nBlockHeight, txNew.ToString()); /* Continued */
         return true;
     }
@@ -315,7 +320,7 @@ bool IsBlockPayeeValid(const CSporkManager& sporkManager, CGovernanceManager& go
 }
 
 void FillBlockPayments(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
-                       CMutableTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockReward,
+                       CMutableTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward,
                        std::vector<CTxOut>& voutMasternodePaymentsRet, std::vector<CTxOut>& voutSuperblockPaymentsRet)
 {
     int nBlockHeight = pindexPrev == nullptr ? 0 : pindexPrev->nHeight + 1;
@@ -327,7 +332,7 @@ void FillBlockPayments(const CSporkManager& sporkManager, CGovernanceManager& go
         CSuperblockManager::GetSuperblockPayments(governanceManager, nBlockHeight, voutSuperblockPaymentsRet);
     }
 
-    if (!GetMasternodeTxOuts(pindexPrev, blockReward, voutMasternodePaymentsRet)) {
+    if (!GetMasternodeTxOuts(pindexPrev, blockSubsidy, feeReward, voutMasternodePaymentsRet)) {
         LogPrint(BCLog::MNPAYMENTS, "%s -- no masternode to pay (MN list probably empty)\n", __func__);
     }
 
@@ -344,7 +349,7 @@ void FillBlockPayments(const CSporkManager& sporkManager, CGovernanceManager& go
     }
 
     LogPrint(BCLog::MNPAYMENTS, "%s -- nBlockHeight %d blockReward %lld voutMasternodePaymentsRet \"%s\" txNew %s", __func__, /* Continued */
-                            nBlockHeight, blockReward, voutMasternodeStr, txNew.ToString());
+                            nBlockHeight, blockSubsidy + feeReward, voutMasternodeStr, txNew.ToString());
 }
 
 CAmount PlatformShare(const CAmount reward)

src/masternode/payments.h

@@ -29,9 +29,9 @@ namespace MasternodePayments
 bool IsBlockValueValid(const CSporkManager& sporkManager, CGovernanceManager& governanceManager, const CMasternodeSync& mn_sync,
                        const CBlock& block, const int nBlockHeight, const CAmount blockReward, std::string& strErrorRet);
 bool IsBlockPayeeValid(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
-                       const CTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockReward);
+                       const CTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward);
 void FillBlockPayments(const CSporkManager& sporkManager, CGovernanceManager& governanceManager,
-                       CMutableTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockReward,
+                       CMutableTransaction& txNew, const CBlockIndex* const pindexPrev, const CAmount blockSubsidy, const CAmount feeReward,
                        std::vector<CTxOut>& voutMasternodePaymentsRet, std::vector<CTxOut>& voutSuperblockPaymentsRet);
 
 /**

src/miner.cpp

@@ -194,7 +194,8 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
     // NOTE: unlike in bitcoin, we need to pass PREVIOUS block height here
     bool fMNRewardReallocated = llmq::utils::IsMNRewardReallocationActive(pindexPrev);
     bool fV20Active = llmq::utils::IsV20Active(pindexPrev);
-    CAmount blockReward = nFees + GetBlockSubsidyInner(pindexPrev->nBits, pindexPrev->nHeight, Params().GetConsensus(), fV20Active, fMNRewardReallocated);
+    CAmount blockSubsidy = GetBlockSubsidyInner(pindexPrev->nBits, pindexPrev->nHeight, Params().GetConsensus(), fV20Active, fMNRewardReallocated);
+    CAmount blockReward = blockSubsidy + nFees;
 
     // Compute regular coinbase transaction.
     coinbaseTx.vout[0].nValue = blockReward;
@@ -237,7 +238,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
                 assert(creditPoolDiff != std::nullopt);
 
                 if (fMNRewardReallocated) {
-                    const CAmount masternodeReward = GetMasternodePayment(nHeight, blockReward, fMNRewardReallocated);
+                    const CAmount masternodeReward = GetMasternodePayment(nHeight, blockSubsidy, fMNRewardReallocated);
                     const CAmount reallocedReward = MasternodePayments::PlatformShare(masternodeReward);
                     LogPrint(BCLog::MNPAYMENTS, "%s: add MN reward %lld (%lld) to credit pool\n", __func__, masternodeReward, reallocedReward);
                     creditPoolDiff->AddRewardRealloced(reallocedReward);
@@ -251,7 +252,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
 
     // Update coinbase transaction with additional info about masternode and governance payments,
     // get some info back to pass to getblocktemplate
-    MasternodePayments::FillBlockPayments(spork_manager, governance_manager, coinbaseTx, pindexPrev, blockReward, pblocktemplate->voutMasternodePayments, pblocktemplate->voutSuperblockPayments);
+    MasternodePayments::FillBlockPayments(spork_manager, governance_manager, coinbaseTx, pindexPrev, blockSubsidy, nFees, pblocktemplate->voutMasternodePayments, pblocktemplate->voutSuperblockPayments);
 
     pblock->vtx[0] = MakeTransactionRef(std::move(coinbaseTx));
     pblocktemplate->vTxFees[0] = -nFees;

src/rpc/masternode.cpp

@@ -457,8 +457,8 @@ static UniValue masternode_payments(const JSONRPCRequest& request, const Chainst
 
         std::vector<CTxOut> voutMasternodePayments, voutDummy;
         CMutableTransaction dummyTx;
-        CAmount blockReward = nBlockFees + GetBlockSubsidy(pindex, Params().GetConsensus());
-        MasternodePayments::FillBlockPayments(*sporkManager, *governance, dummyTx, pindex->pprev, blockReward, voutMasternodePayments, voutDummy);
+        CAmount blockSubsidy = GetBlockSubsidy(pindex, Params().GetConsensus());
+        MasternodePayments::FillBlockPayments(*sporkManager, *governance, dummyTx, pindex->pprev, blockSubsidy, nBlockFees, voutMasternodePayments, voutDummy);
 
         UniValue blockObj(UniValue::VOBJ);
         CAmount payedPerBlock{0};

src/validation.cpp

@@ -2397,21 +2397,23 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
     // DASH : MODIFIED TO CHECK MASTERNODE PAYMENTS AND SUPERBLOCKS
 
     // TODO: resync data (both ways?) and try to reprocess this block later.
-    CAmount blockReward = nFees + GetBlockSubsidy(pindex, m_params.GetConsensus());
+    CAmount blockSubsidy = GetBlockSubsidy(pindex, m_params.GetConsensus());
+    CAmount feeReward = nFees;
     std::string strError = "";
 
     int64_t nTime5_2 = GetTimeMicros(); nTimeSubsidy += nTime5_2 - nTime5_1;
     LogPrint(BCLog::BENCHMARK, "      - GetBlockSubsidy: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_2 - nTime5_1), nTimeSubsidy * MICRO, nTimeSubsidy * MILLI / nBlocksTotal);
 
-    if (!CheckCreditPoolDiffForBlock(block, pindex, m_params.GetConsensus(), blockReward, state)) {
+    // TODO remove "if testnet" condition when we re-organize testnet
+    if (!CheckCreditPoolDiffForBlock(block, pindex, m_params.GetConsensus(), (m_params.NetworkIDString() == CBaseChainParams::TESTNET) ? (blockSubsidy + feeReward) : blockSubsidy, state)) {
         return error("ConnectBlock(DASH): CheckCreditPoolDiffForBlock for block %s failed with %s",
                      pindex->GetBlockHash().ToString(), state.ToString());
     }
 
     int64_t nTime5_3 = GetTimeMicros(); nTimeCreditPool += nTime5_3 - nTime5_2;
     LogPrint(BCLog::BENCHMARK, "      - CheckCreditPoolDiffForBlock: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_3 - nTime5_2), nTimeCreditPool * MICRO, nTimeCreditPool * MILLI / nBlocksTotal);
 
-    if (!MasternodePayments::IsBlockValueValid(*sporkManager, *governance, *::masternodeSync, block, pindex->nHeight, blockReward, strError)) {
+    if (!MasternodePayments::IsBlockValueValid(*sporkManager, *governance, *::masternodeSync, block, pindex->nHeight, blockSubsidy + feeReward, strError)) {
         // 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");
@@ -2420,7 +2422,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state,
     int64_t nTime5_4 = GetTimeMicros(); nTimeValueValid += nTime5_4 - nTime5_3;
     LogPrint(BCLog::BENCHMARK, "      - IsBlockValueValid: %.2fms [%.2fs (%.2fms/blk)]\n", MILLI * (nTime5_4 - nTime5_3), nTimeValueValid * MICRO, nTimeValueValid * MILLI / nBlocksTotal);
 
-    if (!MasternodePayments::IsBlockPayeeValid(*sporkManager, *governance, *block.vtx[0], pindex->pprev, blockReward)) {
+    if (!MasternodePayments::IsBlockPayeeValid(*sporkManager, *governance, *block.vtx[0], pindex->pprev, blockSubsidy, feeReward)) {
         // NOTE: Do not punish, the node might be missing governance data
         LogPrintf("ERROR: ConnectBlock(DASH): couldn't find masternode or superblock payments\n");
         return state.Invalid(BlockValidationResult::BLOCK_RESULT_UNSET, "bad-cb-payee");

test/functional/feature_asset_locks.py

@@ -527,10 +527,7 @@ def run_test(self):
         new_total += platform_reward + COIN
         node.generate(1)
         self.sync_all()
-        # part of fee is going to master node reward
-        # these 2 conditions need to check a range
-        assert_greater_than(self.get_credit_pool_balance(), new_total)
-        assert_greater_than(new_total + tiny_amount, self.get_credit_pool_balance())
+        assert_equal(new_total, self.get_credit_pool_balance())
 
 
 if __name__ == '__main__':