QT freeze during batch verification fixed

qa/rpc-tests/spark_batching.py

@@ -1,10 +1,10 @@
 #!/usr/bin/env python3
-"""Test deferred Spark batch proof verification (-batching) across reindex.
+"""Test per-block Spark batch proof verification (-batching) across reindex.
 
 All blocks are mined with timestamps more than a day in the past, so a
 later reindex takes the old-block batching path: Spark spend proofs are
-collected into the batch container and must batch-verify before the node
-persists validation state and clears the reindex flag.
+collected and verified within each block before its validation state is
+persisted.
 """
 import os
 import time
@@ -38,9 +38,7 @@ def reindex(self, batching):
         open(os.path.join(self.options.tmpdir, "node0", "regtest", "debug.log"), "w").close()
         extra_args = [["-reindex", "-batching=" + ("1" if batching else "0")]]
         self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
-        # The tip can reach the target height while the final deferred batch
-        # is still pending, so a batched reindex is only complete once the
-        # batch verification success marker is in the log as well.
+        # Require proof that the reindex exercised the batching path.
         deadline = time.time() + 300
         while True:
             if self.nodes[0].getblockcount() >= blockcount and \
@@ -66,7 +64,7 @@ def wait_spark_balance(self, expected):
 
     def run_test(self):
         # Mine everything with old timestamps so a later reindex treats the
-        # whole chain as old blocks and defers Spark proof verification.
+        # whole chain as old blocks and batches Spark proof verification.
         set_node_times(self.nodes, int(time.time()) - 2 * 86400)
 
         self.nodes[0].generate(501)

src/batchproof_container.cpp

@@ -4,165 +4,53 @@
 #include "util.h"
 
 #include <boost/filesystem.hpp>
+#include <set>
+#include <unordered_map>
 
-extern bool fReindex;
-
-std::unique_ptr<BatchProofContainer> BatchProofContainer::instance;
-
-static boost::filesystem::path RecoveryMarkerPath()
-{
-    return GetDataDir() / "sparkbatchfailed";
-}
-
-static void WriteRecoveryMarker()
+namespace
 {
-    const auto path = RecoveryMarkerPath();
-    if (boost::filesystem::exists(path))
-        return;
-    FILE* file = fopen(path.string().c_str(), "wb");
-    if (file)
-        fclose(file);
-    else
-        LogPrintf("Failed to write Spark batch recovery marker\n");
-}
 
-bool BatchProofContainer::HasRecoveryMarker()
+bool VerifySparkBatch(
+    const std::vector<spark::SpendTransaction>& sparkTransactions,
+    const std::vector<uint256>& sparkTxIds,
+    const std::vector<spark::SpendTransaction>& historicalSparkTransactions,
+    const std::vector<uint256>& historicalSparkTxIds,
+    const std::unordered_map<uint64_t, std::vector<spark::Coin>>& coverSets)
 {
-    return boost::filesystem::exists(RecoveryMarkerPath());
-}
-
-void BatchProofContainer::RemoveRecoveryMarker()
-{
-    boost::filesystem::remove(RecoveryMarkerPath());
-}
-
-BatchProofContainer* BatchProofContainer::get_instance() {
-    if (instance) {
-        return instance.get();
-    } else {
-        instance.reset(new BatchProofContainer());
-        return instance.get();
-    }
-}
-
-void BatchProofContainer::init() {
-    tempSparkTransactions.clear();
-    tempSparkTxIds.clear();
-    tempHistoricalSparkTransactions.clear();
-    tempHistoricalSparkTxIds.clear();
-    if (fCollectProofs)
-        WriteRecoveryMarker();
-}
-
-void BatchProofContainer::finalize() {
-    if (fCollectProofs) {
-        sparkTransactions.insert(sparkTransactions.end(), tempSparkTransactions.begin(), tempSparkTransactions.end());
-        sparkTxIds.insert(sparkTxIds.end(), tempSparkTxIds.begin(), tempSparkTxIds.end());
-        historicalSparkTransactions.insert(
-            historicalSparkTransactions.end(),
-            tempHistoricalSparkTransactions.begin(),
-            tempHistoricalSparkTransactions.end());
-        historicalSparkTxIds.insert(
-            historicalSparkTxIds.end(),
-            tempHistoricalSparkTxIds.begin(),
-            tempHistoricalSparkTxIds.end());
-    }
-    tempSparkTransactions.clear();
-    tempSparkTxIds.clear();
-    tempHistoricalSparkTransactions.clear();
-    tempHistoricalSparkTxIds.clear();
-    fCollectProofs = false;
-}
-
-bool BatchProofContainer::verify_pending() {
-    bool passed = true;
-    if (!fCollectProofs) {
-        init();
-        passed = batch_spark();
-        if (!passed)
-            WriteRecoveryMarker();
-        else if (!fReindex)
-            RemoveRecoveryMarker();
-    }
-    fCollectProofs = false;
-    return passed;
-}
-
-void BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) {
-    tempSparkTransactions.push_back(tx);
-    tempSparkTxIds.push_back(txHash);
-}
-
-void BatchProofContainer::addHistorical(
-    const spark::SpendTransaction& tx, const uint256& txHash) {
-    tempHistoricalSparkTransactions.push_back(tx);
-    tempHistoricalSparkTxIds.push_back(txHash);
-}
-
-void BatchProofContainer::remove(const spark::SpendTransaction& tx) {
-    bool fBatchChanged = false;
-    for (std::size_t i = sparkTransactions.size(); i-- > 0;) {
-        if (sparkTransactions[i].getUsedLTags() == tx.getUsedLTags()) {
-            sparkTransactions.erase(sparkTransactions.begin() + i);
-            sparkTxIds.erase(sparkTxIds.begin() + i);
-            fBatchChanged = true;
-        }
-    }
-    for (std::size_t i = historicalSparkTransactions.size(); i-- > 0;) {
-        if (historicalSparkTransactions[i].getUsedLTags() == tx.getUsedLTags()) {
-            historicalSparkTransactions.erase(historicalSparkTransactions.begin() + i);
-            historicalSparkTxIds.erase(historicalSparkTxIds.begin() + i);
-            fBatchChanged = true;
-        }
-    }
-    if (fBatchChanged) {
-        // the pending batch changed, so a previous failure verdict no longer applies
-        fBatchFailed = false;
-    }
-}
-
-bool BatchProofContainer::batch_spark() {
     if (sparkTransactions.empty() && historicalSparkTransactions.empty())
         return true;
-    if (fBatchFailed)
-        return false;
 
     LogPrintf("Spark batch verification started.\n");
     uiInterface.UpdateProgressBarLabel("Batch verifying Spark Proofs...");
 
-    spark::CSparkState* sparkState = spark::CSparkState::GetState();
-    std::vector<spark::Coin> loadedCoverSet;
-    const spark::SpendTransaction::CoverSetProvider coverSetProvider =
-        [sparkState, &loadedCoverSet](uint64_t id)
-            -> const std::vector<spark::Coin>& {
-        loadedCoverSet.clear();
-        sparkState->GetCoinSet(static_cast<int32_t>(id), loadedCoverSet);
-        return loadedCoverSet;
-    };
     auto* params = spark::Params::get_default();
 
     bool passed = true;
     try {
         if (!sparkTransactions.empty()) {
             passed = spark::SpendTransaction::verify(
-                params, sparkTransactions, coverSetProvider);
+                params, sparkTransactions, coverSets);
         }
         if (passed && !historicalSparkTransactions.empty()) {
             passed = spark::SpendTransaction::verifyHistorical(
-                params, historicalSparkTransactions, coverSetProvider);
+                params, historicalSparkTransactions, coverSets);
         }
+    } catch (const std::bad_alloc&) {
+        throw;
     } catch (const std::exception &) {
         passed = false;
     }
 
     if (!passed) {
-        // Re-verify the retained proofs individually so the operator can see
+        // Re-verify the batch members individually so the operator can see
         // exactly which spends are invalid without a diagnostic reindex.
         for (std::size_t i = 0; i < sparkTransactions.size(); ++i) {
             bool fProofValid;
             try {
                 fProofValid = spark::SpendTransaction::verify(
-                    params, {sparkTransactions[i]}, coverSetProvider);
+                    params, {sparkTransactions[i]}, coverSets);
+            } catch (const std::bad_alloc&) {
+                throw;
             } catch (const std::exception &) {
                 fProofValid = false;
             }
@@ -174,7 +62,9 @@ bool BatchProofContainer::batch_spark() {
             bool fProofValid;
             try {
                 fProofValid = spark::SpendTransaction::verifyHistorical(
-                    params, {historicalSparkTransactions[i]}, coverSetProvider);
+                    params, {historicalSparkTransactions[i]}, coverSets);
+            } catch (const std::bad_alloc&) {
+                throw;
             } catch (const std::exception &) {
                 fProofValid = false;
             }
@@ -183,14 +73,122 @@ bool BatchProofContainer::batch_spark() {
             }
         }
         LogPrintf("Spark batch verification failed.\n");
-        fBatchFailed = true;
         return false;
     }
 
     LogPrintf("Spark batch verification finished successfully.\n");
+    return true;
+}
+
+} // namespace
+
+std::unique_ptr<BatchProofContainer> BatchProofContainer::instance;
+
+static boost::filesystem::path RecoveryMarkerPath()
+{
+    return GetDataDir() / "sparkbatchfailed";
+}
+
+bool BatchProofContainer::HasRecoveryMarker()
+{
+    return boost::filesystem::exists(RecoveryMarkerPath());
+}
+
+void BatchProofContainer::RemoveRecoveryMarker()
+{
+    boost::filesystem::remove(RecoveryMarkerPath());
+}
+
+BatchProofContainer* BatchProofContainer::get_instance()
+{
+    if (instance) {
+        return instance.get();
+    } else {
+        instance.reset(new BatchProofContainer());
+        return instance.get();
+    }
+}
+
+void BatchProofContainer::init(bool collectProofs)
+{
+    LOCK(cs_batch);
     sparkTransactions.clear();
     sparkTxIds.clear();
     historicalSparkTransactions.clear();
     historicalSparkTxIds.clear();
+    fCollectProofs = collectProofs;
+}
+
+void BatchProofContainer::abort()
+{
+    init();
+}
+
+void BatchProofContainer::finalize()
+{
+    LOCK(cs_batch);
+    fCollectProofs = false;
+}
+
+bool BatchProofContainer::verify_pending()
+{
+    std::vector<spark::SpendTransaction> snapshotTransactions;
+    std::vector<uint256> snapshotTxIds;
+    std::vector<spark::SpendTransaction> snapshotHistoricalTransactions;
+    std::vector<uint256> snapshotHistoricalTxIds;
+    {
+        LOCK(cs_batch);
+        if (fCollectProofs)
+            return true;
+
+        snapshotTransactions.swap(sparkTransactions);
+        snapshotTxIds.swap(sparkTxIds);
+        snapshotHistoricalTransactions.swap(historicalSparkTransactions);
+        snapshotHistoricalTxIds.swap(historicalSparkTxIds);
+    }
+
+    std::set<uint64_t> coverSetIds;
+    for (auto& tx : snapshotTransactions) {
+        for (uint64_t id : tx.getCoinGroupIds())
+            coverSetIds.insert(id);
+    }
+    for (auto& tx : snapshotHistoricalTransactions) {
+        for (uint64_t id : tx.getCoinGroupIds())
+            coverSetIds.insert(id);
+    }
+    std::unordered_map<uint64_t, std::vector<spark::Coin>> coverSets;
+    spark::CSparkState* sparkState = spark::CSparkState::GetState();
+    for (uint64_t id : coverSetIds) {
+        std::vector<spark::Coin> coins;
+        sparkState->GetCoinSet(static_cast<int32_t>(id), coins);
+        coverSets.emplace(id, std::move(coins));
+    }
+
+    return VerifySparkBatch(
+        snapshotTransactions,
+        snapshotTxIds,
+        snapshotHistoricalTransactions,
+        snapshotHistoricalTxIds,
+        coverSets);
+}
+
+bool BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash)
+{
+    LOCK(cs_batch);
+    if (!fCollectProofs)
+        return false;
+    sparkTransactions.push_back(tx);
+    sparkTxIds.push_back(txHash);
+    return true;
+}
+
+bool BatchProofContainer::addHistorical(
+    const spark::SpendTransaction& tx, const uint256& txHash)
+{
+    LOCK(cs_batch);
+    if (!fCollectProofs)
+        return false;
+    historicalSparkTransactions.push_back(tx);
+    historicalSparkTxIds.push_back(txHash);
     return true;
 }