Merge #2425: [Cleanup] The zerocoin garbage collector is in town once…

… more.

017ee3e Validation cleanup: Inside ConnectBlock, move coinSpend parse and validation to zerocoin_verify file. (furszy)
e8206e8 Remove unneeded hashBlock arg from ContextualCheckZerocoinSpend and ContextualCheckZerocoinSpendNoSerialCheck (furszy)
c402eef Remove dead zerocoin.h/cpp (furszy)
0a3f069 Remove dead mintpool.h/cpp. (furszy)

Pull request description:

  Built on top of #2391.

  Made the following changes:
  1) Removed dead `mintpool.h/cpp`.
  2) Removed dead `zerocoin.h/cpp`.
  3) Removed unneeded hashBlock arg from `ContextualCheckZerocoinSpend` and `ContextualCheckZerocoinSpendNoSerialCheck`.
  4) Validation cleanup: Inside ConnectBlock, moved coinSpend parse and validation to `zerocoin_verify` files.

ACKs for top commit:
  random-zebra:
    utACK 017ee3e

Tree-SHA512: 1df0d91906234741e3baee5af828d8cfb77a701597d10f09d88b0a11c4c4899aeeb1ca9c801fd57cfbe64fbd747041d92f2752a218c4b17d4d03dedf75c5855a

CMakeLists.txt

@@ -257,10 +257,8 @@ set(WALLET_SOURCES
         ./src/wallet/db.cpp
         ./src/addressbook.cpp
         ./src/crypter.cpp
-        ./src/zpiv/mintpool.cpp
         ./src/wallet/hdchain.cpp
         ./src/wallet/rpcdump.cpp
-        ./src/zpiv/zerocoin.cpp
         ./src/wallet/fees.cpp
         ./src/wallet/init.cpp
         ./src/wallet/scriptpubkeyman.cpp
@@ -367,7 +365,6 @@ set(COMMON_SOURCES
         ./src/consensus/zerocoin_verify.cpp
         ./src/primitives/block.cpp
         ./src/primitives/transaction.cpp
-        ./src/zpiv/zerocoin.cpp
         ./src/core_read.cpp
         ./src/core_write.cpp
         ./src/hash.cpp

src/Makefile.am

@@ -307,8 +307,6 @@ BITCOIN_CORE_H = \
   wallet/walletdb.h \
   warnings.h \
   zpivchain.h \
-  zpiv/mintpool.h \
-  zpiv/zerocoin.h \
   zpiv/zpos.h \
   zmq/zmqabstractnotifier.h \
   zmq/zmqconfig.h \
@@ -410,8 +408,6 @@ libbitcoin_wallet_a_SOURCES = \
   destination_io.cpp \
   wallet/wallet.cpp \
   wallet/walletdb.cpp \
-  zpiv/zerocoin.cpp \
-  zpiv/mintpool.cpp \
   stakeinput.cpp \
   zpiv/zpos.cpp \
   $(BITCOIN_CORE_H) \
@@ -502,7 +498,6 @@ libbitcoin_common_a_SOURCES = \
   key_io.cpp \
   primitives/block.cpp \
   primitives/transaction.cpp \
-  zpiv/zerocoin.cpp \
   core_read.cpp \
   core_write.cpp \
   hash.cpp \

src/consensus/zerocoin_verify.cpp

@@ -194,9 +194,9 @@ bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& stat
     return true;
 }
 
-bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock)
+bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight)
 {
-    if(!ContextualCheckZerocoinSpendNoSerialCheck(tx, spend, nHeight, hashBlock)){
+    if(!ContextualCheckZerocoinSpendNoSerialCheck(tx, spend, nHeight)){
         return false;
     }
 
@@ -209,7 +209,7 @@ bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::Coi
     return true;
 }
 
-bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock)
+bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight)
 {
     const Consensus::Params& consensus = Params().GetConsensus();
     //Check to see if the zPIV is properly signed
@@ -250,3 +250,46 @@ bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const lib
     return true;
 }
 
+Optional<CoinSpendValues> ParseAndValidateZerocoinSpend(const Consensus::Params& consensus,
+                                                        const CTransaction& tx, int chainHeight,
+                                                        CValidationState& state)
+{
+    for (const CTxIn& txIn : tx.vin) {
+        bool isPublicSpend = txIn.IsZerocoinPublicSpend();
+        bool isPrivZerocoinSpend = txIn.IsZerocoinSpend();
+        if (!isPrivZerocoinSpend && !isPublicSpend)
+            continue;
+
+        // Check enforcement
+        if (!CheckPublicCoinSpendEnforced(chainHeight, isPublicSpend)){
+            return nullopt;
+        }
+
+        if (isPublicSpend) {
+            libzerocoin::ZerocoinParams* params = consensus.Zerocoin_Params(false);
+            PublicCoinSpend publicSpend(params);
+            if (!ZPIVModule::ParseZerocoinPublicSpend(txIn, tx, state, publicSpend) ||
+                !CheckPublicCoinSpendVersion(publicSpend.getCoinVersion())){
+                return nullopt;
+            }
+            //queue for db write after the 'justcheck' section has concluded
+            if (!ContextualCheckZerocoinSpend(tx, &publicSpend, chainHeight)) {
+                state.DoS(100, error("%s: failed to add block %s with invalid public zc spend", __func__,
+                                     tx.GetHash().GetHex()), REJECT_INVALID);
+                return nullopt;
+            }
+            // return value
+            return Optional<CoinSpendValues>(CoinSpendValues(publicSpend.getCoinSerialNumber(), publicSpend.getDenomination() * COIN));
+        } else {
+            libzerocoin::CoinSpend spend = TxInToZerocoinSpend(txIn);
+            //queue for db write after the 'justcheck' section has concluded
+            if (!ContextualCheckZerocoinSpend(tx, &spend, chainHeight)) {
+                state.DoS(100, error("%s: failed to add block %s with invalid zerocoinspend", __func__,
+                                     tx.GetHash().GetHex()), REJECT_INVALID);
+                return nullopt;
+            }
+            return Optional<CoinSpendValues>(CoinSpendValues(spend.getCoinSerialNumber(), spend.getDenomination() * COIN));
+        }
+    }
+    return nullopt;
+}

src/consensus/zerocoin_verify.h

@@ -16,7 +16,18 @@ bool CheckPublicCoinSpendEnforced(int blockHeight, bool isPublicSpend);
 int CurrentPublicCoinSpendVersion();
 bool CheckPublicCoinSpendVersion(int version);
 bool ContextualCheckZerocoinTx(const CTransactionRef& tx, CValidationState& state, const Consensus::Params& consensus, int nHeight);
-bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock);
-bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight, const uint256& hashBlock);
+bool ContextualCheckZerocoinSpend(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight);
+bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const libzerocoin::CoinSpend* spend, int nHeight);
+
+struct CoinSpendValues {
+public:
+    explicit CoinSpendValues(const CBigNum& s, CAmount v) : serial(s), value(v) {}
+    CBigNum serial;
+    CAmount value;
+};
+// Returns nullopt if coin spend is invalid. Invalidity/DoS causes are treated inside the function.
+Optional<CoinSpendValues> ParseAndValidateZerocoinSpend(const Consensus::Params& consensus,
+                                                               const CTransaction& tx, int chainHeight,
+                                                               CValidationState& state);
 
 #endif //PIVX_CONSENSUS_ZEROCOIN_VERIFY_H

src/txdb.cpp

@@ -10,7 +10,6 @@
 #include "pow.h"
 #include "uint256.h"
 #include "util/system.h"
-#include "zpiv/zerocoin.h"
 #include "util/vector.h"
 
 #include <stdint.h>
@@ -345,12 +344,12 @@ CZerocoinDB::CZerocoinDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapp
 {
 }
 
-bool CZerocoinDB::WriteCoinSpendBatch(const std::vector<std::pair<libzerocoin::CoinSpend, uint256> >& spendInfo)
+bool CZerocoinDB::WriteCoinSpendBatch(const std::vector<std::pair<CBigNum, uint256> >& spendInfo)
 {
     CDBBatch batch;
     size_t count = 0;
-    for (std::vector<std::pair<libzerocoin::CoinSpend, uint256> >::const_iterator it=spendInfo.begin(); it != spendInfo.end(); it++) {
-        CBigNum bnSerial = it->first.getCoinSerialNumber();
+    for (std::vector<std::pair<CBigNum, uint256> >::const_iterator it=spendInfo.begin(); it != spendInfo.end(); it++) {
+        CBigNum bnSerial = it->first;
         CDataStream ss(SER_GETHASH, 0);
         ss << bnSerial;
         uint256 hash = Hash(ss.begin(), ss.end());

src/txdb.h

@@ -160,8 +160,10 @@ class CZerocoinDB : public CDBWrapper
     void operator=(const CZerocoinDB&);
 
 public:
-    /** Write zPIV spends to the zerocoinDB in a batch */
-    bool WriteCoinSpendBatch(const std::vector<std::pair<libzerocoin::CoinSpend, uint256> >& spendInfo);
+    /** Write zPIV spends to the zerocoinDB in a batch
+     * Pair of: CBigNum -> coinSerialNumber and uint256 -> txHash.
+     */
+    bool WriteCoinSpendBatch(const std::vector<std::pair<CBigNum, uint256> >& spendInfo);
     bool ReadCoinSpend(const CBigNum& bnSerial, uint256& txHash);
     bool EraseCoinSpend(const CBigNum& bnSerial);
 

src/validation.cpp

@@ -50,7 +50,6 @@
 #include "validationinterface.h"
 #include "warnings.h"
 #include "zpivchain.h"
-#include "zpiv/zerocoin.h"
 #include "zpiv/zpivmodule.h"
 
 #include <future>
@@ -1548,14 +1547,13 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
     unsigned int nSigOps = 0;
     CDiskTxPos pos(pindex->GetBlockPos(), GetSizeOfCompactSize(block.vtx.size()));
     std::vector<std::pair<uint256, CDiskTxPos> > vPos;
-    std::vector<std::pair<libzerocoin::CoinSpend, uint256> > vSpends;
+    std::vector<std::pair<CBigNum, uint256> > vSpends;
     vPos.reserve(block.vtx.size());
     CBlockUndo blockundo;
     blockundo.vtxundo.reserve(block.vtx.size() - 1);
     CAmount nValueOut = 0;
     CAmount nValueIn = 0;
     unsigned int nMaxBlockSigOps = MAX_BLOCK_SIGOPS_CURRENT;
-    std::vector<uint256> vSpendsInBlock;
     uint256 hashBlock = block.GetHash();
 
     // Sapling
@@ -1589,43 +1587,12 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
         }
 
         if (tx.HasZerocoinSpendInputs()) {
-            const uint256& txid = tx.GetHash();
-            vSpendsInBlock.emplace_back(txid);
-
-            //Check for double spending of serial #'s
-            std::set<CBigNum> setSerials;
-            for (const CTxIn& txIn : tx.vin) {
-                bool isPublicSpend = txIn.IsZerocoinPublicSpend();
-                bool isPrivZerocoinSpend = txIn.IsZerocoinSpend();
-                if (!isPrivZerocoinSpend && !isPublicSpend)
-                    continue;
-
-                // Check enforcement
-                if (!CheckPublicCoinSpendEnforced(pindex->nHeight, isPublicSpend)){
-                    return false;
-                }
-
-                if (isPublicSpend) {
-                    libzerocoin::ZerocoinParams* params = consensus.Zerocoin_Params(false);
-                    PublicCoinSpend publicSpend(params);
-                    if (!ZPIVModule::ParseZerocoinPublicSpend(txIn, tx, state, publicSpend)){
-                        return false;
-                    }
-                    nValueIn += publicSpend.getDenomination() * COIN;
-                    //queue for db write after the 'justcheck' section has concluded
-                    vSpends.emplace_back(publicSpend, tx.GetHash());
-                    if (!ContextualCheckZerocoinSpend(tx, &publicSpend, pindex->nHeight, hashBlock))
-                        return state.DoS(100, error("%s: failed to add block %s with invalid public zc spend", __func__, tx.GetHash().GetHex()), REJECT_INVALID);
-                } else {
-                    libzerocoin::CoinSpend spend = TxInToZerocoinSpend(txIn);
-                    nValueIn += spend.getDenomination() * COIN;
-                    //queue for db write after the 'justcheck' section has concluded
-                    vSpends.emplace_back(spend, tx.GetHash());
-                    if (!ContextualCheckZerocoinSpend(tx, &spend, pindex->nHeight, hashBlock))
-                        return state.DoS(100, error("%s: failed to add block %s with invalid zerocoinspend", __func__, tx.GetHash().GetHex()), REJECT_INVALID);
-                }
+            auto opCoinSpendValues = ParseAndValidateZerocoinSpend(consensus, tx, pindex->nHeight, state);
+            if (!opCoinSpendValues) {
+                return false; // Invalidity/DoS is handled by ParseAndValidateZerocoinSpend.
             }
-
+            nValueIn += opCoinSpendValues->value;
+            vSpends.emplace_back(opCoinSpendValues->serial, tx.GetHash());
         } else if (!tx.IsCoinBase()) {
             if (!view.HaveInputs(tx)) {
                 return state.DoS(100, false, REJECT_INVALID, "bad-txns-inputs-missingorspent");
@@ -3310,7 +3277,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockInde
                             return state.DoS(100, error("%s: serial double spent on main chain", __func__));
                     }
 
-                    if (!ContextualCheckZerocoinSpendNoSerialCheck(stakeTxIn, &spend, pindex->nHeight, UINT256_ZERO))
+                    if (!ContextualCheckZerocoinSpendNoSerialCheck(stakeTxIn, &spend, pindex->nHeight))
                         return state.DoS(100,error("%s: forked chain ContextualCheckZerocoinSpend failed for tx %s", __func__,
                                                    stakeTxIn.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-zpiv");
 
@@ -3334,7 +3301,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, CBlockInde
             if(!isBlockFromFork)
                 for (const CTxIn& zPivInput : zPIVInputs) {
                         libzerocoin::CoinSpend spend = TxInToZerocoinSpend(zPivInput);
-                        if (!ContextualCheckZerocoinSpend(stakeTxIn, &spend, pindex->nHeight, UINT256_ZERO))
+                        if (!ContextualCheckZerocoinSpend(stakeTxIn, &spend, pindex->nHeight))
                             return state.DoS(100,error("%s: main chain ContextualCheckZerocoinSpend failed for tx %s", __func__,
                                     stakeTxIn.GetHash().GetHex()), REJECT_INVALID, "bad-txns-invalid-zpiv");
                 }