[BUG] Remove double counted nValueIn for zerocoin txes

Zerocoin spends values are already returned by GetValueIn below

Author: random-zebra

src/coins.cpp

@@ -338,13 +338,14 @@ CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const
     if (tx.IsCoinBase())
         return 0;
 
-    //todo are there any security precautions to take here?
-    if (tx.HasZerocoinSpendInputs())
-        return tx.GetZerocoinSpent();
-
     CAmount nResult = 0;
-    for (unsigned int i = 0; i < tx.vin.size(); i++)
-        nResult += AccessCoin(tx.vin[i].prevout).out.nValue;
+    for (const CTxIn& in : tx.vin) {
+        if (in.IsZerocoinSpend() || in.IsZerocoinPublicSpend()) {
+            nResult += in.nSequence * COIN;
+        } else {
+            nResult += AccessCoin(in.prevout).out.nValue;
+        }
+    }
 
     // Sapling
     nResult += tx.GetShieldedValueIn();

src/primitives/transaction.cpp

@@ -223,19 +223,6 @@ CAmount CTransaction::GetShieldedValueIn() const
     return nValue;
 }
 
-CAmount CTransaction::GetZerocoinSpent() const
-{
-    CAmount nValueOut = 0;
-    for (const CTxIn& txin : vin) {
-        if(!txin.IsZerocoinSpend())
-            continue;
-
-        nValueOut += txin.nSequence * COIN;
-    }
-
-    return nValueOut;
-}
-
 unsigned int CTransaction::GetTotalSize() const
 {
     return ::GetSerializeSize(*this, PROTOCOL_VERSION);

src/primitives/transaction.h

@@ -368,8 +368,6 @@ class CTransaction
         return HasZerocoinSpendInputs() || HasZerocoinMintOutputs();
     }
 
-    CAmount GetZerocoinSpent() const;
-
     bool IsCoinBase() const
     {
         return (vin.size() == 1 && vin[0].prevout.IsNull() && !vin[0].scriptSig.IsZerocoinSpend());

src/validation.cpp

@@ -1568,7 +1568,6 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
                 return false; // Invalidity/DoS is handled by ParseAndValidateZerocoinSpends.
             }
             for (const CoinSpendValue& s : *opCoinSpendValues) {
-                nValueIn += s.value;
                 vSpends.emplace_back(s.serial, tx.GetHash());
             }
         } else if (!tx.IsCoinBase()) {
@@ -1592,10 +1591,12 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
         // Cache the sig ser hashes
         precomTxData.emplace_back(tx);
 
+        CAmount txValueOut = tx.GetValueOut();
         if (!tx.IsCoinBase()) {
+            CAmount txValueIn = view.GetValueIn(tx);
             if (!tx.IsCoinStake())
-                nFees += view.GetValueIn(tx) - tx.GetValueOut();
-            nValueIn += view.GetValueIn(tx);
+                nFees += txValueIn - txValueOut;
+            nValueIn += txValueIn;
 
             std::vector<CScriptCheck> vChecks;
             unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DERSIG;
@@ -1607,7 +1608,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
                 return error("%s: Check inputs on %s failed with %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
             control.Add(vChecks);
         }
-        nValueOut += tx.GetValueOut();
+        nValueOut += txValueOut;
 
         CTxUndo undoDummy;
         if (i > 0) {
@@ -1642,6 +1643,7 @@ static bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockInd
     }
 
     // track mint amount info
+    assert(nFees >= 0);
     const int64_t nMint = (nValueOut - nValueIn) + nFees;
 
     int64_t nTime1 = GetTimeMicros();