[Refactoring] Remove zerocoin disabling check in ATMP and CheckBlock

as we already checking it via ContextualCheckBlock -->
ContextualCheckTransaction --> ContextualCheckZerocoinTx

Author: random-zebra

src/consensus/tx_verify.cpp

@@ -134,7 +134,7 @@ bool ContextualCheckTransaction(const CTransactionRef& tx, CValidationState& sta
     }
 
     // Dispatch to ZerocoinTx validator
-    if (!ContextualCheckZerocoinTx(tx, state, chainparams.GetConsensus(), nHeight)) {
+    if (!ContextualCheckZerocoinTx(tx, state, chainparams.GetConsensus(), nHeight, isMined)) {
         return false; // Failure reason has been set in validation state object
     }
 

src/consensus/zerocoin_verify.cpp

@@ -127,13 +127,13 @@ bool CheckPublicCoinSpendEnforced(int blockHeight, bool isPublicSpend)
     return true;
 }
 
-bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight)
+bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight, bool isMined)
 {
     // zerocoin enforced via block time. First block with a zc mint is 863735
     const bool fZerocoinEnforced = (nHeight >= consensus.ZC_HeightStart);
     const bool fPublicSpendEnforced = consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_ZC_PUBLIC);
-    const bool fRejectMintsAndPrivateSpends = !fZerocoinEnforced || fPublicSpendEnforced;
-    const bool fRejectPublicSpends = !fPublicSpendEnforced || consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0);
+    const bool fRejectMintsAndPrivateSpends = !isMined || !fZerocoinEnforced || fPublicSpendEnforced;
+    const bool fRejectPublicSpends = !isMined || !fPublicSpendEnforced || consensus.NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0);
 
     const bool hasPrivateSpendInputs = !tx->vin.empty() && tx->vin[0].IsZerocoinSpend();
     const bool hasPublicSpendInputs = !tx->vin.empty() && tx->vin[0].IsZerocoinPublicSpend();

src/consensus/zerocoin_verify.h

@@ -13,7 +13,7 @@
 bool isBlockBetweenFakeSerialAttackRange(int nHeight);
 // Public coin spend
 bool CheckPublicCoinSpendEnforced(int blockHeight, bool isPublicSpend);
-bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight);
+bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight, bool isMined);
 bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight);
 bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight);
 

src/test/validation_tests.cpp

@@ -94,25 +94,25 @@ BOOST_FIXTURE_TEST_CASE(test_simple_shielded_invalid, TestingSetup)
     }
 }
 
-void CheckBlockZcRejection(std::shared_ptr<CBlock>& pblock, int nHeight, CMutableTransaction& mtx)
+void CheckBlockZcRejection(std::shared_ptr<CBlock>& pblock, int nHeight, CMutableTransaction& mtx, const std::string& expected_msg)
 {
     pblock->vtx.emplace_back(MakeTransactionRef(mtx));
     BOOST_CHECK(SolveBlock(pblock, nHeight));
     BlockStateCatcher stateCatcher(pblock->GetHash());
     stateCatcher.registerEvent();
     BOOST_CHECK(!ProcessNewBlock(pblock, nullptr));
     BOOST_CHECK(stateCatcher.found && !stateCatcher.state.IsValid());
-    BOOST_CHECK_EQUAL(stateCatcher.state.GetRejectReason(), "bad-blk-with-zc");
+    BOOST_CHECK_EQUAL(stateCatcher.state.GetRejectReason(), expected_msg);
 }
 
-void CheckMempoolZcRejection(CMutableTransaction& mtx)
+void CheckMempoolZcRejection(CMutableTransaction& mtx, const std::string& expected_msg)
 {
     LOCK(cs_main);
     CValidationState state;
     BOOST_CHECK(!AcceptToMemoryPool(
             mempool, state, MakeTransactionRef(mtx), true, nullptr, false, true));
     BOOST_CHECK(!state.IsValid());
-    BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-tx-with-zc");
+    BOOST_CHECK_EQUAL(state.GetRejectReason(), expected_msg);
 }
 
 /*
@@ -121,6 +121,7 @@ void CheckMempoolZcRejection(CMutableTransaction& mtx)
 BOOST_FIXTURE_TEST_CASE(zerocoin_rejection_tests, WalletRegTestingSetup)
 {
     UpdateNetworkUpgradeParameters(Consensus::UPGRADE_V5_0, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
+    UpdateNetworkUpgradeParameters(Consensus::UPGRADE_ZC_PUBLIC, Consensus::NetworkUpgrade::ALWAYS_ACTIVE);
     const CChainParams& chainparams = Params();
 
     std::unique_ptr<CBlockTemplate> pblocktemplate;
@@ -140,21 +141,21 @@ BOOST_FIXTURE_TEST_CASE(zerocoin_rejection_tests, WalletRegTestingSetup)
                                          CBigNum::randBignum(chainparams.GetConsensus().Zerocoin_Params(false)->coinCommitmentGroup.groupOrder).getvch();
     mtx.vout[0].nValue = 1 * COIN;
     std::shared_ptr<CBlock> pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    CheckBlockZcRejection(pblock, 1, mtx);
-    CheckMempoolZcRejection(mtx);
+    CheckBlockZcRejection(pblock, 1, mtx, "bad-txns-zc-mint");
+    CheckMempoolZcRejection(mtx, "bad-txns-zc-mint");
 
     // Zerocoin spends rejection test
     mtx.vout[0].scriptPubKey = scriptPubKey;
     mtx.vin[0].scriptSig = CScript() << OP_ZEROCOINSPEND;
     pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    CheckBlockZcRejection(pblock, 1, mtx);
-    CheckMempoolZcRejection(mtx);
+    CheckBlockZcRejection(pblock, 1, mtx, "bad-txns-zc-private-spend");
+    CheckMempoolZcRejection(mtx, "bad-txns-zc-private-spend");
 
     // Zerocoin public spends rejection test
     mtx.vin[0].scriptSig = CScript() << OP_ZEROCOINPUBLICSPEND;
     pblock = std::make_shared<CBlock>(pblocktemplate->block);
-    CheckBlockZcRejection(pblock, 1, mtx);
-    CheckMempoolZcRejection(mtx);
+    CheckBlockZcRejection(pblock, 1, mtx, "bad-txns-zc-public-spend");
+    CheckMempoolZcRejection(mtx, "bad-txns-zc-public-spend");
 }
 
 BOOST_AUTO_TEST_SUITE_END()

src/validation.cpp

@@ -410,12 +410,6 @@ bool AcceptToMemoryPoolWorker(CTxMemPool& pool, CValidationState &state, const C
     const Consensus::Params& consensus = params.GetConsensus();
     int chainHeight = chainActive.Height();
 
-    // Zerocoin txes are not longer accepted in the mempool.
-    if (tx.ContainsZerocoins()) {
-        return state.DoS(100, error("%s : v5 upgrade enforced, zerocoin disabled", __func__),
-                         REJECT_INVALID, "bad-tx-with-zc");
-    }
-
     // Check transaction
     bool fColdStakingActive = !sporkManager.IsSporkActive(SPORK_19_COLDSTAKING_MAINTENANCE);
     if (!CheckTransaction(tx, state, fColdStakingActive))
@@ -1549,7 +1543,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
             return state.DoS(100, error("%s : shielded transactions are currently in maintenance mode", __func__));
         }
 
-        // When v5 is enforced CheckBlock rejects zerocoin transactions.
+        // When v5 is enforced ContextualCheckTransaction rejects zerocoin transactions.
         // Therefore no need to call HasZerocoinSpendInputs after the enforcement.
         if (!isV5UpgradeEnforced && tx.HasZerocoinSpendInputs()) {
             auto opCoinSpendValues = ParseAndValidateZerocoinSpends(consensus, tx, pindex->nHeight, state);
@@ -2740,9 +2734,6 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
     // Cold Staking enforcement (true during sync - reject P2CS outputs when false)
     bool fColdStakingActive = true;
 
-    // Zerocoin activation
-    bool fZerocoinActive = block.GetBlockTime() > Params().GetConsensus().ZC_TimeStart;
-
     // masternode payments / budgets
     CBlockIndex* pindexPrev = chainActive.Tip();
     int nHeight = 0;
@@ -2779,7 +2770,6 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
     }
 
     // Check transactions
-    bool fSaplingActive = Params().GetConsensus().NetworkUpgradeActive(nHeight, Consensus::UPGRADE_V5_0);
     std::vector<CBigNum> vBlockSerials;
     for (const auto& txIn : block.vtx) {
         const CTransaction& tx = *txIn;
@@ -2793,20 +2783,13 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
             // pass the state returned by the function above
             return false;
         }
-
-        // No need to check for zerocoin anymore after sapling, they are networkely disabled
-        // and checkpoints are preventing the chain for any massive reorganization.
-        if (fSaplingActive && tx.ContainsZerocoins()) {
-            return state.DoS(100, error("%s : v5 upgrade enforced, zerocoin disabled", __func__),
-                             REJECT_INVALID, "bad-blk-with-zc");
-        }
     }
 
     unsigned int nSigOps = 0;
     for (const auto& tx : block.vtx) {
         nSigOps += GetLegacySigOpCount(*tx);
     }
-    unsigned int nMaxBlockSigOps = fZerocoinActive ? MAX_BLOCK_SIGOPS_CURRENT : MAX_BLOCK_SIGOPS_LEGACY;
+    unsigned int nMaxBlockSigOps = block.GetBlockTime() > Params().GetConsensus().ZC_TimeStart ? MAX_BLOCK_SIGOPS_CURRENT : MAX_BLOCK_SIGOPS_LEGACY;
     if (nSigOps > nMaxBlockSigOps)
         return state.DoS(100, error("%s : out-of-bounds SigOpCount", __func__),
             REJECT_INVALID, "bad-blk-sigops", true);