From ad8bf09650990d6f319c1303002db43bdcc7a0f8 Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 19 Jun 2021 16:25:31 +0200 Subject: [PATCH 1/2] [Refactor] Decouple enforcement of cbase payment from block version New rules shouldn't be enforced with a new block version (e.g. IsCoinbaseValueValid), because there might be blocks with that version *before* the relative network enforcement. And in this case we don't need to, as the check is already contextual. --- src/blockassembler.cpp | 6 ++-- src/budget/budgetmanager.cpp | 2 +- src/masternode-payments.cpp | 2 +- src/masternode-payments.h | 2 +- src/primitives/block.h | 2 +- src/validation.cpp | 38 ++++++++++----------- test/functional/tiertwo_mn_compatibility.py | 3 +- 7 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/blockassembler.cpp b/src/blockassembler.cpp index bf6c3d371a1e..74222c7de3a9 100644 --- a/src/blockassembler.cpp +++ b/src/blockassembler.cpp @@ -519,10 +519,8 @@ void IncrementExtraNonce(std::shared_ptr& 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; // v9 (since PIVX 5.0.1) } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V4_0)) { return 7; } else if (consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V3_4)) { diff --git a/src/budget/budgetmanager.cpp b/src/budget/budgetmanager.cpp index 3abdd8e61b78..5d6f513a69bd 100644 --- a/src/budget/budgetmanager.cpp +++ b/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); diff --git a/src/masternode-payments.cpp b/src/masternode-payments.cpp index 5c029749bfc9..18069f12999b 100644 --- a/src/masternode-payments.cpp +++ b/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); diff --git a/src/masternode-payments.h b/src/masternode-payments.h index db90697df0c1..1995e4f43460 100644 --- a/src/masternode-payments.h +++ b/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); diff --git a/src/primitives/block.h b/src/primitives/block.h index 786a6ab7eeba..8e28bc6b7278 100644 --- a/src/primitives/block.h +++ b/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=9; int32_t nVersion; uint256 hashPrevBlock; uint256 hashMerkleRoot; diff --git a/src/validation.cpp b/src/validation.cpp index a907e2b78e58..90056482f25f 100644 --- a/src/validation.cpp +++ b/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 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); diff --git a/test/functional/tiertwo_mn_compatibility.py b/test/functional/tiertwo_mn_compatibility.py index fa407cb62760..d1abb81d991e 100755 --- a/test/functional/tiertwo_mn_compatibility.py +++ b/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") From 84261bcc6c4b58db4e36ef6ef1838d2743e19f3a Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 19 Jun 2021 16:39:54 +0200 Subject: [PATCH 2/2] [Validation] Bump default block version to 10 (no enforcement) This commit will be backported to 5.2 branch to signal the new release. Before 6.0 there will be another block version bump in master. --- src/blockassembler.cpp | 2 +- src/primitives/block.h | 2 +- test/functional/test_framework/messages.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/blockassembler.cpp b/src/blockassembler.cpp index 74222c7de3a9..9df83ef0ce30 100644 --- a/src/blockassembler.cpp +++ b/src/blockassembler.cpp @@ -520,7 +520,7 @@ void IncrementExtraNonce(std::shared_ptr& pblock, int nHeight, unsigned int32_t ComputeBlockVersion(const Consensus::Params& consensus, int nHeight) { if (NetworkUpgradeActive(nHeight, consensus, Consensus::UPGRADE_V5_0)) { - return CBlockHeader::CURRENT_VERSION; // v9 (since PIVX 5.0.1) + 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)) { diff --git a/src/primitives/block.h b/src/primitives/block.h index 8e28bc6b7278..def939b83a51 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -23,7 +23,7 @@ class CBlockHeader { public: // header - static const int32_t CURRENT_VERSION=9; + static const int32_t CURRENT_VERSION=10; // since v5.1.99 int32_t nVersion; uint256 hashPrevBlock; uint256 hashMerkleRoot; diff --git a/test/functional/test_framework/messages.py b/test/functional/test_framework/messages.py index 8408d86b3ee9..7ff905495495 100644 --- a/test/functional/test_framework/messages.py +++ b/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