Merge PIVX-Project#1878: [Core][RPC][BUG] Use sum of utxo values as money supply

dcc032c [Cleanup] Remove ZC_WrappedSerialsSupply no longer needed (random-zebra)
0e7921e [DB] Also prune invalid/fraudulent/frozen outputs from the coins view (random-zebra)
b8647bd [Tests] Fix expected PIV supply in mining_pos_reorg (random-zebra)
40b7ea3 [DB] Prune zerocoin mints from coins DB at startup (random-zebra)
b89bb91 [BUG] Don't try to spend zc mint coins from the cache in DisconnectBlock (random-zebra)
cfc06ed [Core] Don't add zerocoin mints to the coins cache (random-zebra)
e8a782c [Cleanup] Remove unused function GetInvalidUTXOValue (random-zebra)
db7d8fd [Core] Remove nMoneySupply global and RecalculatePIVSupply function (random-zebra)
b674fde [DB] Remove money supply databasing (random-zebra)
373ee90 [Cleanup] Remove --reindexmoneysupply startup flag (random-zebra)
02c9aaa [RPC] Return total available utxo value as moneysupply (random-zebra)
cdf9bc1 [Core] Implement CCoinsViewCache::GetTotalAmount (random-zebra)

Pull request description:

  Instead of updating and databasing independently `nMoneySupply`, just use the sum of all unspent tx outputs, returned by the coins cache.

  This has the following advantages:
    - Cleaner code: we don't need to add special cases for exceptional one-time events, such as wrapped serials, accumulator recalculation, etc.
    - More robust: with the old code, `nMoneySupply` is databased at every block and does not  follow the chainstate flushing strategy, so it's very easy to get an inconsistent state (see PIVX-Project#1873).
  Even more so, considering the absence of any consistency check between the actual chain height and the height at which the supply was updated.
    - More accurate number: this returns only the __spendable__ supply, so it excludes all unspendable outputs (OP_RETURN), such as proposals / finalized budgets collaterals, that were originally counted in.
  Those outputs, in fact, are effectively "burnt" coins and should not be part of the total supply calculation.

  Disadvantage:
    - Calculating the supply from the coins DB is a bit slow, but the only place where this is done is in the RPC `getinfo`.

  This PR removes the global variable `nMoneySupply` and all related code (Closes PIVX-Project#1873) and gets rid of the startup flag `--reindexmoneysupply` no longer needed (PIVX-Project#1864 introduces `reindex-chainstate`).

  ⚠️ This also fixes the following bug with zerocoin txes:
  Currently, on master, `gettxoutsetinfo` reports a total amount of about **811 million PIV**!
  This is due to improper handling of zerocoin-related "utxos".
  Basically mints are added to the coins cache (`AddCoin` only skips OP_RETURN coins, not mints) and *never* pruned/spent (as they are skipped inside `UpdateCoins`), except in `DisconnectBlock`.

  Here we completely remove zerocoin mint outputs from the coins DB.
  This makes sense, as originally they were not meant to be linked when spending, so should have been considered unspendable outputs, and not included in the first place. With the introduction of Public Spends, mint outputs are now referenced when spending, but we don't need to keep them in the cache either, as double spending is checked only via the zerocoin serial (and, as said, the coins aren't even spent in `UpdateCoins`).
  We also remove all those utxos flagged as invalid.

  Removing zerocoin mints from the unspent coins DB also gives a nice reduction to the utxoset size on disk (about 37%, from 428 Mb to 270 Mb).

ACKs for top commit:
  Fuzzbawls:
    ACK dcc032c

Tree-SHA512: da50dd81b9e3a56291812cf35158e1858f5a0c6ebdf1e5018d1e1d20b834dbdccdaa4cb0d82830c6f0a405c71428355615fd57f582065b992d42fa9a7313235e

Author: random-zebra

src/chainparams.cpp

@@ -183,7 +183,6 @@ class CMainParams : public CChainParams
         consensus.ZC_MinMintFee = 1 * CENT;
         consensus.ZC_MinStakeDepth = 200;
         consensus.ZC_TimeStart = 1508214600;        // October 17, 2017 4:30:00 AM
-        consensus.ZC_WrappedSerialsSupply = 4131563 * COIN;   // zerocoin supply at height_last_ZC_WrappedSerials
 
         // Network upgrades
         consensus.vUpgrades[Consensus::BASE_NETWORK].nActivationHeight =
@@ -323,7 +322,6 @@ class CTestNetParams : public CMainParams
         consensus.ZC_MinMintFee = 1 * CENT;
         consensus.ZC_MinStakeDepth = 200;
         consensus.ZC_TimeStart = 1501776000;
-        consensus.ZC_WrappedSerialsSupply = 0;   // WrappedSerials only on main net
 
         // Network upgrades
         consensus.vUpgrades[Consensus::BASE_NETWORK].nActivationHeight =
@@ -465,7 +463,6 @@ class CRegTestParams : public CTestNetParams
         consensus.ZC_MinMintFee = 1 * CENT;
         consensus.ZC_MinStakeDepth = 10;
         consensus.ZC_TimeStart = 0;                 // not implemented on regtest
-        consensus.ZC_WrappedSerialsSupply = 0;
 
         // Network upgrades
         consensus.vUpgrades[Consensus::BASE_NETWORK].nActivationHeight =

src/coins.cpp

@@ -6,6 +6,7 @@
 #include "coins.h"
 
 #include "consensus/consensus.h"
+#include "invalid.h"
 #include "memusage.h"
 #include "random.h"
 
@@ -65,6 +66,7 @@ bool CCoinsViewCache::GetCoin(const COutPoint& outpoint, Coin& coin) const
 void CCoinsViewCache::AddCoin(const COutPoint& outpoint, Coin&& coin, bool possible_overwrite) {
     assert(!coin.IsSpent());
     if (coin.out.scriptPubKey.IsUnspendable()) return;
+    if (coin.out.IsZerocoinMint()) return;
     CCoinsMap::iterator it;
     bool inserted;
     std::tie(it, inserted) = cacheCoins.emplace(std::piecewise_construct, std::forward_as_tuple(outpoint), std::tuple<>());
@@ -276,6 +278,37 @@ int CCoinsViewCache::GetCoinDepthAtHeight(const COutPoint& output, int nHeight)
     return -1;
 }
 
+CAmount CCoinsViewCache::GetTotalAmount() const
+{
+    CAmount nTotal = 0;
+
+    std::unique_ptr<CCoinsViewCursor> pcursor(Cursor());
+    while (pcursor->Valid()) {
+        Coin coin;
+        if (pcursor->GetValue(coin) && !coin.IsSpent()) {
+            nTotal += coin.out.nValue;
+        }
+        pcursor->Next();
+    }
+
+    return nTotal;
+}
+
+void CCoinsViewCache::PruneInvalidEntries()
+{
+    // Prune zerocoin Mints and fraudulent/frozen outputs
+    std::unique_ptr<CCoinsViewCursor> pcursor(Cursor());
+    while (pcursor->Valid()) {
+        COutPoint key;
+        Coin coin;
+        if (pcursor->GetKey(key) && pcursor->GetValue(coin)) {
+            if (coin.out.IsZerocoinMint() || invalid_out::ContainsOutPoint(key))
+                SpendCoin(key);
+        }
+        pcursor->Next();
+    }
+}
+
 static const size_t MAX_OUTPUTS_PER_BLOCK = MAX_BLOCK_SIZE_CURRENT /  ::GetSerializeSize(CTxOut(), SER_NETWORK, PROTOCOL_VERSION); // TODO: merge with similar definition in undo.h.
 
 const Coin& AccessByTxid(const CCoinsViewCache& view, const uint256& txid)
@@ -288,3 +321,4 @@ const Coin& AccessByTxid(const CCoinsViewCache& view, const uint256& txid)
     }
     return coinEmpty;
 }
+

src/coins.h

@@ -290,6 +290,16 @@ class CCoinsViewCache : public CCoinsViewBacked
      */
     int GetCoinDepthAtHeight(const COutPoint& output, int nHeight) const;
 
+    /*
+     * Return the sum of the value of all transaction outputs
+     */
+    CAmount GetTotalAmount() const;
+
+    /*
+     * Prune zerocoin mints and frozen outputs - do it once, after initialization
+     */
+    void PruneInvalidEntries();
+
 
 private:
     CCoinsMap::iterator FetchCoin(const COutPoint& outpoint) const;

src/consensus/params.h

@@ -169,7 +169,6 @@ struct Params {
     CAmount ZC_MinMintFee;
     int ZC_MinStakeDepth;
     int ZC_TimeStart;
-    CAmount ZC_WrappedSerialsSupply;
 
     libzerocoin::ZerocoinParams* Zerocoin_Params(bool useModulusV1) const
     {

src/consensus/zerocoin_verify.cpp

@@ -191,111 +191,3 @@ bool ContextualCheckZerocoinSpendNoSerialCheck(const CTransaction& tx, const lib
     return true;
 }
 
-bool RecalculatePIVSupply(int nHeightStart, bool fSkipZpiv)
-{
-    AssertLockHeld(cs_main);
-
-    const Consensus::Params& consensus = Params().GetConsensus();
-    const int chainHeight = chainActive.Height();
-    if (nHeightStart > chainHeight)
-        return false;
-
-    CBlockIndex* pindex = chainActive[nHeightStart];
-    if (IsActivationHeight(nHeightStart, consensus, Consensus::UPGRADE_ZC))
-        nMoneySupply = CAmount(5449796547496199);
-
-    if (!fSkipZpiv) {
-        // initialize supply to 0
-        mapZerocoinSupply.clear();
-        for (auto& denom : libzerocoin::zerocoinDenomList) mapZerocoinSupply.emplace(denom, 0);
-    }
-
-    uiInterface.ShowProgress(_("Recalculating PIV supply..."), 0);
-    while (true) {
-        if (pindex->nHeight % 1000 == 0) {
-            LogPrintf("%s : block %d...\n", __func__, pindex->nHeight);
-            int percent = std::max(1, std::min(99, (int)((double)((pindex->nHeight - nHeightStart) * 100) / (chainHeight - nHeightStart))));
-            uiInterface.ShowProgress(_("Recalculating PIV supply..."), percent);
-        }
-
-        CBlock block;
-        assert(ReadBlockFromDisk(block, pindex));
-
-        CAmount nValueIn = 0;
-        CAmount nValueOut = 0;
-        for (const auto& txIn : block.vtx) {
-            const CTransaction& tx = *txIn;
-            for (unsigned int i = 0; i < tx.vin.size(); i++) {
-                if (tx.IsCoinBase())
-                    break;
-
-                if (tx.vin[i].IsZerocoinSpend()) {
-                    nValueIn += tx.vin[i].nSequence * COIN;
-                    continue;
-                }
-
-                COutPoint prevout = tx.vin[i].prevout;
-                CTransaction txPrev;
-                uint256 hashBlock;
-                assert(GetTransaction(prevout.hash, txPrev, hashBlock, true));
-                nValueIn += txPrev.vout[prevout.n].nValue;
-            }
-
-            for (unsigned int i = 0; i < tx.vout.size(); i++) {
-                if (i == 0 && tx.IsCoinStake())
-                    continue;
-
-                nValueOut += tx.vout[i].nValue;
-            }
-        }
-
-        // Rewrite money supply
-        nMoneySupply += (nValueOut - nValueIn);
-
-        // Rewrite zpiv supply too
-        if (!fSkipZpiv && consensus.NetworkUpgradeActive(pindex->nHeight, Consensus::UPGRADE_ZC)) {
-            UpdateZPIVSupplyConnect(block, pindex, true);
-        }
-
-        // Add fraudulent funds to the supply and remove any recovered funds.
-        if (pindex->nHeight == consensus.height_ZC_RecalcAccumulators) {
-            const CAmount nInvalidAmountFiltered = 268200*COIN;    //Amount of invalid coins filtered through exchanges, that should be considered valid
-            LogPrintf("%s : Original money supply=%s\n", __func__, FormatMoney(nMoneySupply));
-
-            nMoneySupply += nInvalidAmountFiltered;
-            LogPrintf("%s : Adding filtered funds to supply + %s : supply=%s\n", __func__, FormatMoney(nInvalidAmountFiltered), FormatMoney(nMoneySupply));
-
-            CAmount nLocked = GetInvalidUTXOValue();
-            nMoneySupply -= nLocked;
-            LogPrintf("%s : Removing locked from supply - %s : supply=%s\n", __func__, FormatMoney(nLocked), FormatMoney(nMoneySupply));
-        }
-
-        assert(pblocktree->WriteBlockIndex(CDiskBlockIndex(pindex)));
-
-        // Stop if shutdown was requested
-        if (ShutdownRequested()) return false;
-
-        if (pindex->nHeight < chainHeight)
-            pindex = chainActive.Next(pindex);
-        else
-            break;
-    }
-    uiInterface.ShowProgress("", 100);
-    return true;
-}
-
-CAmount GetInvalidUTXOValue()
-{
-    CAmount nValue = 0;
-    for (auto out : invalid_out::setInvalidOutPoints) {
-        bool fSpent = false;
-        CCoinsViewCache cache(pcoinsTip);
-        const Coin& coin = cache.AccessCoin(out);
-        if(coin.IsSpent())
-            fSpent = true;
-        if (!fSpent)
-            nValue += coin.out.nValue;
-    }
-
-    return nValue;
-}

src/consensus/zerocoin_verify.h

@@ -19,7 +19,5 @@ int CurrentPublicCoinSpendVersion();
 bool CheckPublicCoinSpendVersion(int version);
 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 RecalculatePIVSupply(int nHeightStart, bool fSkipZpiv = true);
-CAmount GetInvalidUTXOValue();
 
 #endif //PIVX_CONSENSUS_ZEROCOIN_VERIFY_H

src/init.cpp

@@ -418,7 +418,6 @@ std::string HelpMessage(HelpMessageMode mode)
     strUsage += HelpMessageOpt("-pid=<file>", strprintf(_("Specify pid file (default: %s)"), PIVX_PID_FILENAME));
 #endif
     strUsage += HelpMessageOpt("-reindex", _("Rebuild block chain index from current blk000??.dat files") + " " + _("on startup"));
-    strUsage += HelpMessageOpt("-reindexmoneysupply", strprintf(_("Reindex the %s and z%s money supply statistics"), CURRENCY_UNIT, CURRENCY_UNIT) + " " + _("on startup"));
     strUsage += HelpMessageOpt("-resync", _("Delete blockchain folders and resync from scratch") + " " + _("on startup"));
 #if !defined(WIN32)
     strUsage += HelpMessageOpt("-sysperms", _("Create new files with system default permissions, instead of umask 077 (only effective with disabled wallet functionality)"));
@@ -1554,26 +1553,34 @@ bool AppInit2()
                 invalid_out::LoadSerials();
 
                 bool fReindexZerocoin = GetBoolArg("-reindexzerocoin", false);
-                bool fReindexMoneySupply = GetBoolArg("-reindexmoneysupply", false);
 
                 int chainHeight;
+                bool fZerocoinActive;
                 {
                     LOCK(cs_main);
                     chainHeight = chainActive.Height();
+                    fZerocoinActive = consensus.NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_ZC);
+
+                    // Prune zerocoin mints that were improperly stored in the coins database
+                    // Do it only once, when removing money supply (key 'M') from the DB. Can be skipped in future versions.
+                    int64_t nDummySupply;
+                    if (fZerocoinActive && pblocktree->Read('M', nDummySupply)) {
+                        LogPrintf("Pruning zerocoin mints / invalid outs, at height %d\n", chainHeight);
+                        pcoinsTip->PruneInvalidEntries();
+                        pblocktree->Erase('M');
+                    }
 
-                    // initialize PIV and zPIV supply to 0
+                    // initialize zPIV supply to 0
                     mapZerocoinSupply.clear();
                     for (auto& denom : libzerocoin::zerocoinDenomList) mapZerocoinSupply.emplace(denom, 0);
-                    nMoneySupply = 0;
 
-                    // Load PIV and zPIV supply from DB
+                    // Load zPIV supply from DB
                     if (chainHeight >= 0) {
                         const uint256& tipHash = chainActive[chainHeight]->GetBlockHash();
                         CLegacyBlockIndex bi;
 
                         // Load zPIV supply map
-                        if (!fReindexZerocoin && consensus.NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_ZC) &&
-                                !zerocoinDB->ReadZCSupply(mapZerocoinSupply)) {
+                        if (!fReindexZerocoin && fZerocoinActive && !zerocoinDB->ReadZCSupply(mapZerocoinSupply)) {
                             // try first reading legacy block index from DB
                             if (pblocktree->ReadLegacyBlockIndex(tipHash, bi) && !bi.mapZerocoinSupply.empty()) {
                                 mapZerocoinSupply = bi.mapZerocoinSupply;
@@ -1582,22 +1589,11 @@ bool AppInit2()
                                 fReindexZerocoin = true;
                             }
                         }
-
-                        // Load PIV supply amount
-                        if (!fReindexMoneySupply && !pblocktree->ReadMoneySupply(nMoneySupply)) {
-                            // try first reading legacy block index from DB
-                            if (pblocktree->ReadLegacyBlockIndex(tipHash, bi)) {
-                                nMoneySupply = bi.nMoneySupply;
-                            } else {
-                                // reindex from disk
-                                fReindexMoneySupply = true;
-                            }
-                        }
                     }
                 }
 
                 // Drop all information from the zerocoinDB and repopulate
-                if (fReindexZerocoin && consensus.NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_ZC)) {
+                if (fReindexZerocoin && fZerocoinActive) {
                     LOCK(cs_main);
                     uiInterface.InitMessage(_("Reindexing zerocoin database..."));
                     std::string strError = ReindexZerocoinDB();
@@ -1607,13 +1603,6 @@ bool AppInit2()
                     }
                 }
 
-                // Recalculate money supply
-                if (fReindexMoneySupply) {
-                    LOCK(cs_main);
-                    // Skip zpiv if already reindexed
-                    RecalculatePIVSupply(1, fReindexZerocoin);
-                }
-
                 if (!fReindex) {
                     uiInterface.InitMessage(_("Verifying blocks..."));
 

src/miner.cpp

@@ -271,47 +271,45 @@ CBlockTemplate* CreateNewBlock(const CScript& scriptPubKeyIn, CWallet* pwallet,
             double dPriority = 0;
             CAmount nTotalIn = 0;
             bool fMissingInputs = false;
-            bool hasZerocoinSpends = tx.HasZerocoinSpendInputs();
-            if (hasZerocoinSpends)
+
+            if (tx.HasZerocoinSpendInputs()) {
                 nTotalIn = tx.GetZerocoinSpent();
+            } else {
+                for (const CTxIn& txin : tx.vin) {
+                    // Read prev transaction
+                    if (!view.HaveCoin(txin.prevout)) {
+                        // This should never happen; all transactions in the memory
+                        // pool should connect to either transactions in the chain
+                        // or other transactions in the memory pool.
+                        if (!mempool.mapTx.count(txin.prevout.hash)) {
+                            LogPrintf("ERROR: mempool transaction missing input\n");
+                            fMissingInputs = true;
+                            if (porphan)
+                                vOrphan.pop_back();
+                            break;
+                        }
 
-            for (const CTxIn& txin : tx.vin) {
-                // Read prev transaction
-                if (!view.HaveCoin(txin.prevout)) {
-                    // This should never happen; all transactions in the memory
-                    // pool should connect to either transactions in the chain
-                    // or other transactions in the memory pool.
-                    if (!mempool.mapTx.count(txin.prevout.hash)) {
-                        LogPrintf("ERROR: mempool transaction missing input\n");
-                        fMissingInputs = true;
-                        if (porphan)
-                            vOrphan.pop_back();
-                        break;
+                        // Has to wait for dependencies
+                        if (!porphan) {
+                            // Use list for automatic deletion
+                            vOrphan.push_back(COrphan(&tx));
+                            porphan = &vOrphan.back();
+                        }
+                        mapDependers[txin.prevout.hash].push_back(porphan);
+                        porphan->setDependsOn.insert(txin.prevout.hash);
+                        nTotalIn += mempool.mapTx.find(txin.prevout.hash)->GetTx().vout[txin.prevout.n].nValue;
+                        continue;
                     }
 
-                    // Has to wait for dependencies
-                    if (!porphan) {
-                        // Use list for automatic deletion
-                        vOrphan.push_back(COrphan(&tx));
-                        porphan = &vOrphan.back();
-                    }
-                    mapDependers[txin.prevout.hash].push_back(porphan);
-                    porphan->setDependsOn.insert(txin.prevout.hash);
-                    nTotalIn += mempool.mapTx.find(txin.prevout.hash)->GetTx().vout[txin.prevout.n].nValue;
-                    continue;
-                }
+                    const Coin& coin = view.AccessCoin(txin.prevout);
+                    assert(!coin.IsSpent());
 
-                const Coin& coin = view.AccessCoin(txin.prevout);
-                assert(hasZerocoinSpends || !coin.IsSpent());
-
-                CAmount nValueIn = coin.out.nValue;
-                nTotalIn += nValueIn;
-
-                int nConf = nHeight - coin.nHeight;
-
-                // zPIV spends can have very large priority, use non-overflowing safe functions
-                dPriority = double_safe_addition(dPriority, ((double)nValueIn * nConf));
+                    CAmount nValueIn = coin.out.nValue;
+                    nTotalIn += nValueIn;
 
+                    int nConf = nHeight - coin.nHeight;
+                    dPriority = double_safe_addition(dPriority, ((double)nValueIn * nConf));
+                }
             }
             if (fMissingInputs) continue;