Spark: fail closed on deferred batch verification

Author: reubenyap

src/batchproof_container.cpp

@@ -20,15 +20,23 @@ void BatchProofContainer::init() {
 void BatchProofContainer::finalize() {
     if (fCollectProofs) {
         sparkTransactions.insert(sparkTransactions.end(), tempSparkTransactions.begin(), tempSparkTransactions.end());
+        tempSparkTransactions.clear();
     }
     fCollectProofs = false;
 }
 
-void BatchProofContainer::verify() {
-    if (!fCollectProofs) {
-        batch_spark();
+bool BatchProofContainer::verify() {
+    if (fCollectProofs) {
+        fCollectProofs = false;
+        return true;
     }
-    fCollectProofs = false;
+
+    return batch_spark();
+}
+
+bool BatchProofContainer::verify_pending() {
+    finalize();
+    return batch_spark();
 }
 
 void BatchProofContainer::add(const spark::SpendTransaction& tx) {
@@ -42,12 +50,12 @@ void BatchProofContainer::remove(const spark::SpendTransaction& tx) {
                             sparkTransactions.end());
 }
 
-void BatchProofContainer::batch_spark() {
+bool BatchProofContainer::batch_spark() {
     if (!sparkTransactions.empty()){
         LogPrintf("Spark batch verification started.\n");
         uiInterface.UpdateProgressBarLabel("Batch verifying Spark Proofs...");
     } else {
-        return;
+        return true;
     }
 
     std::unordered_map<uint64_t, std::vector<spark::Coin>> cover_sets;
@@ -75,10 +83,11 @@ void BatchProofContainer::batch_spark() {
 
     if (!passed) {
         LogPrintf("Spark batch verification failed.");
-        throw std::invalid_argument("Spark batch verification failed, please run Firo with -reindex -batching=0");
+        return false;
     }
 
     if (!sparkTransactions.empty())
         LogPrintf("Spark batch verification finished successfully.\n");
     sparkTransactions.clear();
-}
+    return true;
+}

src/batchproof_container.h

@@ -15,11 +15,12 @@ class BatchProofContainer {
 
     void finalize();
 
-    void verify();
+    bool verify();
+    bool verify_pending();
 
     void add(const spark::SpendTransaction& tx);
     void remove(const spark::SpendTransaction& tx);
-    void batch_spark();
+    bool batch_spark();
 public:
     bool fCollectProofs = 0;
 

src/init.cpp

@@ -41,7 +41,6 @@
 #include "validationinterface.h"
 #include "validation.h"
 #include "mtpstate.h"
-#include "batchproof_container.h"
 #include <crypto/progpow/include/ethash/progpow.hpp>
 #include "leveldb/env.h"
 
@@ -264,8 +263,9 @@ void Shutdown()
     StopHTTPServer();
     llmq::StopLLMQSystem();
 
-    BatchProofContainer::get_instance()->finalize();
-    BatchProofContainer::get_instance()->verify();
+    CValidationState sparkBatchState;
+    if (!VerifyPendingSparkBatch(sparkBatchState, "shutdown"))
+        LogPrintf("Shutdown continuing after Spark batch verification failure: %s\n", FormatStateMessage(sparkBatchState));
 
 #ifdef ENABLE_WALLET
     if (pwalletMain)
@@ -761,6 +761,11 @@ void ThreadImport(std::vector <boost::filesystem::path> vImportFiles) {
             LoadExternalBlockFile(chainparams, file, &pos);
             nFile++;
         }
+        CValidationState state;
+        if (!VerifyPendingSparkBatch(state, "clearing reindex flag")) {
+            LogPrintf("Reindexing stopped before clearing reindex flag: %s\n", FormatStateMessage(state));
+            return;
+        }
         pblocktree->WriteReindexing(false);
         fReindex = false;
         LogPrintf("Reindexing finished\n");

src/validation.cpp

@@ -2234,6 +2234,17 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std
     return state.Error(strMessage);
 }
 
+bool VerifyPendingSparkBatch(CValidationState& state, const std::string& reason)
+{
+    BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance();
+    if (!batchProofContainer->verify_pending()) {
+        return AbortNode(state,
+                         strprintf("Spark batch verification failed before %s", reason),
+                         _("Spark batch verification failed. Please restart with -reindex -batching=0 to identify the invalid Spark spend."));
+    }
+    return true;
+}
+
 enum DisconnectResult
 {
     DISCONNECT_OK,      // All good.
@@ -3084,6 +3095,8 @@ bool static FlushStateToDisk(CValidationState &state, FlushStateMode mode, int n
     bool fPeriodicFlush = mode == FLUSH_STATE_PERIODIC && nNow > nLastFlush + (int64_t)DATABASE_FLUSH_INTERVAL * 1000000;
     // Combine all conditions that result in a full cache flush.
     bool fDoFullFlush = (mode == FLUSH_STATE_ALWAYS) || fCacheLarge || fCacheCritical || fPeriodicFlush || fFlushForPrune;
+    if ((fDoFullFlush || fPeriodicWrite) && !VerifyPendingSparkBatch(state, "flushing block index or chainstate"))
+        return false;
     // Write blocks and block index to disk.
     if (fDoFullFlush || fPeriodicWrite) {
         // Depend on nMinDiskSpace to ensure we can write block index
@@ -3798,7 +3811,11 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams,
         // Do batch verification if we reach 1 day old block,
         BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance();
         batchProofContainer->fCollectProofs = ((GetSystemTimeInSeconds() - pindexNewTip->GetBlockTime()) > 86400) && GetBoolArg("-batching", true);
-        batchProofContainer->verify();
+        if (!batchProofContainer->verify()) {
+            return AbortNode(state,
+                             "Spark batch verification failed",
+                             _("Spark batch verification failed. Please restart with -reindex -batching=0 to identify the invalid Spark spend."));
+        }
 
         // When we reach this point, we switched to a new tip (stored in pindexNewTip).
 

src/validation.h

@@ -303,6 +303,8 @@ bool IsInitialBlockDownload();
 bool GetTransaction(const uint256 &hash, CTransactionRef &tx, const Consensus::Params& params, uint256 &hashBlock, bool fAllowSlow = false);
 /** Find the best known block, and make it the tip of the block chain */
 bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr<const CBlock> pblock = std::shared_ptr<const CBlock>());
+/** Verify any pending deferred Spark proof batch before persisting validation state. */
+bool VerifyPendingSparkBatch(CValidationState& state, const std::string& reason);
 CAmount GetBlockSubsidyWithMTPFlag(int nHeight, const Consensus::Params& consensusParams, bool fMTP, bool fShorterBlockDistance);
 CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams, int nTime = 1475020800);
 CAmount GetMasternodePayment(int nHeight, int nTime, CAmount blockValue);