Merge #995: [Consensus] Define SPORK_18

e06f7b6 [Tests] Minor optimizations in zerocoin_valid_public_spend test (random-zebra)
cdc18ea [RPC] fix parameters check for spendzerocoin (missing ispublicspend) (random-zebra)
72bc39a [Tests] Add zerocoin_valid_public_spend to testRunner (random-zebra)
26045d3 [Consensus] Set zc PublicSpend version v3/v4 via SPORK (random-zebra)

Pull request description:

  This follows #936
  It sets the required version for zerocoin PublicSpends via SPORK instead of having it height-based.

  When `SPORK_18_ZEROCOIN_PUBLICSPEND_V4` is not active (default), the required version is v3 (where old version 1 serials cannot be spent. ref: #891 ).

  It also fixes the relative functional test `zerocoin_valid_public_spend.py` and adds it to the test_runner.

ACKs for top commit:
  furszy:
    utACK e06f7b6
  Warrows:
    utACK e06f7b6

Tree-SHA512: 07f18e77a91503f5d48dafad32364cc6292ffd1f9a3f9286a78de72dafc98b07342e6abd96eaabe55d20193f10be383c8d5bbac1b33c45712ade8c522e457b3f

src/chainparams.cpp

@@ -132,13 +132,6 @@ bool CChainParams::HasStakeMinAgeOrDepth(const int contextHeight, const uint32_t
     return (contextHeight - utxoFromBlockHeight >= nStakeMinDepth);
 }
 
-int CChainParams::Zerocoin_PublicSpendVersion(const int nHeight) const
-{
-    if (nHeight < nPublicZCSpendsV4)
-        return 3;
-    return 4;
-}
-
 class CMainParams : public CChainParams
 {
 public:
@@ -194,7 +187,6 @@ class CMainParams : public CChainParams
 
         // Public coin spend enforcement
         nPublicZCSpends = 1880000;
-        nPublicZCSpendsV4 = 2880000;
 
         // New P2P messages signatures
         nBlockEnforceNewMessageSignatures = 2967000;
@@ -335,7 +327,6 @@ class CTestNetParams : public CMainParams
 
         // Public coin spend enforcement
         nPublicZCSpends = 1106100;
-        nPublicZCSpendsV4 = 2106100;
 
         // New P2P messages signatures
         nBlockEnforceNewMessageSignatures = 2214000;
@@ -434,7 +425,6 @@ class CRegTestParams : public CTestNetParams
 
         // Public coin spend enforcement
         nPublicZCSpends = 350;
-        nPublicZCSpendsV4 = 450;
 
         // New P2P messages signatures
         nBlockEnforceNewMessageSignatures = 1;

src/chainparams.h

@@ -144,7 +144,6 @@ class CChainParams
     int Zerocoin_Block_V2_Start() const { return nBlockZerocoinV2; }
     bool IsStakeModifierV2(const int nHeight) const { return nHeight >= nBlockStakeModifierlV2; }
     int NewSigsActive(const int nHeight) const { return nHeight >= nBlockEnforceNewMessageSignatures; }
-    int Zerocoin_PublicSpendVersion(const int nHeight) const;
 
     // fake serial attack
     int Zerocoin_Block_EndFakeSerial() const { return nFakeSerialBlockheightEnd; }
@@ -227,7 +226,6 @@ class CChainParams
     int nBlockZerocoinV2;
     int nBlockDoubleAccumulated;
     int nPublicZCSpends;
-    int nPublicZCSpendsV4;
     int nBlockStakeModifierlV2;
     int nBlockEnforceNewMessageSignatures;
 

src/main.cpp

@@ -1009,8 +1009,12 @@ bool CheckPublicCoinSpendEnforced(int blockHeight, bool isPublicSpend) {
     return true;
 }
 
-bool CheckPublicCoinSpendVersion(int blockHeight, int version) {
-    return version >= Params().Zerocoin_PublicSpendVersion(blockHeight);
+int CurrentPublicCoinSpendVersion() {
+    return sporkManager.IsSporkActive(SPORK_18_ZEROCOIN_PUBLICSPEND_V4) ? 4 : 3;
+}
+
+bool CheckPublicCoinSpendVersion(int version) {
+    return version == CurrentPublicCoinSpendVersion();
 }
 
 bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, CBlockIndex* pindex, const uint256& hashBlock)
@@ -1052,12 +1056,6 @@ bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const lib
         }
     }
 
-    // Check spend version
-    if (pindex->nHeight >= Params().Zerocoin_Block_Public_Spend_Enabled() &&
-            !CheckPublicCoinSpendVersion(pindex->nHeight, (int)spend->getVersion())) {
-        return error("%s: Invalid public spend version for tx %s", __func__, tx.GetHash().GetHex());
-    }
-
     bool fUseV1Params = spend->getCoinVersion() < libzerocoin::PrivateCoin::PUBKEY_VERSION;
 
     //Reject serial's that are not in the acceptable value range
@@ -1453,6 +1451,13 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState& state, const CTransa
                     if (!ContextualCheckZerocoinSpend(tx, &publicSpend, chainActive.Tip(), 0))
                         return state.Invalid(error("%s: ContextualCheckZerocoinSpend failed for tx %s",
                                 __func__, tx.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-zpiv");
+
+                    // Check that the version matches the one enforced with SPORK_18
+                    if (!CheckPublicCoinSpendVersion(publicSpend.getVersion())) {
+                        return state.Invalid(error("%s : Public Zerocoin spend version %d not accepted. must be version %d.",
+                                __func__, publicSpend.getVersion(), CurrentPublicCoinSpendVersion()), REJECT_INVALID, "bad-txns-invalid-zpiv");
+                    }
+
                 } else {
                     libzerocoin::CoinSpend spend = TxInToZerocoinSpend(txIn);
                     if (!ContextualCheckZerocoinSpend(tx, &spend, chainActive.Tip(), 0))
@@ -4571,6 +4576,11 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
                             return false;
                         }
                         spend = publicSpend;
+                        // check that the version matches the one enforced with SPORK_18 (don't ban if it fails)
+                        if (!IsInitialBlockDownload() && !CheckPublicCoinSpendVersion(spend.getVersion())) {
+                            return state.DoS(0, error("%s : Public Zerocoin spend version %d not accepted. must be version %d.",
+                                    __func__, spend.getVersion(), CurrentPublicCoinSpendVersion()), REJECT_INVALID, "bad-zcspend-version");
+                        }
                     } else {
                         spend = TxInToZerocoinSpend(txIn);
                     }

src/main.h

@@ -375,7 +375,8 @@ bool isBlockBetweenFakeSerialAttackRange(int nHeight);
 
 // Public coin spend
 bool CheckPublicCoinSpendEnforced(int blockHeight, bool isPublicSpend);
-bool CheckPublicCoinSpendVersion(int blockHeight, int version);
+int CurrentPublicCoinSpendVersion();
+bool CheckPublicCoinSpendVersion(int version);
 
 /**
  * Check if transaction will be final in the next block to be created.

src/spork.cpp

@@ -25,6 +25,7 @@ std::vector<CSporkDef> sporkDefs = {
     MAKE_SPORK_DEF(SPORK_15_NEW_PROTOCOL_ENFORCEMENT_2,     4070908800ULL), // OFF
     MAKE_SPORK_DEF(SPORK_16_ZEROCOIN_MAINTENANCE_MODE,      4070908800ULL), // OFF
     MAKE_SPORK_DEF(SPORK_17_COLDSTAKING_ENFORCEMENT,        4070908800ULL), // OFF
+    MAKE_SPORK_DEF(SPORK_18_ZEROCOIN_PUBLICSPEND_V4,        4070908800ULL), // OFF
 };
 
 CSporkManager sporkManager;

src/sporkid.h

@@ -23,6 +23,7 @@ enum SporkId : int32_t {
     SPORK_15_NEW_PROTOCOL_ENFORCEMENT_2         = 10014,
     SPORK_16_ZEROCOIN_MAINTENANCE_MODE          = 10015,
     SPORK_17_COLDSTAKING_ENFORCEMENT            = 10017,
+    SPORK_18_ZEROCOIN_PUBLICSPEND_V4            = 10018,
 
     SPORK_INVALID                               = -1
 };

src/wallet/rpcwallet.cpp

@@ -3393,9 +3393,9 @@ UniValue mintzerocoin(const UniValue& params, bool fHelp)
 
 UniValue spendzerocoin(const UniValue& params, bool fHelp)
 {
-    if (fHelp || params.size() > 4 || params.size() < 3)
+    if (fHelp || params.size() > 5 || params.size() < 3)
         throw std::runtime_error(
-            "spendzerocoin amount mintchange minimizechange ( \"address\" )\n"
+            "spendzerocoin amount mintchange minimizechange ( \"address\" ispublicspend)\n"
             "\nSpend zPIV to a PIV address.\n" +
             HelpRequiringPassphrase() + "\n"
 
@@ -3447,7 +3447,7 @@ UniValue spendzerocoin(const UniValue& params, bool fHelp)
         throw JSONRPCError(RPC_WALLET_ERROR, "zPIV minting is DISABLED, cannot mint change");
     bool fMinimizeChange = params[2].get_bool();    // Minimize change
     std::string address_str = params.size() > 3 ? params[3].get_str() : "";
-    bool ispublicspend = params.size() > 4 ? params[3].get_bool() : true;
+    bool ispublicspend = params.size() > 4 ? params[4].get_bool() : true;
 
     std::vector<CZerocoinMint> vMintsSelected;
 
@@ -3573,7 +3573,7 @@ extern UniValue DoZpivSpend(const CAmount nAmount, bool fMintChange, bool fMinim
         outputs.push_back(std::pair<CBitcoinAddress*, CAmount>(&address, nAmount));
     }
 
-    fSuccess = pwalletMain->SpendZerocoin(nAmount, wtx, receipt, vMintsSelected, fMintChange, fMinimizeChange, outputs);
+    fSuccess = pwalletMain->SpendZerocoin(nAmount, wtx, receipt, vMintsSelected, fMintChange, fMinimizeChange, outputs, nullptr, ispublicspend);
 
     if (!fSuccess)
         throw JSONRPCError(RPC_WALLET_ERROR, receipt.GetStatusMessage());

src/wallet/wallet.cpp

@@ -4367,7 +4367,7 @@ bool CWallet::MintsToInputVectorPublicSpend(std::map<CBigNum, CZerocoinMint>& ma
     if (!nHeight)
         return error("%s: Unable to get chain tip height", __func__);
 
-    int spendVersion = Params().Zerocoin_PublicSpendVersion(nHeight);
+    int spendVersion = CurrentPublicCoinSpendVersion();
 
     int nLockAttempts = 0;
     while (nLockAttempts < 100) {

test/functional/test_runner.py

@@ -73,6 +73,7 @@
 
     'p2p_pos_fakestake.py',
     'p2p_pos_fakestake_accepted.py',
+    'zerocoin_valid_public_spend.py',
     #'p2p_zpos_fakestake.py',
     #'p2p_zpos_fakestake_accepted.py',
     #'zerocoin_wrapped_serials.py',