Refactor ConnectBlock() to segregate state tracking items from the 'Just Check' section.

- Remove a `WriteBlockIndex` that was in the middle of `ConnectBlock()`. This really shouldn't be in this code at all because the index is written by adding it to a list of dirty. This is very likely what has caused PIVX to especially likely to corrupt on force close.

- Remove the auto-repair db code on init. This seems to actually cause more problems than it fixes.

- Do not write serials as spent until after the 'Just Check' section. Hold them in a vector until the just check section has passed.

- Do not write the accumulator checkpoints until the 'Just Check' section has passed.

- Refactor aspects of the zerocoin checks to be within their own functions external to `ConnectBlock()`. Functionally the same, but easier to read.

Author: presstab

src/accumulators.cpp

@@ -152,7 +152,7 @@ bool EraseCheckpoints(int nStartHeight, int nEndHeight)
 }
 
 //Get checkpoint value for a specific block height
-bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint)
+bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint, AccumulatorMap& mapAccumulators)
 {
     if (nHeight < Params().Zerocoin_StartHeight()) {
         nCheckpoint = 0;
@@ -166,7 +166,7 @@ bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint)
     }
 
     //set the accumulators to last checkpoint value
-    AccumulatorMap mapAccumulators;
+    mapAccumulators.Reset();
     if (!mapAccumulators.Load(chainActive[nHeight - 1]->nAccumulatorCheckpoint)) {
         if (chainActive[nHeight - 1]->nAccumulatorCheckpoint == 0) {
             //Before zerocoin is fully activated so set to init state
@@ -216,14 +216,12 @@ bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint)
         //grab mints from this block
         CBlock block;
         if(!ReadBlockFromDisk(block, pindex)) {
-            LogPrint("zero","%s: failed to read block from disk\n", __func__);
-            return false;
+            return error("%s: failed to read block from disk\n", __func__);
         }
 
         std::list<PublicCoin> listPubcoins;
         if (!BlockToPubcoinList(block, listPubcoins, fFilterInvalid)) {
-            LogPrint("zero","%s: failed to get zerocoin mintlist from block %n\n", __func__, pindex->nHeight);
-            return false;
+            return error("%s: failed to get zerocoin mintlist from block %d\n", __func__, pindex->nHeight);
         }
 
         nTotalMintsFound += listPubcoins.size();
@@ -232,23 +230,18 @@ bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint)
         //add the pubcoins to accumulator
         for (const PublicCoin pubcoin : listPubcoins) {
             if(!mapAccumulators.Accumulate(pubcoin, true)) {
-                LogPrintf("%s: failed to add pubcoin to accumulator at height %n\n", __func__, pindex->nHeight);
-                return false;
+                return error("%s: failed to add pubcoin to accumulator at height %n\n", __func__, pindex->nHeight);
             }
         }
         pindex = chainActive.Next(pindex);
     }
 
     // if there were no new mints found, the accumulator checkpoint will be the same as the last checkpoint
-    if (nTotalMintsFound == 0) {
+    if (nTotalMintsFound == 0)
         nCheckpoint = chainActive[nHeight - 1]->nAccumulatorCheckpoint;
-    }
     else
         nCheckpoint = mapAccumulators.GetCheckpoint();
 
-    // make sure that these values are databased because reorgs may have deleted the checksums from DB
-    DatabaseChecksums(mapAccumulators);
-
     LogPrint("zero", "%s checkpoint=%s\n", __func__, nCheckpoint.GetHex());
     return true;
 }
@@ -258,6 +251,51 @@ bool InvalidCheckpointRange(int nHeight)
     return nHeight > Params().Zerocoin_Block_LastGoodCheckpoint() && nHeight < Params().Zerocoin_Block_RecalculateAccumulators();
 }
 
+bool ValidateAccumulatorCheckpoint(const CBlock& block, CBlockIndex* pindex, AccumulatorMap& mapAccumulators)
+{
+    if (!fVerifyingBlocks && pindex->nHeight >= Params().Zerocoin_StartHeight() && pindex->nHeight % 10 == 0) {
+        uint256 nCheckpointCalculated = 0;
+
+        // if IDB, invalid outpoints must be calculated or else acc checkpoint will be incorrect
+        if (pindex->nHeight == Params().Zerocoin_Block_RecalculateAccumulators())
+            PopulateInvalidOutPointMap();
+
+        if (!CalculateAccumulatorCheckpoint(pindex->nHeight, nCheckpointCalculated, mapAccumulators)) {
+            //Calculate list of checkpoints that may be missing due to deletion on block 809000, and rewinding back before 809000
+            int nStop = Params().Zerocoin_Block_RecalculateAccumulators() + 20;
+            if (pindex->nHeight < nStop && pindex->nHeight > Params().Zerocoin_Block_LastGoodCheckpoint()) {
+                LogPrintf("%s : Checkpoint not found for block %d, recalculating accumulators\n", __func__, pindex->nHeight);
+                CBlockIndex* pindexCheckpoint = chainActive[Params().Zerocoin_Block_LastGoodCheckpoint()];
+                list<uint256> listCheckpoints;
+                while (pindexCheckpoint->nHeight <= nStop) {
+                    if (!count(listCheckpoints.begin(), listCheckpoints.end(), pindexCheckpoint->nAccumulatorCheckpoint))
+                        listCheckpoints.emplace_back(pindexCheckpoint->nAccumulatorCheckpoint);
+
+                    pindexCheckpoint = chainActive.Next(pindexCheckpoint);
+                    if (!pindexCheckpoint)
+                        break;
+                }
+
+                string strError;
+                if (!ReindexAccumulators(listCheckpoints, strError) || !CalculateAccumulatorCheckpoint(pindex->nHeight, nCheckpointCalculated, mapAccumulators))
+                    return error("%s : failed to recalculate accumulator checkpoint", __func__);
+            } else {
+                return error("%s : failed to calculate accumulator checkpoint", __func__);
+            }
+        }
+
+        if (nCheckpointCalculated != block.nAccumulatorCheckpoint) {
+            LogPrintf("%s: block=%d calculated: %s\n block: %s\n", __func__, pindex->nHeight, nCheckpointCalculated.GetHex(), block.nAccumulatorCheckpoint.GetHex());
+            return error("%s : accumulator does not match calculated value", __func__);
+        }
+    } else if (!fVerifyingBlocks) {
+        if (block.nAccumulatorCheckpoint != pindex->pprev->nAccumulatorCheckpoint)
+            return error("%s : new accumulator checkpoint generated on a block that is not multiple of 10", __func__);
+    }
+
+    return true;
+}
+
 bool GenerateAccumulatorWitness(const PublicCoin &coin, Accumulator& accumulator, AccumulatorWitness& witness, int nSecurityLevel, int& nMintsAdded, string& strError)
 {
     uint256 txid;

src/accumulators.h

@@ -6,20 +6,24 @@
 #define PIVX_ACCUMULATORS_H
 
 #include "libzerocoin/Accumulator.h"
-#include "libzerocoin/Denominations.h"
 #include "libzerocoin/Coin.h"
+#include "libzerocoin/Denominations.h"
 #include "primitives/zerocoin.h"
+#include "accumulatormap.h"
+#include "chain.h"
 #include "uint256.h"
 
 bool GenerateAccumulatorWitness(const libzerocoin::PublicCoin &coin, libzerocoin::Accumulator& accumulator, libzerocoin::AccumulatorWitness& witness, int nSecurityLevel, int& nMintsAdded, std::string& strError);
 bool GetAccumulatorValueFromDB(uint256 nCheckpoint, libzerocoin::CoinDenomination denom, CBigNum& bnAccValue);
 bool GetAccumulatorValueFromChecksum(uint32_t nChecksum, bool fMemoryOnly, CBigNum& bnAccValue);
 void AddAccumulatorChecksum(const uint32_t nChecksum, const CBigNum &bnValue, bool fMemoryOnly);
-bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint);
+bool CalculateAccumulatorCheckpoint(int nHeight, uint256& nCheckpoint, AccumulatorMap& mapAccumulators);
+void DatabaseChecksums(AccumulatorMap& mapAccumulators);
 bool LoadAccumulatorValuesFromDB(const uint256 nCheckpoint);
 bool EraseAccumulatorValues(const uint256& nCheckpointErase, const uint256& nCheckpointPrevious);
 uint32_t ParseChecksum(uint256 nChecksum, libzerocoin::CoinDenomination denomination);
 uint32_t GetChecksum(const CBigNum &bnValue);
 bool InvalidCheckpointRange(int nHeight);
+bool ValidateAccumulatorCheckpoint(const CBlock& block, CBlockIndex* pindex, AccumulatorMap& mapAccumulators);
 
 #endif //PIVX_ACCUMULATORS_H