Harden DIP3 activation (#2881)

* Harden DIP3 activation height

Also drop all related but no longer used parts.

* Pass current block index to GetCommitmentsFromBlock

* Allow to change dip3 activation height for tests

And fix them.

qa/rpc-tests/bip68-112-113-p2p.py

@@ -99,8 +99,9 @@ def __init__(self):
     def setup_network(self):
         # Must set the blockversion for this test
         # Must also set '-maxtipage=600100' to allow syncing from very old blocks
+        # and '-dip3params=2000:2000' to create pre-dip3 blocks only
         self.nodes = start_nodes(self.num_nodes, self.options.tmpdir,
-                                 extra_args=[['-whitelist=127.0.0.1', '-blockversion=4', '-maxtipage=600100']],
+                                 extra_args=[['-whitelist=127.0.0.1', '-blockversion=4', '-maxtipage=600100', '-dip3params=2000:2000']],
                                  binary=[self.options.testbinary])
 
     def run_test(self):

qa/rpc-tests/dip3-deterministicmns.py

@@ -24,7 +24,7 @@ def __init__(self):
 
         self.extra_args = ["-budgetparams=10:10:10"]
         self.extra_args += ["-sporkkey=cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK"]
-        self.extra_args += ["-bip9params=dip0003:0:999999999999:45:45", "-dip3enforcementheight=150"]
+        self.extra_args += ["-dip3params=135:150"]
 
     def setup_network(self):
         disable_mocktime()
@@ -69,7 +69,7 @@ def run_test(self):
         self.create_mn_collateral(self.nodes[0], before_dip3_mn)
         mns.append(before_dip3_mn)
 
-        # block 500 starts enforcing DIP3 MN payments
+        # block 150 starts enforcing DIP3 MN payments
         while self.nodes[0].getblockcount() < 150:
             self.nodes[0].generate(1)
 
@@ -80,7 +80,7 @@ def run_test(self):
         self.log.info("syncing blocks for all nodes")
         sync_blocks(self.nodes, timeout=120)
 
-        # DIP3 has activated here
+        # DIP3 is fully enforced here
 
         self.register_mn(self.nodes[0], before_dip3_mn)
         self.start_mn(before_dip3_mn)

qa/rpc-tests/p2p-fullblocktest.py

@@ -61,6 +61,12 @@ def __init__(self):
         self.tip = None
         self.blocks = {}
 
+    def setup_network(self):
+        # Must set '-dip3params=2000:2000' to create pre-dip3 blocks only
+        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir,
+                                 extra_args=[['-whitelist=127.0.0.1', '-dip3params=2000:2000']],
+                                 binary=[self.options.testbinary])
+
     def add_options(self, parser):
         super().add_options(parser)
         parser.add_option("--runbarelyexpensive", dest="runbarelyexpensive", default=True)

qa/rpc-tests/test_framework/test_framework.py

@@ -266,7 +266,7 @@ def __init__(self, num_nodes, masterodes_count, extra_args, fast_dip3_enforcemen
 
         self.fast_dip3_enforcement = fast_dip3_enforcement
         if fast_dip3_enforcement:
-            self.extra_args += ["-bip9params=dip0003:0:999999999999:10:5", "-dip3enforcementheight=50"]
+            self.extra_args += ["-dip3params=30:50"]
 
     def create_simple_node(self):
         idx = len(self.nodes)

src/chainparams.cpp

@@ -219,6 +219,7 @@ class CMainParams : public CChainParams {
         consensus.BIP65Height = 619382; // 00000000000076d8fcea02ec0963de4abfd01e771fec0863f960c2c64fe6f357
         consensus.BIP66Height = 245817; // 00000000000b1fa2dfa312863570e13fae9ca7b5566cb27e55422620b469aefa
         consensus.DIP0001Height = 782208;
+        consensus.DIP0003Height = 1028160;
         consensus.DIP0003EnforcementHeight = 1047200;
         consensus.DIP0003EnforcementHash = uint256S("000000000000002d1734087b4c5afc3133e4e1c3e1a89218f62bcd9bb3d17f81");
         consensus.powLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 20
@@ -394,6 +395,7 @@ class CTestNetParams : public CChainParams {
         consensus.BIP65Height = 2431; // 0000039cf01242c7f921dcb4806a5994bc003b48c1973ae0c89b67809c2bb2ab
         consensus.BIP66Height = 2075; // 0000002acdd29a14583540cb72e1c5cc83783560e38fa7081495d474fe1671f7
         consensus.DIP0001Height = 5500;
+        consensus.DIP0003Height = 7000;
         consensus.DIP0003EnforcementHeight = 7300;
         consensus.DIP0003EnforcementHash = uint256S("00000055ebc0e974ba3a3fb785c5ad4365a39637d4df168169ee80d313612f8f");
         consensus.powLimit = uint256S("00000fffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 20
@@ -552,6 +554,7 @@ class CDevNetParams : public CChainParams {
         consensus.BIP65Height = 1; // BIP65 activated immediately on devnet
         consensus.BIP66Height = 1; // BIP66 activated immediately on devnet
         consensus.DIP0001Height = 2; // DIP0001 activated immediately on devnet
+        consensus.DIP0003Height = 2; // DIP0003 activated immediately on devnet
         consensus.DIP0003EnforcementHeight = 2; // DIP0003 activated immediately on devnet
         consensus.DIP0003EnforcementHash = uint256();
         consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 1
@@ -719,6 +722,7 @@ class CRegTestParams : public CChainParams {
         consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in rpc activation tests)
         consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in rpc activation tests)
         consensus.DIP0001Height = 2000;
+        consensus.DIP0003Height = 432;
         consensus.DIP0003EnforcementHeight = 500;
         consensus.DIP0003EnforcementHash = uint256();
         consensus.powLimit = uint256S("7fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"); // ~uint256(0) >> 1
@@ -830,9 +834,10 @@ class CRegTestParams : public CChainParams {
         }
     }
 
-    void UpdateDIP3EnforcementHeight(int nHeight)
+    void UpdateDIP3Parameters(int nActivationHeight, int nEnforcementHeight)
     {
-        consensus.DIP0003EnforcementHeight = nHeight;
+        consensus.DIP0003Height = nActivationHeight;
+        consensus.DIP0003EnforcementHeight = nEnforcementHeight;
     }
 
     void UpdateBudgetParameters(int nMasternodePaymentsStartBlock, int nBudgetPaymentsStartBlock, int nSuperblockStartBlock)
@@ -883,9 +888,9 @@ void UpdateRegtestBIP9Parameters(Consensus::DeploymentPos d, int64_t nStartTime,
     regTestParams.UpdateBIP9Parameters(d, nStartTime, nTimeout, nWindowSize, nThreshold);
 }
 
-void UpdateRegtestDIP3EnforcementHeight(int nHeight)
+void UpdateRegtestDIP3Parameters(int nActivationHeight, int nEnforcementHeight)
 {
-    regTestParams.UpdateDIP3EnforcementHeight(nHeight);
+    regTestParams.UpdateDIP3Parameters(nActivationHeight, nEnforcementHeight);
 }
 
 void UpdateRegtestBudgetParameters(int nMasternodePaymentsStartBlock, int nBudgetPaymentsStartBlock, int nSuperblockStartBlock)

src/chainparams.h

@@ -148,9 +148,9 @@ void SelectParams(const std::string& chain);
 void UpdateRegtestBIP9Parameters(Consensus::DeploymentPos d, int64_t nStartTime, int64_t nTimeout, int64_t nWindowSize, int64_t nThreshold);
 
 /**
- * Allows modifying the DIP3 activation height
+ * Allows modifying the DIP3 activation and enforcement height
  */
-void UpdateRegtestDIP3EnforcementHeight(int nHeight);
+void UpdateRegtestDIP3Parameters(int nActivationHeight, int nEnforcementHeight);
 
 /**
  * Allows modifying the budget regtest parameters.

src/consensus/params.h

@@ -145,6 +145,8 @@ struct Params {
     int BIP66Height;
     /** Block height at which DIP0001 becomes active */
     int DIP0001Height;
+    /** Block height at which DIP0003 becomes active */
+    int DIP0003Height;
     /** Block height at which DIP0003 becomes enforced */
     int DIP0003EnforcementHeight;
     uint256 DIP0003EnforcementHash;

src/dsnotificationinterface.cpp

@@ -51,8 +51,7 @@ void CDSNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, con
     // Update global DIP0001 activation status
     fDIP0001ActiveAtTip = pindexNew->nHeight >= Params().GetConsensus().DIP0001Height;
     // update instantsend autolock activation flag (we reuse the DIP3 deployment)
-    instantsend.isAutoLockBip9Active =
-            (VersionBitsState(pindexNew, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE);
+    instantsend.isAutoLockBip9Active = pindexNew->nHeight + 1 >= Params().GetConsensus().DIP0003Height;
 
     if (fInitialDownload)
         return;

src/evo/deterministicmns.cpp

@@ -463,7 +463,8 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
 {
     AssertLockHeld(cs_main);
 
-    bool fDIP0003Active = VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+    const auto& consensusParams = Params().GetConsensus();
+    bool fDIP0003Active = pindex->nHeight >= consensusParams.DIP0003Height;
     if (!fDIP0003Active) {
         return true;
     }
@@ -507,7 +508,6 @@ bool CDeterministicMNManager::ProcessBlock(const CBlock& block, const CBlockInde
         uiInterface.NotifyMasternodeListChanged();
     }
 
-    const auto& consensusParams = Params().GetConsensus();
     if (nHeight == consensusParams.DIP0003EnforcementHeight) {
         if (!consensusParams.DIP0003EnforcementHash.IsNull() && consensusParams.DIP0003EnforcementHash != pindex->GetBlockHash()) {
             LogPrintf("CDeterministicMNManager::%s -- DIP3 enforcement block has wrong hash: hash=%s, expected=%s, nHeight=%d\n", __func__,

src/evo/specialtx.cpp

@@ -22,7 +22,7 @@ bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CVali
     if (tx.nVersion != 3 || tx.nType == TRANSACTION_NORMAL)
         return true;
 
-    if (pindexPrev && VersionBitsState(pindexPrev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) != THRESHOLD_ACTIVE) {
+    if (pindexPrev && pindexPrev->nHeight + 1 < Params().GetConsensus().DIP0003Height) {
         return state.DoS(10, false, REJECT_INVALID, "bad-tx-type");
     }
 

src/init.cpp

@@ -817,17 +817,8 @@ void ThreadImport(std::vector<boost::filesystem::path> vImportFiles)
     pdsNotificationInterface->InitializeCurrentBlockTip();
 
     if (fMasternodeMode) {
-        bool fDIP003Active{false};
-        {
-            LOCK(cs_main);
-            if (chainActive.Tip()->pprev) {
-                fDIP003Active = VersionBitsState(chainActive.Tip()->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
-            }
-        }
-        if (fDIP003Active) {
-            assert(activeMasternodeManager);
-            activeMasternodeManager->Init();
-        }
+        assert(activeMasternodeManager);
+        activeMasternodeManager->Init();
     }
 
 #ifdef ENABLE_WALLET
@@ -1336,8 +1327,25 @@ bool AppInitParameterInteraction()
         }
     }
 
-    if (IsArgSet("-dip3enforcementheight")) {
-        UpdateRegtestDIP3EnforcementHeight(GetArg("-dip3enforcementheight", 0));
+    if (IsArgSet("-dip3params")) {
+        // Allow overriding budget parameters for testing
+        if (!chainparams.MineBlocksOnDemand()) {
+            return InitError("DIP3 parameters may only be overridden on regtest.");
+        }
+        std::string strDIP3Params = GetArg("-dip3params", "");
+        std::vector<std::string> vDIP3Params;
+        boost::split(vDIP3Params, strDIP3Params, boost::is_any_of(":"));
+        if (vDIP3Params.size() != 2) {
+            return InitError("DIP3 parameters malformed, expecting DIP3ActivationHeight:DIP3EnforcementHeight");
+        }
+        int nDIP3ActivationHeight, nDIP3EnforcementHeight;
+        if (!ParseInt32(vDIP3Params[0], &nDIP3ActivationHeight)) {
+            return InitError(strprintf("Invalid nDIP3ActivationHeight (%s)", vDIP3Params[0]));
+        }
+        if (!ParseInt32(vDIP3Params[1], &nDIP3EnforcementHeight)) {
+            return InitError(strprintf("Invalid nDIP3EnforcementHeight (%s)", vDIP3Params[1]));
+        }
+        UpdateRegtestDIP3Parameters(nDIP3ActivationHeight, nDIP3EnforcementHeight);
     }
 
     if (IsArgSet("-budgetparams")) {

src/llmq/quorums_blockprocessor.cpp

@@ -119,14 +119,14 @@ bool CQuorumBlockProcessor::ProcessBlock(const CBlock& block, const CBlockIndex*
 {
     AssertLockHeld(cs_main);
 
-    bool fDIP0003Active = VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+    bool fDIP0003Active = pindex->nHeight >= Params().GetConsensus().DIP0003Height;
     if (!fDIP0003Active) {
         evoDb.Write(DB_BEST_BLOCK_UPGRADE, block.GetHash());
         return true;
     }
 
     std::map<Consensus::LLMQType, CFinalCommitment> qcs;
-    if (!GetCommitmentsFromBlock(block, pindex->pprev, qcs, state)) {
+    if (!GetCommitmentsFromBlock(block, pindex, qcs, state)) {
         return false;
     }
 
@@ -231,7 +231,7 @@ bool CQuorumBlockProcessor::UndoBlock(const CBlock& block, const CBlockIndex* pi
 
     std::map<Consensus::LLMQType, CFinalCommitment> qcs;
     CValidationState dummy;
-    if (!GetCommitmentsFromBlock(block, pindex->pprev, qcs, dummy)) {
+    if (!GetCommitmentsFromBlock(block, pindex, qcs, dummy)) {
         return false;
     }
 
@@ -277,7 +277,7 @@ void CQuorumBlockProcessor::UpgradeDB()
 
             std::map<Consensus::LLMQType, CFinalCommitment> qcs;
             CValidationState dummyState;
-            GetCommitmentsFromBlock(block, pindex->pprev, qcs, dummyState);
+            GetCommitmentsFromBlock(block, pindex, qcs, dummyState);
 
             for (const auto& p : qcs) {
                 const auto& qc = p.second;
@@ -298,12 +298,12 @@ void CQuorumBlockProcessor::UpgradeDB()
     LogPrintf("CQuorumBlockProcessor::%s -- Upgrade done...\n", __func__);
 }
 
-bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, std::map<Consensus::LLMQType, CFinalCommitment>& ret, CValidationState& state)
+bool CQuorumBlockProcessor::GetCommitmentsFromBlock(const CBlock& block, const CBlockIndex* pindex, std::map<Consensus::LLMQType, CFinalCommitment>& ret, CValidationState& state)
 {
     AssertLockHeld(cs_main);
 
     auto& consensus = Params().GetConsensus();
-    bool fDIP0003Active = VersionBitsState(pindexPrev, consensus, Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+    bool fDIP0003Active = pindex->nHeight >= consensus.DIP0003Height;
 
     ret.clear();
 

src/llmq/quorums_blockprocessor.h

@@ -57,7 +57,7 @@ class CQuorumBlockProcessor
     std::map<Consensus::LLMQType, std::vector<const CBlockIndex*>> GetMinedAndActiveCommitmentsUntilBlock(const CBlockIndex* pindex);
 
 private:
-    bool GetCommitmentsFromBlock(const CBlock& block, const CBlockIndex* pindexPrev, std::map<Consensus::LLMQType, CFinalCommitment>& ret, CValidationState& state);
+    bool GetCommitmentsFromBlock(const CBlock& block, const CBlockIndex* pindex, std::map<Consensus::LLMQType, CFinalCommitment>& ret, CValidationState& state);
     bool ProcessCommitment(int nHeight, const uint256& blockHash, const CFinalCommitment& qc, CValidationState& state);
     bool IsMiningPhase(Consensus::LLMQType llmqType, int nHeight);
     bool IsCommitmentRequired(Consensus::LLMQType llmqType, int nHeight);

src/miner.cpp

@@ -146,12 +146,12 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
 
     LOCK2(cs_main, mempool.cs);
 
-    bool fDIP0003Active_context = VersionBitsState(chainActive.Tip(), chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
-    bool fDIP0008Active_context = VersionBitsState(chainActive.Tip(), chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0008, versionbitscache) == THRESHOLD_ACTIVE;
-
     CBlockIndex* pindexPrev = chainActive.Tip();
     nHeight = pindexPrev->nHeight + 1;
 
+    bool fDIP0003Active_context = nHeight >= chainparams.GetConsensus().DIP0003Height;
+    bool fDIP0008Active_context = VersionBitsState(chainActive.Tip(), chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0008, versionbitscache) == THRESHOLD_ACTIVE;
+
     pblock->nVersion = ComputeBlockVersion(pindexPrev, chainparams.GetConsensus(), chainparams.BIP9CheckMasternodesUpgraded());
     // -regtest only: allow overriding block.nVersion with
     // -blockversion=N to test forking scenarios
@@ -166,7 +166,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
                        : pblock->GetBlockTime();
 
     if (fDIP0003Active_context) {
-        for (auto& p : Params().GetConsensus().llmqs) {
+        for (auto& p : chainparams.GetConsensus().llmqs) {
             CTransactionRef qcTx;
             if (llmq::quorumBlockProcessor->GetMinableCommitmentTx(p.first, nHeight, qcTx)) {
                 pblock->vtx.emplace_back(qcTx);

src/validation.cpp

@@ -578,7 +578,7 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state,
 {
     int nHeight = pindexPrev == NULL ? 0 : pindexPrev->nHeight + 1;
     bool fDIP0001Active_context = nHeight >= consensusParams.DIP0001Height;
-    bool fDIP0003Active_context = VersionBitsState(pindexPrev, consensusParams, Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+    bool fDIP0003Active_context = nHeight >= consensusParams.DIP0003Height;
 
     if (fDIP0003Active_context) {
         // check version 3 transaction types
@@ -1607,7 +1607,7 @@ static DisconnectResult DisconnectBlock(const CBlock& block, CValidationState& s
 {
     assert(pindex->GetBlockHash() == view.GetBestBlock());
 
-    bool fDIP0003Active = VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+    bool fDIP0003Active = pindex->nHeight >= Params().GetConsensus().DIP0003Height;
     bool fHasBestBlock = evoDb->VerifyBestBlock(pindex->GetBlockHash());
 
     if (fDIP0003Active && !fHasBestBlock) {
@@ -1768,8 +1768,7 @@ static DisconnectResult DisconnectBlock(const CBlock& block, CValidationState& s
 
     // make sure the flag is reset in case of a chain reorg
     // (we reused the DIP3 deployment)
-    instantsend.isAutoLockBip9Active =
-            (VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE);
+    instantsend.isAutoLockBip9Active = pindex->nHeight >= Params().GetConsensus().DIP0003Height;
 
     evoDb->WriteBestBlock(pindex->pprev->GetBlockHash());
 
@@ -1903,7 +1902,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
     assert(hashPrevBlock == view.GetBestBlock());
 
     if (pindex->pprev) {
-        bool fDIP0003Active = VersionBitsState(pindex->pprev, Params().GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+        bool fDIP0003Active = pindex->nHeight >= chainparams.GetConsensus().DIP0003Height;
         bool fHasBestBlock = evoDb->VerifyBestBlock(pindex->pprev->GetBlockHash());
 
         if (fDIP0003Active && !fHasBestBlock) {
@@ -2300,14 +2299,6 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
     // add this block to the view's block chain
     view.SetBestBlock(pindex->GetBlockHash());
 
-    if (!fJustCheck) {
-        // check if previous block had DIP3 disabled and the new block has it enabled
-        if (VersionBitsState(pindex->pprev, chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) != THRESHOLD_ACTIVE &&
-            VersionBitsState(pindex, chainparams.GetConsensus(), Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE) {
-            LogPrintf("%s -- DIP0003 got activated at height %d\n", __func__, pindex->nHeight);
-        }
-    }
-
     int64_t nTime6 = GetTimeMicros(); nTimeIndex += nTime6 - nTime5;
     LogPrint("bench", "    - Index writing: %.2fms [%.2fs]\n", 0.001 * (nTime6 - nTime5), nTimeIndex * 0.000001);
 
@@ -3375,8 +3366,8 @@ bool ContextualCheckBlock(const CBlock& block, CValidationState& state, const Co
                               ? pindexPrev->GetMedianTimePast()
                               : block.GetBlockTime();
 
-    bool fDIP0001Active_context = nHeight >= Params().GetConsensus().DIP0001Height;
-    bool fDIP0003Active_context = VersionBitsState(pindexPrev, consensusParams, Consensus::DEPLOYMENT_DIP0003, versionbitscache) == THRESHOLD_ACTIVE;
+    bool fDIP0001Active_context = nHeight >= consensusParams.DIP0001Height;
+    bool fDIP0003Active_context = nHeight >= consensusParams.DIP0003Height;
 
     // Size limits
     unsigned int nMaxBlockSize = MaxBlockSize(fDIP0001Active_context);