[Refactor] Decouple enforcement of cbase payment from block version

src/blockassembler.cpp

@@ -519,10 +519,8 @@ void IncrementExtraNonce(std::shared_ptr<CBlock>& pblock, int nHeight, unsigned
 
 int32_t ComputeBlockVersion(const Consensus::Params& consensus, int nHeight)
 {
-    if (NetworkUpgradeActive(nHeight, consensus, Consensus::UPGRADE_V6_0)) {
-        return CBlockHeader::CURRENT_VERSION;    // v10
-    } else if (NetworkUpgradeActive(nHeight, consensus, Consensus::UPGRADE_V5_0)) {
-        return 9;
+    if (NetworkUpgradeActive(nHeight, consensus, Consensus::UPGRADE_V5_0)) {
+        return CBlockHeader::CURRENT_VERSION;       // v10 (since 5.1.99)
     } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V4_0)) {
         return 7;
     } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V3_4)) {

src/budget/budgetmanager.cpp

@@ -437,7 +437,7 @@ bool CBudgetManager::FillBlockPayee(CMutableTransaction& txCoinbase, CMutableTra
 
     CAmount blockValue = GetBlockValue(nHeight);
 
-    // Starting from PIVX v6.0 masternode and budgets are paid in the coinbase tx of PoS blocks (block v10)
+    // Starting from PIVX v6.0 masternode and budgets are paid in the coinbase tx of PoS blocks
     const bool fPayCoinstake = fProofOfStake &&
                                !Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V6_0);
 

src/masternode-payments.cpp

@@ -366,7 +366,7 @@ void CMasternodePayments::FillBlockPayee(CMutableTransaction& txCoinbase, CMutab
         return;
     }
 
-    // Starting from PIVX v6.0 masternode and budgets are paid in the coinbase tx (block v10)
+    // Starting from PIVX v6.0 masternode and budgets are paid in the coinbase tx
     const int nHeight = pindexPrev->nHeight + 1;
     bool fPayCoinstake = fProofOfStake && !Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V6_0);
 

src/masternode-payments.h

@@ -31,7 +31,7 @@ bool IsBlockValueValid(int nHeight, CAmount& nExpectedValue, CAmount nMinted, CA
 void FillBlockPayee(CMutableTransaction& txCoinbase, CMutableTransaction& txCoinstake, const CBlockIndex* pindexPrev, bool fProofOfStake);
 
 /**
- * Check coinbase output value for blocks v10+.
+ * Check coinbase output value for blocks after v6.0 enforcement.
  * It must pay the masternode for regular blocks and a proposal during superblocks.
  */
 bool IsCoinbaseValueValid(const CTransactionRef& tx, CAmount nBudgetAmt, CValidationState& _state);

src/primitives/block.h

@@ -23,7 +23,7 @@ class CBlockHeader
 {
 public:
     // header
-    static const int32_t CURRENT_VERSION=10;     //!> Version 10 Masternode/Budget payments in the coinbase
+    static const int32_t CURRENT_VERSION=10;    // since v5.1.99
     int32_t nVersion;
     uint256 hashPrevBlock;
     uint256 hashMerkleRoot;

src/validation.cpp

@@ -1482,17 +1482,24 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
         LogPrintf("%s: hashPrev=%s view=%s\n", __func__, hashPrevBlock.GetHex(), view.GetBestBlock().GetHex());
     assert(hashPrevBlock == view.GetBestBlock());
 
+    const bool isPoSBlock = block.IsProofOfStake();
+    const Consensus::Params& consensus = Params().GetConsensus();
+    const bool isPoSActive = consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_POS);
+    const bool isV5UpgradeEnforced = consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_V5_0);
+    const bool isV6UpgradeEnforced = consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_V6_0);
+
+    // Coinbase output should be empty if proof-of-stake block (before v6 enforcement)
+    if (!isV6UpgradeEnforced && isPoSBlock && (block.vtx[0]->vout.size() != 1 || !block.vtx[0]->vout[0].IsEmpty()))
+        return state.DoS(100, false, REJECT_INVALID, "bad-cb-pos", false, "coinbase output not empty for proof-of-stake block");
+
     if (pindex->pprev) {
-        bool fDIP3Active = Params().GetConsensus().NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_V6_0);
         bool fHasBestBlock = evoDb->VerifyBestBlock(hashPrevBlock);
 
-        if (fDIP3Active && !fHasBestBlock) {
+        if (isV6UpgradeEnforced && !fHasBestBlock) {
             return AbortNode(state, "Found EvoDB inconsistency, you must reindex to continue");
         }
     }
 
-    const Consensus::Params& consensus = Params().GetConsensus();
-
     // Special case for the genesis block, skipping connection of its transactions
     // (its coinbase is unspendable)
     if (block.GetHash() == consensus.hashGenesisBlock) {
@@ -1502,12 +1509,11 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
         return true;
     }
 
-    const bool isPoSActive = consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_POS);
-    if (!isPoSActive && block.IsProofOfStake())
+    if (!isPoSActive && isPoSBlock)
         return state.DoS(100, error("ConnectBlock() : PoS period not active"),
             REJECT_INVALID, "PoS-early");
 
-    if (isPoSActive && block.IsProofOfWork())
+    if (isPoSActive && !isPoSBlock)
         return state.DoS(100, error("ConnectBlock() : PoW period ended"),
             REJECT_INVALID, "PoW-ended");
 
@@ -1557,9 +1563,6 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
     SaplingMerkleTree sapling_tree;
     assert(view.GetSaplingAnchorAt(view.GetBestAnchor(), sapling_tree));
 
-    //
-    bool isV5UpgradeEnforced = consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_V5_0);
-
     std::vector<PrecomputedTransactionData> precomTxData;
     precomTxData.reserve(block.vtx.size()); // Required so that pointers to individual precomTxData don't get invalidated
     bool fInitialBlockDownload = IsInitialBlockDownload();
@@ -1692,7 +1695,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
     view.PushAnchor(sapling_tree);
 
     // Verify header correctness
-    if (consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_V5_0)) {
+    if (isV5UpgradeEnforced) {
         // If Sapling is active, block.hashFinalSaplingRoot must be the
         // same as the root of the Sapling tree
         if (block.hashFinalSaplingRoot != sapling_tree.root()) {
@@ -1711,7 +1714,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
 
     //PoW phase redistributed fees to miner. PoS stage destroys fees.
     CAmount nExpectedMint = GetBlockValue(pindex->nHeight);
-    if (block.IsProofOfWork())
+    if (!isPoSBlock)
         nExpectedMint += nFees;
 
     //Check that the block does not overmint
@@ -1731,8 +1734,8 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
         }
     }
 
-    // For blocks v10+: Check that the coinbase pays the exact amount
-    if (isPoSActive && pindex->nVersion >= 10 && !IsCoinbaseValueValid(block.vtx[0], nBudgetAmt, state)) {
+    // After v6 enforcement: Check that the coinbase pays the exact amount
+    if (isPoSBlock && isV6UpgradeEnforced && !IsCoinbaseValueValid(block.vtx[0], nBudgetAmt, state)) {
         // pass the state returned by the function above
         return false;
     }
@@ -2800,10 +2803,6 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
             return state.DoS(100, false, REJECT_INVALID, "bad-cb-multiple", false, "more than one coinbase");
 
     if (IsPoS) {
-        // Coinbase output should be empty if proof-of-stake block (before block v10)
-        if (block.nVersion < 10 && (block.vtx[0]->vout.size() != 1 || !block.vtx[0]->vout[0].IsEmpty()))
-            return state.DoS(100, false, REJECT_INVALID, "bad-cb-pos", false, "coinbase output not empty for proof-of-stake block");
-
         // Second transaction must be coinstake, the rest must not be
         if (block.vtx.empty() || !block.vtx[1]->IsCoinStake())
             return state.DoS(100, false, REJECT_INVALID, "bad-cs-missing", false, "second tx is not coinstake");
@@ -2993,8 +2992,7 @@ bool ContextualCheckBlockHeader(const CBlockHeader& block, CValidationState& sta
         (block.nVersion < 5 && consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_BIP65)) ||
         (block.nVersion < 6 && consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V3_4)) ||
         (block.nVersion < 7 && consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V4_0)) ||
-        (block.nVersion < 8 && consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0)) ||
-        (block.nVersion < 10 && consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V6_0)))
+        (block.nVersion < 8 && consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0)))
     {
         std::string stringErr = strprintf("rejected block version %d at height %d", block.nVersion, nHeight);
         return state.Invalid(false, REJECT_OBSOLETE, "bad-version", stringErr);

test/functional/test_framework/messages.py

@@ -33,7 +33,7 @@
 
 MAX_INV_SZ = 50000
 MAX_BLOCK_BASE_SIZE = 2000000
-CURRENT_BLK_VERSION = 9
+CURRENT_BLK_VERSION = 10
 
 COIN = 100000000 # 1 btc in satoshis
 

test/functional/tiertwo_mn_compatibility.py

@@ -61,13 +61,12 @@ def check_mns_status(self, node, txhash):
         assert_equal(status["status"], "Ready")
 
     """
-    Checks the block at specified height (it must be a v10 block).
+    Checks the block at specified height
     Returns the address of the mn paid (in the coinbase), and the json coinstake tx
     """
     def get_block_mnwinner(self, height):
         blk = self.miner.getblock(self.miner.getblockhash(height), True)
         assert_equal(blk['height'], height)
-        assert_equal(blk['version'], 10)
         cbase_tx = self.miner.getrawtransaction(blk['tx'][0], True)
         assert_equal(len(cbase_tx['vin']), 1)
         cbase_script = height.to_bytes(1 + height // 256, byteorder="little")