From f27193ee6c7cf249410be44b80037664213c3bb1 Mon Sep 17 00:00:00 2001 From: levonpetrosyan93 Date: Sun, 30 Aug 2026 18:51:52 +0400 Subject: [PATCH 01/12] QT freez during batch verifiction fixed --- src/validation.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index ed69613b9a..a340fc0914 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3988,12 +3988,13 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, for (unsigned int i = 0; i < block.vtx.size(); i++) GetMainSignals().SyncTransaction(*block.vtx[i], pair.first, i); } - BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - batchProofContainer->fCollectProofs = ShouldBatchSparkProofs(pindexNewTip); - if (!VerifyPendingSparkBatch(state, "connecting new tip")) - return false; } + BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); + batchProofContainer->fCollectProofs = ShouldBatchSparkProofs(pindexNewTip); + if (!VerifyPendingSparkBatch(state, "connecting new tip")) + return false; + // When we reach this point, we switched to a new tip (stored in pindexNewTip). // Notifications/callbacks that can run without cs_main From 8deca81f8cd1060a5ebebf3e4d0fc7957e3ccfa5 Mon Sep 17 00:00:00 2001 From: levonpetrosyan93 Date: Mon, 31 Aug 2026 15:21:33 +0400 Subject: [PATCH 02/12] review comments resolved --- src/batchproof_container.cpp | 239 ++++++++++++++++++++++------------- src/batchproof_container.h | 12 +- 2 files changed, 158 insertions(+), 93 deletions(-) diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index 79e68ef06f..0f91aacd49 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -4,9 +4,79 @@ #include "util.h" #include +#include +#include extern bool fReindex; +namespace { + +bool VerifySparkBatch( + const std::vector& sparkTransactions, + const std::vector& sparkTxIds, + const std::vector& historicalSparkTransactions, + const std::vector& historicalSparkTxIds, + const std::unordered_map>& coverSets) +{ + if (sparkTransactions.empty() && historicalSparkTransactions.empty()) + return true; + + LogPrintf("Spark batch verification started.\n"); + uiInterface.UpdateProgressBarLabel("Batch verifying Spark Proofs..."); + + auto* params = spark::Params::get_default(); + + bool passed = true; + try { + if (!sparkTransactions.empty()) { + passed = spark::SpendTransaction::verify( + params, sparkTransactions, coverSets); + } + if (passed && !historicalSparkTransactions.empty()) { + passed = spark::SpendTransaction::verifyHistorical( + params, historicalSparkTransactions, coverSets); + } + } catch (const std::exception &) { + passed = false; + } + + if (!passed) { + // Re-verify the retained proofs 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]}, coverSets); + } catch (const std::exception &) { + fProofValid = false; + } + if (!fProofValid) { + LogPrintf("Spark batch verification failed for spend transaction %s.\n", sparkTxIds[i].ToString()); + } + } + for (std::size_t i = 0; i < historicalSparkTransactions.size(); ++i) { + bool fProofValid; + try { + fProofValid = spark::SpendTransaction::verifyHistorical( + params, {historicalSparkTransactions[i]}, coverSets); + } catch (const std::exception &) { + fProofValid = false; + } + if (!fProofValid) { + LogPrintf("Spark batch verification failed for spend transaction %s.\n", historicalSparkTxIds[i].ToString()); + } + } + LogPrintf("Spark batch verification failed.\n"); + return false; + } + + LogPrintf("Spark batch verification finished successfully.\n"); + return true; +} + +} // namespace + std::unique_ptr BatchProofContainer::instance; static boost::filesystem::path RecoveryMarkerPath() @@ -46,6 +116,7 @@ BatchProofContainer* BatchProofContainer::get_instance() { } void BatchProofContainer::init() { + LOCK(cs_batch); tempSparkTransactions.clear(); tempSparkTxIds.clear(); tempHistoricalSparkTransactions.clear(); @@ -55,6 +126,7 @@ void BatchProofContainer::init() { } void BatchProofContainer::finalize() { + LOCK(cs_batch); if (fCollectProofs) { sparkTransactions.insert(sparkTransactions.end(), tempSparkTransactions.begin(), tempSparkTransactions.end()); sparkTxIds.insert(sparkTxIds.end(), tempSparkTxIds.begin(), tempSparkTxIds.end()); @@ -75,31 +147,103 @@ void BatchProofContainer::finalize() { } bool BatchProofContainer::verify_pending() { - bool passed = true; - if (!fCollectProofs) { - init(); - passed = batch_spark(); - if (!passed) - WriteRecoveryMarker(); - else if (!fReindex) - RemoveRecoveryMarker(); + { + LOCK(cs_batch); + if (fCollectProofs) { + fCollectProofs = false; + return true; + } + } + + while (true) { + std::vector snapshotTransactions; + std::vector snapshotTxIds; + std::vector snapshotHistoricalTransactions; + std::vector snapshotHistoricalTxIds; + std::size_t batchSize = 0; + std::size_t historicalBatchSize = 0; + { + LOCK(cs_batch); + init(); + if (fBatchFailed) { + fCollectProofs = false; + return false; + } + if (sparkTransactions.empty() && historicalSparkTransactions.empty()) { + fCollectProofs = false; + return true; + } + + snapshotTransactions = sparkTransactions; + snapshotTxIds = sparkTxIds; + snapshotHistoricalTransactions = historicalSparkTransactions; + snapshotHistoricalTxIds = historicalSparkTxIds; + batchSize = sparkTransactions.size(); + historicalBatchSize = historicalSparkTransactions.size(); + } + + std::set 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> coverSets; + spark::CSparkState* sparkState = spark::CSparkState::GetState(); + for (uint64_t id : coverSetIds) { + std::vector coins; + sparkState->GetCoinSet(static_cast(id), coins); + coverSets.emplace(id, std::move(coins)); + } + + const bool passed = VerifySparkBatch( + snapshotTransactions, + snapshotTxIds, + snapshotHistoricalTransactions, + snapshotHistoricalTxIds, + coverSets); + + LOCK(cs_batch); + if (sparkTransactions.size() != batchSize || + historicalSparkTransactions.size() != historicalBatchSize) { + continue; + } + + fCollectProofs = false; + if (passed) { + if (!fReindex) + RemoveRecoveryMarker(); + sparkTransactions.clear(); + sparkTxIds.clear(); + historicalSparkTransactions.clear(); + historicalSparkTxIds.clear(); + return true; + } + + WriteRecoveryMarker(); + fBatchFailed = true; + return false; } - fCollectProofs = false; - return passed; } void BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) { + LOCK(cs_batch); tempSparkTransactions.push_back(tx); tempSparkTxIds.push_back(txHash); } void BatchProofContainer::addHistorical( const spark::SpendTransaction& tx, const uint256& txHash) { + LOCK(cs_batch); tempHistoricalSparkTransactions.push_back(tx); tempHistoricalSparkTxIds.push_back(txHash); } void BatchProofContainer::remove(const spark::SpendTransaction& tx) { + LOCK(cs_batch); bool fBatchChanged = false; for (std::size_t i = sparkTransactions.size(); i-- > 0;) { if (sparkTransactions[i].getUsedLTags() == tx.getUsedLTags()) { @@ -116,81 +260,6 @@ void BatchProofContainer::remove(const spark::SpendTransaction& tx) { } } 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 loadedCoverSet; - const spark::SpendTransaction::CoverSetProvider coverSetProvider = - [sparkState, &loadedCoverSet](uint64_t id) - -> const std::vector& { - loadedCoverSet.clear(); - sparkState->GetCoinSet(static_cast(id), loadedCoverSet); - return loadedCoverSet; - }; - auto* params = spark::Params::get_default(); - - bool passed = true; - try { - if (!sparkTransactions.empty()) { - passed = spark::SpendTransaction::verify( - params, sparkTransactions, coverSetProvider); - } - if (passed && !historicalSparkTransactions.empty()) { - passed = spark::SpendTransaction::verifyHistorical( - params, historicalSparkTransactions, coverSetProvider); - } - } catch (const std::exception &) { - passed = false; - } - - if (!passed) { - // Re-verify the retained proofs 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); - } catch (const std::exception &) { - fProofValid = false; - } - if (!fProofValid) { - LogPrintf("Spark batch verification failed for spend transaction %s.\n", sparkTxIds[i].ToString()); - } - } - for (std::size_t i = 0; i < historicalSparkTransactions.size(); ++i) { - bool fProofValid; - try { - fProofValid = spark::SpendTransaction::verifyHistorical( - params, {historicalSparkTransactions[i]}, coverSetProvider); - } catch (const std::exception &) { - fProofValid = false; - } - if (!fProofValid) { - LogPrintf("Spark batch verification failed for spend transaction %s.\n", historicalSparkTxIds[i].ToString()); - } - } - LogPrintf("Spark batch verification failed.\n"); - fBatchFailed = true; - return false; - } - - LogPrintf("Spark batch verification finished successfully.\n"); - sparkTransactions.clear(); - sparkTxIds.clear(); - historicalSparkTransactions.clear(); - historicalSparkTxIds.clear(); - return true; -} diff --git a/src/batchproof_container.h b/src/batchproof_container.h index 8a8deecbba..a929539d8d 100644 --- a/src/batchproof_container.h +++ b/src/batchproof_container.h @@ -4,6 +4,7 @@ #include #include "chain.h" #include "libspark/spend_transaction.h" +#include "sync.h" extern CChain chainActive; @@ -31,22 +32,17 @@ class BatchProofContainer { void add(const spark::SpendTransaction& tx, const uint256& txHash); void addHistorical(const spark::SpendTransaction& tx, const uint256& txHash); void remove(const spark::SpendTransaction& tx); -public: - bool fCollectProofs = 0; -private: - bool batch_spark(); + bool fCollectProofs = false; +private: static std::unique_ptr instance; - // a pending batch failed verification; fail fast until the batch changes + mutable CCriticalSection cs_batch; bool fBatchFailed = false; - // temp spark transaction proofs and the txids they came from std::vector tempSparkTransactions; std::vector tempSparkTxIds; std::vector tempHistoricalSparkTransactions; std::vector tempHistoricalSparkTxIds; - - // spark transaction proofs and the txids they came from std::vector sparkTransactions; std::vector sparkTxIds; std::vector historicalSparkTransactions; From df973799f7d42e9b0f745b3638743665efe75385 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Tue, 1 Sep 2026 03:48:01 +0800 Subject: [PATCH 03/12] Spark: make batch enrollment atomic --- src/batchproof_container.cpp | 14 ++++++++++---- src/batchproof_container.h | 13 ++++++------- src/spark/state.cpp | 13 +++++++------ src/test/spark_batch_test.cpp | 19 +++++++++++++----- src/test/spark_tests.cpp | 36 +++++++++-------------------------- src/validation.cpp | 8 +++----- 6 files changed, 49 insertions(+), 54 deletions(-) diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index 0f91aacd49..85112d5c10 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -115,12 +115,13 @@ BatchProofContainer* BatchProofContainer::get_instance() { } } -void BatchProofContainer::init() { +void BatchProofContainer::init(bool collectProofs) { LOCK(cs_batch); tempSparkTransactions.clear(); tempSparkTxIds.clear(); tempHistoricalSparkTransactions.clear(); tempHistoricalSparkTxIds.clear(); + fCollectProofs = collectProofs; if (fCollectProofs) WriteRecoveryMarker(); } @@ -150,7 +151,6 @@ bool BatchProofContainer::verify_pending() { { LOCK(cs_batch); if (fCollectProofs) { - fCollectProofs = false; return true; } } @@ -229,17 +229,23 @@ bool BatchProofContainer::verify_pending() { } } -void BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) { +bool BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) { LOCK(cs_batch); + if (!fCollectProofs) + return false; tempSparkTransactions.push_back(tx); tempSparkTxIds.push_back(txHash); + return true; } -void BatchProofContainer::addHistorical( +bool BatchProofContainer::addHistorical( const spark::SpendTransaction& tx, const uint256& txHash) { LOCK(cs_batch); + if (!fCollectProofs) + return false; tempHistoricalSparkTransactions.push_back(tx); tempHistoricalSparkTxIds.push_back(txHash); + return true; } void BatchProofContainer::remove(const spark::SpendTransaction& tx) { diff --git a/src/batchproof_container.h b/src/batchproof_container.h index a929539d8d..638ea8c38c 100644 --- a/src/batchproof_container.h +++ b/src/batchproof_container.h @@ -12,14 +12,14 @@ class BatchProofContainer { public: static BatchProofContainer* get_instance(); - void init(); + void init(bool collectProofs = false); void finalize(); /** * Verify the finalized pending Spark batch when proofs are not being - * collected. Matches master's verify() gate: a no-op while fCollectProofs - * is set, so IBD keeps accumulating until a recent tip. + * collected. A no-op while collection is active, so IBD keeps + * accumulating until a recent tip. * * @return true if collecting, if no batch is pending, or if the batch * verifies; false on verification failure (pending proofs kept). @@ -29,15 +29,14 @@ class BatchProofContainer { static bool HasRecoveryMarker(); static void RemoveRecoveryMarker(); - void add(const spark::SpendTransaction& tx, const uint256& txHash); - void addHistorical(const spark::SpendTransaction& tx, const uint256& txHash); + bool add(const spark::SpendTransaction& tx, const uint256& txHash); + bool addHistorical(const spark::SpendTransaction& tx, const uint256& txHash); void remove(const spark::SpendTransaction& tx); - bool fCollectProofs = false; - private: static std::unique_ptr instance; mutable CCriticalSection cs_batch; + bool fCollectProofs = false; bool fBatchFailed = false; std::vector tempSparkTransactions; std::vector tempSparkTxIds; diff --git a/src/spark/state.cpp b/src/spark/state.cpp index 45988c3c3e..255cfb4a43 100644 --- a/src/spark/state.cpp +++ b/src/spark/state.cpp @@ -1066,7 +1066,7 @@ bool CheckSparkSpendTransaction( std::unordered_map cover_set_data; BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - bool useBatching = batchProofContainer->fCollectProofs && !isVerifyDB && !isCheckWallet && sparkTxInfo && !sparkTxInfo->fInfoIsComplete; + bool useBatching = !isVerifyDB && !isCheckWallet && sparkTxInfo && !sparkTxInfo->fInfoIsComplete; for (const auto& idAndHash : idAndBlockHashes) { const uint64_t wireGroupId = idAndHash.first; @@ -1214,13 +1214,14 @@ bool CheckSparkSpendTransaction( // if we are collecting proofs, skip verification and collect proofs // add proofs into container + bool addedToBatch = false; if (useBatching) { + addedToBatch = isChaumV2 || requireChaumV1SingleInput + ? batchProofContainer->add(*spend, hashTx) + : batchProofContainer->addHistorical(*spend, hashTx); + } + if (addedToBatch) { passVerify = true; - if (isChaumV2 || requireChaumV1SingleInput) { - batchProofContainer->add(*spend, hashTx); - } else { - batchProofContainer->addHistorical(*spend, hashTx); - } } else { try { bool haveCachedSuccess = false; diff --git a/src/test/spark_batch_test.cpp b/src/test/spark_batch_test.cpp index 71976dc777..39901670de 100644 --- a/src/test/spark_batch_test.cpp +++ b/src/test/spark_batch_test.cpp @@ -41,8 +41,7 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) LOCK(cs_main); CValidationState state; spark::CSparkTxInfo info; - container->fCollectProofs = true; - container->init(); + container->init(true); BOOST_CHECK(spark::CheckSparkTransaction( spendTx, state, spendTx.GetHash(), false, chainActive.Height(), false, true, &info)); container->finalize(); @@ -61,10 +60,20 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) invalidSpend.setVout(0); BOOST_REQUIRE(invalidSpend.getUsedLTags() != spark::ParseSparkSpend(spendTx).getUsedLTags()); + // Checking a pending batch while collection is active must not close the + // collection window. Otherwise a proof can be skipped without being + // enqueued at the old-to-recent batching boundary. + container->init(true); + BOOST_CHECK(container->verify_pending()); + BOOST_CHECK(container->add(invalidSpend, spendTxB.GetHash())); + container->finalize(); + BOOST_CHECK(!container->verify_pending()); + container->remove(invalidSpend); + BOOST_CHECK(container->verify_pending()); + collectSpend(); - container->fCollectProofs = true; - container->init(); - container->add(invalidSpend, spendTxB.GetHash()); + container->init(true); + BOOST_REQUIRE(container->add(invalidSpend, spendTxB.GetHash())); container->finalize(); // A batch holding a valid and an invalid proof fails and latches. diff --git a/src/test/spark_tests.cpp b/src/test/spark_tests.cpp index bb1e9366fb..244bf8007e 100644 --- a/src/test/spark_tests.cpp +++ b/src/test/spark_tests.cpp @@ -1528,7 +1528,6 @@ BOOST_AUTO_TEST_CASE(spark_v2_activation_and_wallet_selection) } ~ResetActivationHeights() { - BatchProofContainer::get_instance()->fCollectProofs = false; BatchProofContainer::get_instance()->init(); consensus.nSparkNamesStartBlock = sparkNamesStartBlock; } @@ -1709,8 +1708,7 @@ BOOST_AUTO_TEST_CASE(spark_v2_activation_and_wallet_selection) &activeV2Info)); BatchProofContainer* batch = BatchProofContainer::get_instance(); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState batchedV2State; CSparkTxInfo batchedV2Info; BOOST_REQUIRE(CheckSparkTransaction( @@ -2105,13 +2103,11 @@ BOOST_AUTO_TEST_CASE(unbound_cover_set_is_rejected_after_chaum_v2) referencedBlock->sparkSetHash.erase(groupId); BatchProofContainer* batch = BatchProofContainer::get_instance(); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); struct RestoreBatchCollection { BatchProofContainer* batch; ~RestoreBatchCollection() { - batch->fCollectProofs = false; batch->init(); } } restoreBatch{batch}; @@ -2970,7 +2966,6 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) } ~ResetBatchAndActivation() { - batch->fCollectProofs = false; batch->init(); } } reset{batch}; @@ -3008,8 +3003,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) parsedAlias.getCoinGroupIds().front() > static_cast(std::numeric_limits::max())); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState historicalState; CSparkTxInfo historicalInfo; BOOST_REQUIRE(CheckSparkTransaction( @@ -3040,8 +3034,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) true, &legacyAliasInfo)); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState legacyBatchState; CSparkTxInfo legacyBatchInfo; BOOST_REQUIRE(CheckSparkTransaction( @@ -3060,8 +3053,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) // Exercise the post-single-input batch as well; it has a separate // collection and verification path. UpdateRegtestSparkSingleInputHeight(chainActive.Height()); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState currentBatchState; CSparkTxInfo currentBatchInfo; BOOST_REQUIRE(CheckSparkTransaction( @@ -3108,8 +3100,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) BOOST_REQUIRE(activeAliasState.IsInvalid(activeAliasDoS)); BOOST_CHECK_EQUAL(activeAliasDoS, 100); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState activeBatchAliasState; CSparkTxInfo activeBatchAliasInfo; BOOST_CHECK(!CheckSparkTransaction( @@ -3124,8 +3115,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) batch->finalize(); BOOST_CHECK(batch->verify_pending()); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState activeState; CSparkTxInfo activeInfo; BOOST_CHECK(!CheckSparkTransaction( @@ -3149,7 +3139,6 @@ BOOST_AUTO_TEST_CASE(batched_spark_proofs_are_verified_inside_connect_block) BatchProofContainer* batch; ~ResetBatch() { - batch->fCollectProofs = false; batch->init(); } } reset{batch}; @@ -3211,7 +3200,6 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) BatchProofContainer* batch; ~ResetBatch() { - batch->fCollectProofs = false; batch->init(); } } reset{batch}; @@ -3254,7 +3242,6 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) // Master deferred batching does not abort() on ConnectBlock failure; the // next ConnectBlock init() (or an explicit init here) drops temps. batch->init(); - batch->fCollectProofs = false; BOOST_CHECK(batch->verify_pending()); mempool.clear(); @@ -3263,7 +3250,6 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) BOOST_AUTO_TEST_CASE(verifydb_level_four_reconnects_spark_spend_and_mints) { - BatchProofContainer::get_instance()->fCollectProofs = false; BatchProofContainer::get_instance()->init(); GenerateBlocks(500); @@ -3286,7 +3272,6 @@ BOOST_AUTO_TEST_CASE(verifydb_level_four_reconnects_spark_spend_and_mints) BOOST_AUTO_TEST_CASE(verifydb_rejects_invalid_standalone_spark_mint) { - BatchProofContainer::get_instance()->fCollectProofs = false; BatchProofContainer::get_instance()->init(); GenerateBlocks(500); @@ -3323,7 +3308,6 @@ BOOST_AUTO_TEST_CASE(verifydb_rejects_invalid_standalone_spark_mint) BOOST_AUTO_TEST_CASE(verifydb_rejects_same_block_spark_double_spend) { - BatchProofContainer::get_instance()->fCollectProofs = false; BatchProofContainer::get_instance()->init(); GenerateBlocks(500); @@ -3392,7 +3376,6 @@ BOOST_AUTO_TEST_CASE(verifydb_rejects_same_block_spark_double_spend) BOOST_AUTO_TEST_CASE(verifydb_rejects_cross_block_spark_double_spend) { - BatchProofContainer::get_instance()->fCollectProofs = false; BatchProofContainer::get_instance()->init(); GenerateBlocks(500); @@ -3874,7 +3857,7 @@ BOOST_AUTO_TEST_CASE(spark_unknown_cover_set_reference_is_not_mempool_admissible UpdateRegtestSparkChaumV2Height(exactReferencesHeight); BatchProofContainer* batch = BatchProofContainer::get_instance(); - batch->fCollectProofs = false; + batch->init(); CValidationState legacyExtraReferenceState; CSparkTxInfo legacyExtraReferenceInfo; BOOST_REQUIRE(CheckSparkTransaction( @@ -3887,8 +3870,7 @@ BOOST_AUTO_TEST_CASE(spark_unknown_cover_set_reference_is_not_mempool_admissible true, &legacyExtraReferenceInfo)); - batch->init(); - batch->fCollectProofs = true; + batch->init(true); CValidationState legacyBatchedExtraReferenceState; CSparkTxInfo legacyBatchedExtraReferenceInfo; BOOST_REQUIRE(CheckSparkTransaction( diff --git a/src/validation.cpp b/src/validation.cpp index a340fc0914..11f405c9f8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2799,8 +2799,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin bool isMainNet = chainparams.GetConsensus().IsMain(); // batch verify Lelantus/Sigma if block is older than a day, that means we are syncing or reindexing BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - batchProofContainer->fCollectProofs = ShouldBatchSparkProofs(pindex); - batchProofContainer->init(); + batchProofContainer->init(ShouldBatchSparkProofs(pindex)); std::size_t nSigma = 0; std::size_t nLelantus = 0; @@ -3990,9 +3989,8 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } } - BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - batchProofContainer->fCollectProofs = ShouldBatchSparkProofs(pindexNewTip); - if (!VerifyPendingSparkBatch(state, "connecting new tip")) + if (!ShouldBatchSparkProofs(pindexNewTip) && + !VerifyPendingSparkBatch(state, "connecting new tip")) return false; // When we reach this point, we switched to a new tip (stored in pindexNewTip). From 8e9150e38d147f7c06c7e28c088fdb0d40658eba Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Tue, 1 Sep 2026 03:48:10 +0800 Subject: [PATCH 04/12] Spark: lock complete cover-set reads --- src/spark/state.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/spark/state.cpp b/src/spark/state.cpp index 255cfb4a43..82c1fc88a5 100644 --- a/src/spark/state.cpp +++ b/src/spark/state.cpp @@ -1920,14 +1920,11 @@ CSparkState* CSparkState::GetState() { void CSparkState::GetCoinSet( int coinGroupID, std::vector& coins_out) { - int maxHeight; uint256 blockHash; std::vector setHash; - { - FIRO_UNUSED const auto ¶ms = ::Params().GetConsensus(); - LOCK(cs_main); - maxHeight = chainActive.Height() - (ZC_MINT_CONFIRMATIONS - 1); - } + FIRO_UNUSED const auto ¶ms = ::Params().GetConsensus(); + LOCK(cs_main); + int maxHeight = chainActive.Height() - (ZC_MINT_CONFIRMATIONS - 1); GetCoinSetForSpend( &chainActive, maxHeight, From f8d59594beed5c6d2d02ed38d0c37fc69791e76f Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Tue, 1 Sep 2026 03:55:40 +0800 Subject: [PATCH 05/12] Validation: verify Spark batches before commit Keep each batch local to ConnectBlock, verify it under cs_main before global or persistent state changes, and discard it on every exit. Retain legacy recovery-marker handling for upgrades, but stop creating markers for block-local work. --- qa/rpc-tests/spark_batching.py | 12 +-- src/batchproof_container.cpp | 187 +++++++++------------------------ src/batchproof_container.h | 15 +-- src/init.cpp | 25 +---- src/test/spark_batch_test.cpp | 62 +++++++---- src/test/spark_tests.cpp | 54 +++++++--- src/validation.cpp | 74 +++++-------- src/validation.h | 6 -- 8 files changed, 174 insertions(+), 261 deletions(-) diff --git a/qa/rpc-tests/spark_batching.py b/qa/rpc-tests/spark_batching.py index 8367b200ba..a3189a5f11 100755 --- a/qa/rpc-tests/spark_batching.py +++ b/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) diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index 85112d5c10..e5ea1b6720 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -7,8 +7,6 @@ #include #include -extern bool fReindex; - namespace { bool VerifySparkBatch( @@ -36,18 +34,22 @@ bool VerifySparkBatch( passed = spark::SpendTransaction::verifyHistorical( 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]}, coverSets); + } catch (const std::bad_alloc &) { + throw; } catch (const std::exception &) { fProofValid = false; } @@ -60,6 +62,8 @@ bool VerifySparkBatch( try { fProofValid = spark::SpendTransaction::verifyHistorical( params, {historicalSparkTransactions[i]}, coverSets); + } catch (const std::bad_alloc &) { + throw; } catch (const std::exception &) { fProofValid = false; } @@ -84,18 +88,6 @@ static boost::filesystem::path RecoveryMarkerPath() return GetDataDir() / "sparkbatchfailed"; } -static void WriteRecoveryMarker() -{ - 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() { return boost::filesystem::exists(RecoveryMarkerPath()); @@ -117,124 +109,69 @@ BatchProofContainer* BatchProofContainer::get_instance() { void BatchProofContainer::init(bool collectProofs) { LOCK(cs_batch); - tempSparkTransactions.clear(); - tempSparkTxIds.clear(); - tempHistoricalSparkTransactions.clear(); - tempHistoricalSparkTxIds.clear(); + sparkTransactions.clear(); + sparkTxIds.clear(); + historicalSparkTransactions.clear(); + historicalSparkTxIds.clear(); fCollectProofs = collectProofs; - if (fCollectProofs) - WriteRecoveryMarker(); +} + +void BatchProofContainer::abort() { + init(); } void BatchProofContainer::finalize() { LOCK(cs_batch); - 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() { + std::vector snapshotTransactions; + std::vector snapshotTxIds; + std::vector snapshotHistoricalTransactions; + std::vector snapshotHistoricalTxIds; { LOCK(cs_batch); - if (fCollectProofs) { + if (fCollectProofs) return true; - } - } - - while (true) { - std::vector snapshotTransactions; - std::vector snapshotTxIds; - std::vector snapshotHistoricalTransactions; - std::vector snapshotHistoricalTxIds; - std::size_t batchSize = 0; - std::size_t historicalBatchSize = 0; - { - LOCK(cs_batch); - init(); - if (fBatchFailed) { - fCollectProofs = false; - return false; - } - if (sparkTransactions.empty() && historicalSparkTransactions.empty()) { - fCollectProofs = false; - return true; - } - - snapshotTransactions = sparkTransactions; - snapshotTxIds = sparkTxIds; - snapshotHistoricalTransactions = historicalSparkTransactions; - snapshotHistoricalTxIds = historicalSparkTxIds; - batchSize = sparkTransactions.size(); - historicalBatchSize = historicalSparkTransactions.size(); - } - - std::set 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> coverSets; - spark::CSparkState* sparkState = spark::CSparkState::GetState(); - for (uint64_t id : coverSetIds) { - std::vector coins; - sparkState->GetCoinSet(static_cast(id), coins); - coverSets.emplace(id, std::move(coins)); - } - - const bool passed = VerifySparkBatch( - snapshotTransactions, - snapshotTxIds, - snapshotHistoricalTransactions, - snapshotHistoricalTxIds, - coverSets); - LOCK(cs_batch); - if (sparkTransactions.size() != batchSize || - historicalSparkTransactions.size() != historicalBatchSize) { - continue; - } - - fCollectProofs = false; - if (passed) { - if (!fReindex) - RemoveRecoveryMarker(); - sparkTransactions.clear(); - sparkTxIds.clear(); - historicalSparkTransactions.clear(); - historicalSparkTxIds.clear(); - return true; - } + snapshotTransactions.swap(sparkTransactions); + snapshotTxIds.swap(sparkTxIds); + snapshotHistoricalTransactions.swap(historicalSparkTransactions); + snapshotHistoricalTxIds.swap(historicalSparkTxIds); + } - WriteRecoveryMarker(); - fBatchFailed = true; - return false; + std::set 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> coverSets; + spark::CSparkState* sparkState = spark::CSparkState::GetState(); + for (uint64_t id : coverSetIds) { + std::vector coins; + sparkState->GetCoinSet(static_cast(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; - tempSparkTransactions.push_back(tx); - tempSparkTxIds.push_back(txHash); + sparkTransactions.push_back(tx); + sparkTxIds.push_back(txHash); return true; } @@ -243,29 +180,7 @@ bool BatchProofContainer::addHistorical( LOCK(cs_batch); if (!fCollectProofs) return false; - tempHistoricalSparkTransactions.push_back(tx); - tempHistoricalSparkTxIds.push_back(txHash); + historicalSparkTransactions.push_back(tx); + historicalSparkTxIds.push_back(txHash); return true; } - -void BatchProofContainer::remove(const spark::SpendTransaction& tx) { - LOCK(cs_batch); - 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) { - fBatchFailed = false; - } -} diff --git a/src/batchproof_container.h b/src/batchproof_container.h index 638ea8c38c..8837814b07 100644 --- a/src/batchproof_container.h +++ b/src/batchproof_container.h @@ -14,15 +14,16 @@ class BatchProofContainer { void init(bool collectProofs = false); + void abort(); + void finalize(); /** - * Verify the finalized pending Spark batch when proofs are not being - * collected. A no-op while collection is active, so IBD keeps - * accumulating until a recent tip. + * Verify the finalized Spark batch for the current block. A no-op while + * collection is active. * * @return true if collecting, if no batch is pending, or if the batch - * verifies; false on verification failure (pending proofs kept). + * verifies; false on verification failure. */ bool verify_pending(); @@ -31,17 +32,11 @@ class BatchProofContainer { bool add(const spark::SpendTransaction& tx, const uint256& txHash); bool addHistorical(const spark::SpendTransaction& tx, const uint256& txHash); - void remove(const spark::SpendTransaction& tx); private: static std::unique_ptr instance; mutable CCriticalSection cs_batch; bool fCollectProofs = false; - bool fBatchFailed = false; - std::vector tempSparkTransactions; - std::vector tempSparkTxIds; - std::vector tempHistoricalSparkTransactions; - std::vector tempHistoricalSparkTxIds; std::vector sparkTransactions; std::vector sparkTxIds; std::vector historicalSparkTransactions; diff --git a/src/init.cpp b/src/init.cpp index 2c1e4b4f9a..a9a55ca4d8 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -268,13 +268,6 @@ void Shutdown() StopHTTPServer(); llmq::StopLLMQSystem(); - { - LOCK(cs_main); - BatchProofContainer::get_instance()->finalize(); - CValidationState state; - VerifyPendingSparkBatch(state, "shutdown"); - } - #ifdef ENABLE_WALLET if (pwalletMain) pwalletMain->Flush(false); @@ -776,15 +769,6 @@ void ThreadImport(std::vector vImportFiles) { LoadExternalBlockFile(chainparams, file, &pos); nFile++; } - { - LOCK(cs_main); - BatchProofContainer::get_instance()->finalize(); - 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; BatchProofContainer::RemoveRecoveryMarker(); @@ -1972,11 +1956,10 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 7b: load block chain - // If the previous run aborted on a failed Spark batch verification, or - // crashed while Spark proofs were still being collected, drop chainstate - // and verify Spark proofs block by block on this run. Checked here rather - // than in LoadBlockIndexDB() because a run restarted with -reindex wipes - // the block tree database and never calls LoadBlockIndexDB(). + // Honor recovery markers left by versions that allowed Spark batches to + // outlive ConnectBlock. Checked here rather than in LoadBlockIndexDB() + // because a run restarted with -reindex wipes the block tree database and + // never calls LoadBlockIndexDB(). if (BatchProofContainer::HasRecoveryMarker()) { LogPrintf("Previous run did not finish Spark batch verification, disabling -batching and forcing -reindex for this run\n"); ForceSetArg("-batching", "0"); diff --git a/src/test/spark_batch_test.cpp b/src/test/spark_batch_test.cpp index 39901670de..fa7a773ca9 100644 --- a/src/test/spark_batch_test.cpp +++ b/src/test/spark_batch_test.cpp @@ -1,5 +1,6 @@ #include "../batchproof_container.h" #include "../spark/state.h" +#include "../ui_interface.h" #include "../validation.h" #include "../wallet/wallet.h" #include "fixtures.h" @@ -37,16 +38,16 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) // With batching active the spend must be deferred into the container // instead of being verified inline. - auto collectSpend = [&]() { + auto addValidSpend = [&]() { LOCK(cs_main); CValidationState state; spark::CSparkTxInfo info; - container->init(true); BOOST_CHECK(spark::CheckSparkTransaction( spendTx, state, spendTx.GetHash(), false, chainActive.Height(), false, true, &info)); - container->finalize(); }; - collectSpend(); + container->init(true); + addValidSpend(); + container->finalize(); // The pending batch holds a valid proof and verifies successfully. BOOST_CHECK(container->verify_pending()); @@ -55,11 +56,36 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) // A raw-parsed spend lacks the out-coin/cover-set/vout data the binding // hash commits to, so its Chaum proof can never verify: an invalid batch - // member whose serialized lTags still identify it for removal. + // member. spark::SpendTransaction invalidSpend = spark::ParseSparkSpend(spendTxB); invalidSpend.setVout(0); BOOST_REQUIRE(invalidSpend.getUsedLTags() != spark::ParseSparkSpend(spendTx).getUsedLTags()); + // Replacing a finalized batch while its snapshot is being verified must + // not let the old verdict clear or validate the replacement. + container->init(true); + addValidSpend(); + container->finalize(); + bool replacementAdded = false; + bool replaced = false; + boost::signals2::scoped_connection replaceBatch( + uiInterface.UpdateProgressBarLabel.connect( + [&](const std::string&) { + if (replaced) + return; + replaced = true; + container->init(true); + replacementAdded = container->add( + invalidSpend, spendTxB.GetHash()); + container->finalize(); + })); + BOOST_CHECK(container->verify_pending()); + replaceBatch.disconnect(); + BOOST_REQUIRE(replaced); + BOOST_REQUIRE(replacementAdded); + BOOST_CHECK(!container->verify_pending()); + BOOST_CHECK(container->verify_pending()); + // Checking a pending batch while collection is active must not close the // collection window. Otherwise a proof can be skipped without being // enqueued at the old-to-recent batching boundary. @@ -68,37 +94,31 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) BOOST_CHECK(container->add(invalidSpend, spendTxB.GetHash())); container->finalize(); BOOST_CHECK(!container->verify_pending()); - container->remove(invalidSpend); + // A failed block-local batch is consumed, so it cannot poison the next + // block. BOOST_CHECK(container->verify_pending()); - collectSpend(); container->init(true); + addValidSpend(); BOOST_REQUIRE(container->add(invalidSpend, spendTxB.GetHash())); container->finalize(); - // A batch holding a valid and an invalid proof fails and latches. - BOOST_CHECK(!container->verify_pending()); + // A batch holding a valid and an invalid proof fails closed. BOOST_CHECK(!container->verify_pending()); - - // Removing only the offending spend (as a disconnect would) clears the - // latch even though the batch stays non-empty: the remaining valid proof - // must verify again. - container->remove(invalidSpend); BOOST_CHECK(container->verify_pending()); // Re-collect the same spend, then wipe the Spark state so the cover sets // it references can no longer be built: verification must fail closed. - collectSpend(); + container->init(true); + addValidSpend(); + container->finalize(); spark::CSparkState::GetState()->Reset(); BOOST_CHECK(!container->verify_pending()); - // The failed batch is retained and keeps failing. - BOOST_CHECK(!container->verify_pending()); - - // Only removing the offending spend (as a disconnect would) empties the - // batch and lets verification pass again. - container->remove(spark::ParseSparkSpend(spendTx)); + // No new recovery marker is needed because verification now precedes + // block-state publication. BOOST_CHECK(container->verify_pending()); + BOOST_CHECK(!BatchProofContainer::HasRecoveryMarker()); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/spark_tests.cpp b/src/test/spark_tests.cpp index 244bf8007e..19dd88ea87 100644 --- a/src/test/spark_tests.cpp +++ b/src/test/spark_tests.cpp @@ -2,6 +2,7 @@ #include "../batchproof_container.h" #include "../pow.h" #include "../consensus/consensus.h" +#include "../consensus/merkle.h" #include "../script/sign.h" #include "../script/standard.h" #include "../validation.h" @@ -2989,7 +2990,6 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) BOOST_REQUIRE_EQUAL(selectedMints.size(), 2U); const CTransaction multiInputSpend( GenerateCustomSparkSpend(selectedMints, 9 * COIN)); - SpendTransaction parsedMultiInput = ParseSparkSpend(multiInputSpend); constexpr uint64_t groupIdAliasOffset = uint64_t{1} << 32; const CTransaction aliasedSpend(GenerateCustomSparkSpend( {selectedMints.front()}, 4 * COIN, groupIdAliasOffset)); @@ -3017,7 +3017,6 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) &historicalInfo)); batch->finalize(); BOOST_CHECK(batch->verify_pending()); - batch->remove(parsedMultiInput); // Pre-activation blocks retain the deployed 32-bit group ID @@ -3048,7 +3047,6 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) &legacyBatchInfo)); batch->finalize(); BOOST_CHECK(batch->verify_pending()); - batch->remove(parsedAlias); // Exercise the post-single-input batch as well; it has a separate // collection and verification path. @@ -3067,7 +3065,6 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) ¤tBatchInfo)); batch->finalize(); BOOST_CHECK(batch->verify_pending()); - batch->remove(parsedAlias); // Upgraded mempools reject aliases before consensus activation. CValidationState mempoolAliasState; @@ -3153,26 +3150,50 @@ BOOST_AUTO_TEST_CASE(batched_spark_proofs_are_verified_inside_connect_block) CMutableTransaction invalidSpend( GenerateSparkSpend({4 * COIN}, {}, nullptr)); BOOST_REQUIRE(!invalidSpend.vout.empty()); - ++invalidSpend.vout.front().nValue; mempool.clear(); CBlock candidate = CreateBlock({invalidSpend}, script); + ++invalidSpend.vout.front().nValue; + candidate.vtx.back() = MakeTransactionRef(invalidSpend); + candidate.hashMerkleRoot = BlockMerkleRoot(candidate); + if (candidate.IsProgPow()) { + while (!CheckProofOfWork( + progpow_hash_full(candidate.GetProgPowHeader(), candidate.mix_hash), + candidate.nBits, + consensus)) { + ++candidate.nNonce64; + } + } else { + while (!CheckProofOfWork(candidate.GetHash(), candidate.nBits, consensus)) + ++candidate.nNonce; + } + const auto usedLTags = ParseSparkSpend(*candidate.vtx.back()).getUsedLTags(); + BOOST_REQUIRE(!usedLTags.empty()); uint256 candidateHash = candidate.GetHash(); CBlockIndex candidateIndex(candidate); candidateIndex.phashBlock = &candidateHash; candidateIndex.pprev = chainActive.Tip(); candidateIndex.nHeight = chainActive.Height() + 1; - // Use a recent block time so ConnectBlock verifies proofs inline instead - // of deferring them (master IBD batching path). - candidateIndex.nTime = GetSystemTimeInSeconds(); + // Use an old block time so ConnectBlock takes the batching path. + candidateIndex.nTime = GetSystemTimeInSeconds() - 86401; CValidationState state; CCoinsViewCache view(pcoinsTip); { LOCK(cs_main); + for (const auto& lTag : usedLTags) + BOOST_REQUIRE(!sparkState->IsUsedLTag(lTag)); BOOST_CHECK(!ConnectBlock( - candidate, state, &candidateIndex, view, ::Params(), true)); + candidate, state, &candidateIndex, view, ::Params(), false)); + BOOST_CHECK(chainActive.Tip() == candidateIndex.pprev); + BOOST_CHECK( + view.GetBestBlock() == candidateIndex.pprev->GetBlockHash()); + BOOST_CHECK(candidateIndex.GetUndoPos().IsNull()); + BOOST_CHECK(!candidateIndex.IsValid(BLOCK_VALID_SCRIPTS)); + for (const auto& lTag : usedLTags) + BOOST_CHECK(!sparkState->IsUsedLTag(lTag)); } + BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-spark-batch-proof"); // VerifyDB must avoid tip-state mutation without skipping the proof. CValidationState verifyState; @@ -3214,7 +3235,6 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) CMutableTransaction invalidSpend( GenerateSparkSpend({4 * COIN}, {}, nullptr)); BOOST_REQUIRE(!invalidSpend.vout.empty()); - ++invalidSpend.vout.front().nValue; CMutableTransaction missingInput; missingInput.vin.emplace_back(COutPoint(uint256S("01"), 0)); @@ -3222,13 +3242,16 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) mempool.clear(); CBlock candidate = CreateBlock({invalidSpend, missingInput}, script); + ++invalidSpend.vout.front().nValue; + candidate.vtx[candidate.vtx.size() - 2] = MakeTransactionRef(invalidSpend); + candidate.hashMerkleRoot = BlockMerkleRoot(candidate); uint256 candidateHash = candidate.GetHash(); CBlockIndex candidateIndex(candidate); candidateIndex.phashBlock = &candidateHash; candidateIndex.pprev = chainActive.Tip(); candidateIndex.nHeight = chainActive.Height() + 1; - // Old enough to enable deferred batching while ConnectBlock still fails - // on the missing transparent input before finalize. + // Old enough to enable batching while ConnectBlock still fails on the + // missing transparent input before finalization. candidateIndex.nTime = GetSystemTimeInSeconds() - 86401; CValidationState state; @@ -3239,9 +3262,10 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) candidate, state, &candidateIndex, view, ::Params(), true)); } - // Master deferred batching does not abort() on ConnectBlock failure; the - // next ConnectBlock init() (or an explicit init here) drops temps. - batch->init(); + // Scope cleanup must discard the partially collected proof. Closing a + // leftover collection window would otherwise expose it here. + BOOST_CHECK(batch->verify_pending()); + batch->finalize(); BOOST_CHECK(batch->verify_pending()); mempool.clear(); diff --git a/src/validation.cpp b/src/validation.cpp index 11f405c9f8..5c18615d9d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2309,20 +2309,10 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std static bool ShouldBatchSparkProofs(const CBlockIndex* pindex) { - // Defer Spark proof verification for blocks older than a day, which means we are syncing or reindexing + // Batch Spark proofs in old blocks while syncing or reindexing. return ((GetSystemTimeInSeconds() - pindex->GetBlockTime()) > 86400) && GetBoolArg("-batching", true); } -bool VerifyPendingSparkBatch(CValidationState& state, const std::string& reason) -{ - if (!BatchProofContainer::get_instance()->verify_pending()) { - return AbortNode(state, - strprintf("Spark batch verification failed before %s", reason), - _("Spark batch verification failed. The invalid spend transactions are listed in debug.log. Restart the node: batching is disabled and a reindex is started automatically so chainstate is rebuilt and Spark proofs are checked block by block.")); - } - return true; -} - enum DisconnectResult { DISCONNECT_OK, // All good. @@ -2797,9 +2787,17 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::set txIds; bool isMainNet = chainparams.GetConsensus().IsMain(); - // batch verify Lelantus/Sigma if block is older than a day, that means we are syncing or reindexing + // Batch Spark proofs within old blocks while syncing or reindexing. BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); batchProofContainer->init(ShouldBatchSparkProofs(pindex)); + struct BatchProofCleanup + { + BatchProofContainer* container; + ~BatchProofCleanup() + { + container->abort(); + } + } batchProofCleanup{batchProofContainer}; std::size_t nSigma = 0; std::size_t nLelantus = 0; @@ -2990,6 +2988,25 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } } + // Batch within this block, but verify under cs_main before any global or + // persistent block state is updated. This bounds the lock hold to one + // block instead of one accumulated IBD batch. + batchProofContainer->finalize(); + bool batchVerified = false; + try { + batchVerified = batchProofContainer->verify_pending(); + } catch (const std::bad_alloc&) { + return state.Error( + "ConnectBlock(): memory allocation failed during Spark batch verification"); + } + if (!batchVerified) { + return state.DoS( + 100, + error("ConnectBlock(): Spark batch proof verification failed"), + REJECT_INVALID, + "bad-spark-batch-proof"); + } + if (!ProcessSpecialTxsInBlock(block, pindex, state, isVerifyDB ? false : fJustCheck, fScriptChecks, !isVerifyDB)) { return error("ConnectBlock(): ProcessSpecialTxsInBlock for block %s at height %i failed with %s", pindex->GetBlockHash().ToString(), pindex->nHeight, FormatStateMessage(state)); @@ -3114,9 +3131,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // add this block to the view's block chain view.SetBestBlock(pindex->GetBlockHash()); - // do batch verification if remains a day or collect proofs - batchProofContainer->finalize(); - int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4; LogPrint("bench", " - Index writing: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeIndex * 0.000001); @@ -3401,26 +3415,6 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara return AbortNode(state, "Failed to read block"); - // retrieve all mints - block.sparkTxInfo = std::make_shared(); - - std::vector sparkTransactionsToRemove; - for (CTransactionRef tx : block.vtx) { - CheckTransaction(*tx, state, false, tx->GetHash(), false, pindexDelete->pprev->nHeight, - false, false, block.sparkTxInfo.get()); - if(GetBoolArg("-batching", true)) { - if (tx->IsSparkSpend()) { - try { - spark::SpendTransaction spendTransaction = spark::ParseSparkSpend(*tx); - sparkTransactionsToRemove.push_back(spendTransaction); - } - catch (CBadTxIn &) { - continue; - } - } - } - } - // Apply the block atomically to the chain state. int64_t nStart = GetTimeMicros(); { @@ -3437,12 +3431,6 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara spark::DisconnectTipSpark(block, pindexDelete); - BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - - for (auto& sparkTransaction : sparkTransactionsToRemove) { - batchProofContainer->remove(sparkTransaction); - } - // Roll back MTP state MTPState::GetMTPState()->SetLastBlock(pindexDelete->pprev, chainparams.GetConsensus()); @@ -3989,10 +3977,6 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } } - if (!ShouldBatchSparkProofs(pindexNewTip) && - !VerifyPendingSparkBatch(state, "connecting new tip")) - return false; - // When we reach this point, we switched to a new tip (stored in pindexNewTip). // Notifications/callbacks that can run without cs_main diff --git a/src/validation.h b/src/validation.h index a0ffae67db..30b8ef2fb1 100644 --- a/src/validation.h +++ b/src/validation.h @@ -303,12 +303,6 @@ 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 pblock = std::shared_ptr()); -/** - * Verify the pending Spark batch when proofs are not being collected. - * On failure the node is aborted, a datadir marker is written so the next - * start disables batching and reindexes, and false is returned (no throw). - */ -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); From 8b24379409ba426eb739f3bfa53c15009b9293d4 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Tue, 1 Sep 2026 15:47:37 +0800 Subject: [PATCH 06/12] Trivial: fix batch proof style --- src/batchproof_container.cpp | 30 +++++++++++++++++++----------- src/spark/state.cpp | 2 +- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index e5ea1b6720..e023804f50 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -7,7 +7,8 @@ #include #include -namespace { +namespace +{ bool VerifySparkBatch( const std::vector& sparkTransactions, @@ -34,7 +35,7 @@ bool VerifySparkBatch( passed = spark::SpendTransaction::verifyHistorical( params, historicalSparkTransactions, coverSets); } - } catch (const std::bad_alloc &) { + } catch (const std::bad_alloc&) { throw; } catch (const std::exception &) { passed = false; @@ -48,7 +49,7 @@ bool VerifySparkBatch( try { fProofValid = spark::SpendTransaction::verify( params, {sparkTransactions[i]}, coverSets); - } catch (const std::bad_alloc &) { + } catch (const std::bad_alloc&) { throw; } catch (const std::exception &) { fProofValid = false; @@ -62,7 +63,7 @@ bool VerifySparkBatch( try { fProofValid = spark::SpendTransaction::verifyHistorical( params, {historicalSparkTransactions[i]}, coverSets); - } catch (const std::bad_alloc &) { + } catch (const std::bad_alloc&) { throw; } catch (const std::exception &) { fProofValid = false; @@ -98,7 +99,8 @@ void BatchProofContainer::RemoveRecoveryMarker() boost::filesystem::remove(RecoveryMarkerPath()); } -BatchProofContainer* BatchProofContainer::get_instance() { +BatchProofContainer* BatchProofContainer::get_instance() +{ if (instance) { return instance.get(); } else { @@ -107,7 +109,8 @@ BatchProofContainer* BatchProofContainer::get_instance() { } } -void BatchProofContainer::init(bool collectProofs) { +void BatchProofContainer::init(bool collectProofs) +{ LOCK(cs_batch); sparkTransactions.clear(); sparkTxIds.clear(); @@ -116,16 +119,19 @@ void BatchProofContainer::init(bool collectProofs) { fCollectProofs = collectProofs; } -void BatchProofContainer::abort() { +void BatchProofContainer::abort() +{ init(); } -void BatchProofContainer::finalize() { +void BatchProofContainer::finalize() +{ LOCK(cs_batch); fCollectProofs = false; } -bool BatchProofContainer::verify_pending() { +bool BatchProofContainer::verify_pending() +{ std::vector snapshotTransactions; std::vector snapshotTxIds; std::vector snapshotHistoricalTransactions; @@ -166,7 +172,8 @@ bool BatchProofContainer::verify_pending() { coverSets); } -bool BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) { +bool BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) +{ LOCK(cs_batch); if (!fCollectProofs) return false; @@ -176,7 +183,8 @@ bool BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& } bool BatchProofContainer::addHistorical( - const spark::SpendTransaction& tx, const uint256& txHash) { + const spark::SpendTransaction& tx, const uint256& txHash) +{ LOCK(cs_batch); if (!fCollectProofs) return false; diff --git a/src/spark/state.cpp b/src/spark/state.cpp index 82c1fc88a5..bff7af27eb 100644 --- a/src/spark/state.cpp +++ b/src/spark/state.cpp @@ -1922,7 +1922,7 @@ void CSparkState::GetCoinSet( std::vector& coins_out) { uint256 blockHash; std::vector setHash; - FIRO_UNUSED const auto ¶ms = ::Params().GetConsensus(); + FIRO_UNUSED const auto& params = ::Params().GetConsensus(); LOCK(cs_main); int maxHeight = chainActive.Height() - (ZC_MINT_CONFIRMATIONS - 1); GetCoinSetForSpend( From 59019f077677f580d4eca663f6a7d6f3befecdc6 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Tue, 1 Sep 2026 17:06:48 +0800 Subject: [PATCH 07/12] Trivial: prefix Spark batch flags --- src/spark/state.cpp | 10 +++++----- src/test/spark_batch_test.cpp | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/spark/state.cpp b/src/spark/state.cpp index bff7af27eb..7ad06c1ab6 100644 --- a/src/spark/state.cpp +++ b/src/spark/state.cpp @@ -1066,7 +1066,7 @@ bool CheckSparkSpendTransaction( std::unordered_map cover_set_data; BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - bool useBatching = !isVerifyDB && !isCheckWallet && sparkTxInfo && !sparkTxInfo->fInfoIsComplete; + bool fUseBatching = !isVerifyDB && !isCheckWallet && sparkTxInfo && !sparkTxInfo->fInfoIsComplete; for (const auto& idAndHash : idAndBlockHashes) { const uint64_t wireGroupId = idAndHash.first; @@ -1214,13 +1214,13 @@ bool CheckSparkSpendTransaction( // if we are collecting proofs, skip verification and collect proofs // add proofs into container - bool addedToBatch = false; - if (useBatching) { - addedToBatch = isChaumV2 || requireChaumV1SingleInput + bool fAddedToBatch = false; + if (fUseBatching) { + fAddedToBatch = isChaumV2 || requireChaumV1SingleInput ? batchProofContainer->add(*spend, hashTx) : batchProofContainer->addHistorical(*spend, hashTx); } - if (addedToBatch) { + if (fAddedToBatch) { passVerify = true; } else { try { diff --git a/src/test/spark_batch_test.cpp b/src/test/spark_batch_test.cpp index fa7a773ca9..197fca1bb7 100644 --- a/src/test/spark_batch_test.cpp +++ b/src/test/spark_batch_test.cpp @@ -66,23 +66,23 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) container->init(true); addValidSpend(); container->finalize(); - bool replacementAdded = false; - bool replaced = false; + bool fReplacementAdded = false; + bool fReplaced = false; boost::signals2::scoped_connection replaceBatch( uiInterface.UpdateProgressBarLabel.connect( [&](const std::string&) { - if (replaced) + if (fReplaced) return; - replaced = true; + fReplaced = true; container->init(true); - replacementAdded = container->add( + fReplacementAdded = container->add( invalidSpend, spendTxB.GetHash()); container->finalize(); })); BOOST_CHECK(container->verify_pending()); replaceBatch.disconnect(); - BOOST_REQUIRE(replaced); - BOOST_REQUIRE(replacementAdded); + BOOST_REQUIRE(fReplaced); + BOOST_REQUIRE(fReplacementAdded); BOOST_CHECK(!container->verify_pending()); BOOST_CHECK(container->verify_pending()); From 1ea640ea9cd6db54778528b98ecb760b875f7eb7 Mon Sep 17 00:00:00 2001 From: levonpetrosyan93 Date: Tue, 1 Sep 2026 13:23:58 +0400 Subject: [PATCH 08/12] Bringing back per-state batching with minor improvements --- qa/rpc-tests/spark_batching.py | 12 +- src/batchproof_container.cpp | 219 ++++++++++++++++++++++++--------- src/batchproof_container.h | 18 ++- src/init.cpp | 27 +++- src/test/spark_batch_test.cpp | 46 ++++--- src/test/spark_tests.cpp | 52 ++------ src/validation.cpp | 77 ++++++++---- src/validation.h | 6 + 8 files changed, 304 insertions(+), 153 deletions(-) diff --git a/qa/rpc-tests/spark_batching.py b/qa/rpc-tests/spark_batching.py index a3189a5f11..8367b200ba 100755 --- a/qa/rpc-tests/spark_batching.py +++ b/qa/rpc-tests/spark_batching.py @@ -1,10 +1,10 @@ #!/usr/bin/env python3 -"""Test per-block Spark batch proof verification (-batching) across reindex. +"""Test deferred 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 and verified within each block before its validation state is -persisted. +collected into the batch container and must batch-verify before the node +persists validation state and clears the reindex flag. """ import os import time @@ -38,7 +38,9 @@ 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) - # Require proof that the reindex exercised the batching path. + # 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. deadline = time.time() + 300 while True: if self.nodes[0].getblockcount() >= blockcount and \ @@ -64,7 +66,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 batches Spark proof verification. + # whole chain as old blocks and defers Spark proof verification. set_node_times(self.nodes, int(time.time()) - 2 * 86400) self.nodes[0].generate(501) diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index e023804f50..662418e62a 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -4,11 +4,13 @@ #include "util.h" #include +#include #include #include -namespace -{ +extern bool fReindex; + +namespace { bool VerifySparkBatch( const std::vector& sparkTransactions, @@ -42,7 +44,7 @@ bool VerifySparkBatch( } if (!passed) { - // Re-verify the batch members individually so the operator can see + // Re-verify the retained proofs 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; @@ -89,6 +91,18 @@ static boost::filesystem::path RecoveryMarkerPath() return GetDataDir() / "sparkbatchfailed"; } +static void WriteRecoveryMarker() +{ + 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() { return boost::filesystem::exists(RecoveryMarkerPath()); @@ -99,8 +113,7 @@ void BatchProofContainer::RemoveRecoveryMarker() boost::filesystem::remove(RecoveryMarkerPath()); } -BatchProofContainer* BatchProofContainer::get_instance() -{ +BatchProofContainer* BatchProofContainer::get_instance() { if (instance) { return instance.get(); } else { @@ -109,86 +122,178 @@ BatchProofContainer* BatchProofContainer::get_instance() } } -void BatchProofContainer::init(bool collectProofs) -{ +void BatchProofContainer::init(bool collectProofs) { LOCK(cs_batch); - sparkTransactions.clear(); - sparkTxIds.clear(); - historicalSparkTransactions.clear(); - historicalSparkTxIds.clear(); + tempSparkTransactions.clear(); + tempSparkTxIds.clear(); + tempHistoricalSparkTransactions.clear(); + tempHistoricalSparkTxIds.clear(); fCollectProofs = collectProofs; + if (fCollectProofs) + WriteRecoveryMarker(); } -void BatchProofContainer::abort() -{ - init(); +void BatchProofContainer::finalize() { + LOCK(cs_batch); + 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; } -void BatchProofContainer::finalize() +void BatchProofContainer::discard_temps() { LOCK(cs_batch); + tempSparkTransactions.clear(); + tempSparkTxIds.clear(); + tempHistoricalSparkTransactions.clear(); + tempHistoricalSparkTxIds.clear(); fCollectProofs = false; } -bool BatchProofContainer::verify_pending() -{ - std::vector snapshotTransactions; - std::vector snapshotTxIds; - std::vector snapshotHistoricalTransactions; - std::vector snapshotHistoricalTxIds; +bool BatchProofContainer::verify_pending() { { LOCK(cs_batch); - if (fCollectProofs) + if (fCollectProofs) { return true; - - snapshotTransactions.swap(sparkTransactions); - snapshotTxIds.swap(sparkTxIds); - snapshotHistoricalTransactions.swap(historicalSparkTransactions); - snapshotHistoricalTxIds.swap(historicalSparkTxIds); + } } - std::set 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> coverSets; - spark::CSparkState* sparkState = spark::CSparkState::GetState(); - for (uint64_t id : coverSetIds) { - std::vector coins; - sparkState->GetCoinSet(static_cast(id), coins); - coverSets.emplace(id, std::move(coins)); - } + while (true) { + std::vector snapshotTransactions; + std::vector snapshotTxIds; + std::vector snapshotHistoricalTransactions; + std::vector snapshotHistoricalTxIds; + { + LOCK(cs_batch); + init(); + if (fBatchFailed) { + fCollectProofs = false; + return false; + } + if (sparkTransactions.empty() && historicalSparkTransactions.empty()) { + fCollectProofs = false; + return true; + } + + snapshotTransactions.swap(sparkTransactions); + snapshotTxIds.swap(sparkTxIds); + snapshotHistoricalTransactions.swap(historicalSparkTransactions); + snapshotHistoricalTxIds.swap(historicalSparkTxIds); + } + + std::set 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> coverSets; + spark::CSparkState* sparkState = spark::CSparkState::GetState(); + for (uint64_t id : coverSetIds) { + std::vector coins; + sparkState->GetCoinSet(static_cast(id), coins); + coverSets.emplace(id, std::move(coins)); + } - return VerifySparkBatch( - snapshotTransactions, - snapshotTxIds, - snapshotHistoricalTransactions, - snapshotHistoricalTxIds, - coverSets); + const bool passed = VerifySparkBatch( + snapshotTransactions, + snapshotTxIds, + snapshotHistoricalTransactions, + snapshotHistoricalTxIds, + coverSets); + + LOCK(cs_batch); + if (!sparkTransactions.empty() || !historicalSparkTransactions.empty()) { + sparkTransactions.insert( + sparkTransactions.end(), + std::make_move_iterator(snapshotTransactions.begin()), + std::make_move_iterator(snapshotTransactions.end())); + sparkTxIds.insert( + sparkTxIds.end(), + std::make_move_iterator(snapshotTxIds.begin()), + std::make_move_iterator(snapshotTxIds.end())); + historicalSparkTransactions.insert( + historicalSparkTransactions.end(), + std::make_move_iterator(snapshotHistoricalTransactions.begin()), + std::make_move_iterator(snapshotHistoricalTransactions.end())); + historicalSparkTxIds.insert( + historicalSparkTxIds.end(), + std::make_move_iterator(snapshotHistoricalTxIds.begin()), + std::make_move_iterator(snapshotHistoricalTxIds.end())); + continue; + } + + fCollectProofs = false; + if (passed) { + if (!fReindex) + RemoveRecoveryMarker(); + return true; + } + + sparkTransactions.swap(snapshotTransactions); + sparkTxIds.swap(snapshotTxIds); + historicalSparkTransactions.swap(snapshotHistoricalTransactions); + historicalSparkTxIds.swap(snapshotHistoricalTxIds); + WriteRecoveryMarker(); + fBatchFailed = true; + return false; + } } -bool BatchProofContainer::add(const spark::SpendTransaction& tx, const uint256& txHash) -{ +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); + tempSparkTransactions.push_back(tx); + tempSparkTxIds.push_back(txHash); return true; } bool BatchProofContainer::addHistorical( - const spark::SpendTransaction& tx, const uint256& txHash) -{ + const spark::SpendTransaction& tx, const uint256& txHash) { LOCK(cs_batch); if (!fCollectProofs) return false; - historicalSparkTransactions.push_back(tx); - historicalSparkTxIds.push_back(txHash); + tempHistoricalSparkTransactions.push_back(tx); + tempHistoricalSparkTxIds.push_back(txHash); return true; } + +void BatchProofContainer::remove(const spark::SpendTransaction& tx) { + LOCK(cs_batch); + 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) { + fBatchFailed = false; + } +} diff --git a/src/batchproof_container.h b/src/batchproof_container.h index 8837814b07..a6f3b9797f 100644 --- a/src/batchproof_container.h +++ b/src/batchproof_container.h @@ -14,16 +14,18 @@ class BatchProofContainer { void init(bool collectProofs = false); - void abort(); - void finalize(); + /** Drop in-flight per-block temps without merging into the deferred batch. */ + void discard_temps(); + /** - * Verify the finalized Spark batch for the current block. A no-op while - * collection is active. + * Verify the finalized pending Spark batch when proofs are not being + * collected. A no-op while collection is active, so IBD keeps + * accumulating until a recent tip. * * @return true if collecting, if no batch is pending, or if the batch - * verifies; false on verification failure. + * verifies; false on verification failure (pending proofs kept). */ bool verify_pending(); @@ -32,11 +34,17 @@ class BatchProofContainer { bool add(const spark::SpendTransaction& tx, const uint256& txHash); bool addHistorical(const spark::SpendTransaction& tx, const uint256& txHash); + void remove(const spark::SpendTransaction& tx); private: static std::unique_ptr instance; mutable CCriticalSection cs_batch; bool fCollectProofs = false; + bool fBatchFailed = false; + std::vector tempSparkTransactions; + std::vector tempSparkTxIds; + std::vector tempHistoricalSparkTransactions; + std::vector tempHistoricalSparkTxIds; std::vector sparkTransactions; std::vector sparkTxIds; std::vector historicalSparkTransactions; diff --git a/src/init.cpp b/src/init.cpp index a9a55ca4d8..d2e65827b0 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -295,6 +295,14 @@ void Shutdown() // cleanup; the embedded Tor itself is torn down by process exit. g_connman.reset(); UnregisterNodeSignals(GetNodeSignals()); + + { + LOCK(cs_main); + BatchProofContainer::get_instance()->finalize(); + } + CValidationState state; + VerifyPendingSparkBatch(state, "shutdown"); + if (fDumpMempoolLater) DumpMempool(); @@ -769,6 +777,15 @@ void ThreadImport(std::vector vImportFiles) { LoadExternalBlockFile(chainparams, file, &pos); nFile++; } + { + LOCK(cs_main); + BatchProofContainer::get_instance()->finalize(); + } + 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; BatchProofContainer::RemoveRecoveryMarker(); @@ -1956,10 +1973,12 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 7b: load block chain - // Honor recovery markers left by versions that allowed Spark batches to - // outlive ConnectBlock. Checked here rather than in LoadBlockIndexDB() - // because a run restarted with -reindex wipes the block tree database and - // never calls LoadBlockIndexDB(). + // Deferred Spark batching writes sparkbatchfailed when collection starts and + // removes it after a successful verify. A failed batch verify or crash while + // proofs are still pending leaves the marker; force -reindex with -batching=0 + // so chainstate is rebuilt with per-block verification. Checked here rather + // than in LoadBlockIndexDB() because a run restarted with -reindex wipes the + // block tree database and never calls LoadBlockIndexDB(). if (BatchProofContainer::HasRecoveryMarker()) { LogPrintf("Previous run did not finish Spark batch verification, disabling -batching and forcing -reindex for this run\n"); ForceSetArg("-batching", "0"); diff --git a/src/test/spark_batch_test.cpp b/src/test/spark_batch_test.cpp index 197fca1bb7..3f9cef5b3f 100644 --- a/src/test/spark_batch_test.cpp +++ b/src/test/spark_batch_test.cpp @@ -45,9 +45,12 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) BOOST_CHECK(spark::CheckSparkTransaction( spendTx, state, spendTx.GetHash(), false, chainActive.Height(), false, true, &info)); }; - container->init(true); - addValidSpend(); - container->finalize(); + auto collectSpend = [&]() { + container->init(true); + addValidSpend(); + container->finalize(); + }; + collectSpend(); // The pending batch holds a valid proof and verifies successfully. BOOST_CHECK(container->verify_pending()); @@ -56,13 +59,14 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) // A raw-parsed spend lacks the out-coin/cover-set/vout data the binding // hash commits to, so its Chaum proof can never verify: an invalid batch - // member. + // member whose serialized lTags still identify it for removal. spark::SpendTransaction invalidSpend = spark::ParseSparkSpend(spendTxB); invalidSpend.setVout(0); BOOST_REQUIRE(invalidSpend.getUsedLTags() != spark::ParseSparkSpend(spendTx).getUsedLTags()); - // Replacing a finalized batch while its snapshot is being verified must - // not let the old verdict clear or validate the replacement. + // Replacing the deferred batch while its snapshot is being verified must + // retry and fail on the replacement, not clear or validate it from the + // old snapshot's verdict. container->init(true); addValidSpend(); container->finalize(); @@ -79,11 +83,11 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) invalidSpend, spendTxB.GetHash()); container->finalize(); })); - BOOST_CHECK(container->verify_pending()); + BOOST_CHECK(!container->verify_pending()); replaceBatch.disconnect(); BOOST_REQUIRE(fReplaced); BOOST_REQUIRE(fReplacementAdded); - BOOST_CHECK(!container->verify_pending()); + container->remove(invalidSpend); BOOST_CHECK(container->verify_pending()); // Checking a pending batch while collection is active must not close the @@ -94,31 +98,37 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) BOOST_CHECK(container->add(invalidSpend, spendTxB.GetHash())); container->finalize(); BOOST_CHECK(!container->verify_pending()); - // A failed block-local batch is consumed, so it cannot poison the next - // block. + container->remove(invalidSpend); BOOST_CHECK(container->verify_pending()); + collectSpend(); container->init(true); - addValidSpend(); BOOST_REQUIRE(container->add(invalidSpend, spendTxB.GetHash())); container->finalize(); - // A batch holding a valid and an invalid proof fails closed. + // A batch holding a valid and an invalid proof fails and latches. BOOST_CHECK(!container->verify_pending()); + BOOST_CHECK(!container->verify_pending()); + + // Removing only the offending spend (as a disconnect would) clears the + // latch even though the batch stays non-empty: the remaining valid proof + // must verify again. + container->remove(invalidSpend); BOOST_CHECK(container->verify_pending()); // Re-collect the same spend, then wipe the Spark state so the cover sets // it references can no longer be built: verification must fail closed. - container->init(true); - addValidSpend(); - container->finalize(); + collectSpend(); spark::CSparkState::GetState()->Reset(); BOOST_CHECK(!container->verify_pending()); - // No new recovery marker is needed because verification now precedes - // block-state publication. + // The failed batch is retained and keeps failing. + BOOST_CHECK(!container->verify_pending()); + + // Only removing the offending spend (as a disconnect would) empties the + // batch and lets verification pass again. + container->remove(spark::ParseSparkSpend(spendTx)); BOOST_CHECK(container->verify_pending()); - BOOST_CHECK(!BatchProofContainer::HasRecoveryMarker()); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/spark_tests.cpp b/src/test/spark_tests.cpp index 19dd88ea87..6ed108355d 100644 --- a/src/test/spark_tests.cpp +++ b/src/test/spark_tests.cpp @@ -2,7 +2,6 @@ #include "../batchproof_container.h" #include "../pow.h" #include "../consensus/consensus.h" -#include "../consensus/merkle.h" #include "../script/sign.h" #include "../script/standard.h" #include "../validation.h" @@ -2990,6 +2989,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) BOOST_REQUIRE_EQUAL(selectedMints.size(), 2U); const CTransaction multiInputSpend( GenerateCustomSparkSpend(selectedMints, 9 * COIN)); + SpendTransaction parsedMultiInput = ParseSparkSpend(multiInputSpend); constexpr uint64_t groupIdAliasOffset = uint64_t{1} << 32; const CTransaction aliasedSpend(GenerateCustomSparkSpend( {selectedMints.front()}, 4 * COIN, groupIdAliasOffset)); @@ -3017,6 +3017,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) &historicalInfo)); batch->finalize(); BOOST_CHECK(batch->verify_pending()); + batch->remove(parsedMultiInput); // Pre-activation blocks retain the deployed 32-bit group ID @@ -3047,6 +3048,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) &legacyBatchInfo)); batch->finalize(); BOOST_CHECK(batch->verify_pending()); + batch->remove(parsedAlias); // Exercise the post-single-input batch as well; it has a separate // collection and verification path. @@ -3065,6 +3067,7 @@ BOOST_AUTO_TEST_CASE(spark_single_input_historical_batch_verification) ¤tBatchInfo)); batch->finalize(); BOOST_CHECK(batch->verify_pending()); + batch->remove(parsedAlias); // Upgraded mempools reject aliases before consensus activation. CValidationState mempoolAliasState; @@ -3150,50 +3153,26 @@ BOOST_AUTO_TEST_CASE(batched_spark_proofs_are_verified_inside_connect_block) CMutableTransaction invalidSpend( GenerateSparkSpend({4 * COIN}, {}, nullptr)); BOOST_REQUIRE(!invalidSpend.vout.empty()); + ++invalidSpend.vout.front().nValue; mempool.clear(); CBlock candidate = CreateBlock({invalidSpend}, script); - ++invalidSpend.vout.front().nValue; - candidate.vtx.back() = MakeTransactionRef(invalidSpend); - candidate.hashMerkleRoot = BlockMerkleRoot(candidate); - if (candidate.IsProgPow()) { - while (!CheckProofOfWork( - progpow_hash_full(candidate.GetProgPowHeader(), candidate.mix_hash), - candidate.nBits, - consensus)) { - ++candidate.nNonce64; - } - } else { - while (!CheckProofOfWork(candidate.GetHash(), candidate.nBits, consensus)) - ++candidate.nNonce; - } - const auto usedLTags = ParseSparkSpend(*candidate.vtx.back()).getUsedLTags(); - BOOST_REQUIRE(!usedLTags.empty()); uint256 candidateHash = candidate.GetHash(); CBlockIndex candidateIndex(candidate); candidateIndex.phashBlock = &candidateHash; candidateIndex.pprev = chainActive.Tip(); candidateIndex.nHeight = chainActive.Height() + 1; - // Use an old block time so ConnectBlock takes the batching path. - candidateIndex.nTime = GetSystemTimeInSeconds() - 86401; + // Use a recent block time so ConnectBlock verifies proofs inline instead + // of deferring them (master IBD batching path). + candidateIndex.nTime = GetSystemTimeInSeconds(); CValidationState state; CCoinsViewCache view(pcoinsTip); { LOCK(cs_main); - for (const auto& lTag : usedLTags) - BOOST_REQUIRE(!sparkState->IsUsedLTag(lTag)); BOOST_CHECK(!ConnectBlock( - candidate, state, &candidateIndex, view, ::Params(), false)); - BOOST_CHECK(chainActive.Tip() == candidateIndex.pprev); - BOOST_CHECK( - view.GetBestBlock() == candidateIndex.pprev->GetBlockHash()); - BOOST_CHECK(candidateIndex.GetUndoPos().IsNull()); - BOOST_CHECK(!candidateIndex.IsValid(BLOCK_VALID_SCRIPTS)); - for (const auto& lTag : usedLTags) - BOOST_CHECK(!sparkState->IsUsedLTag(lTag)); + candidate, state, &candidateIndex, view, ::Params(), true)); } - BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-spark-batch-proof"); // VerifyDB must avoid tip-state mutation without skipping the proof. CValidationState verifyState; @@ -3235,6 +3214,7 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) CMutableTransaction invalidSpend( GenerateSparkSpend({4 * COIN}, {}, nullptr)); BOOST_REQUIRE(!invalidSpend.vout.empty()); + ++invalidSpend.vout.front().nValue; CMutableTransaction missingInput; missingInput.vin.emplace_back(COutPoint(uint256S("01"), 0)); @@ -3242,16 +3222,13 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) mempool.clear(); CBlock candidate = CreateBlock({invalidSpend, missingInput}, script); - ++invalidSpend.vout.front().nValue; - candidate.vtx[candidate.vtx.size() - 2] = MakeTransactionRef(invalidSpend); - candidate.hashMerkleRoot = BlockMerkleRoot(candidate); uint256 candidateHash = candidate.GetHash(); CBlockIndex candidateIndex(candidate); candidateIndex.phashBlock = &candidateHash; candidateIndex.pprev = chainActive.Tip(); candidateIndex.nHeight = chainActive.Height() + 1; - // Old enough to enable batching while ConnectBlock still fails on the - // missing transparent input before finalization. + // Old enough to enable deferred batching while ConnectBlock still fails + // on the missing transparent input before finalize. candidateIndex.nTime = GetSystemTimeInSeconds() - 86401; CValidationState state; @@ -3262,10 +3239,7 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) candidate, state, &candidateIndex, view, ::Params(), true)); } - // Scope cleanup must discard the partially collected proof. Closing a - // leftover collection window would otherwise expose it here. - BOOST_CHECK(batch->verify_pending()); - batch->finalize(); + // Failed ConnectBlock must not leave per-block temps for shutdown finalize. BOOST_CHECK(batch->verify_pending()); mempool.clear(); diff --git a/src/validation.cpp b/src/validation.cpp index 5c18615d9d..819ee6d34e 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2309,10 +2309,20 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std static bool ShouldBatchSparkProofs(const CBlockIndex* pindex) { - // Batch Spark proofs in old blocks while syncing or reindexing. + // Defer Spark proof verification for blocks older than a day, which means we are syncing or reindexing return ((GetSystemTimeInSeconds() - pindex->GetBlockTime()) > 86400) && GetBoolArg("-batching", true); } +bool VerifyPendingSparkBatch(CValidationState& state, const std::string& reason) +{ + if (!BatchProofContainer::get_instance()->verify_pending()) { + return AbortNode(state, + strprintf("Spark batch verification failed before %s", reason), + _("Spark batch verification failed. The invalid spend transactions are listed in debug.log. Restart the node: batching is disabled and a reindex is started automatically so chainstate is rebuilt and Spark proofs are checked block by block.")); + } + return true; +} + enum DisconnectResult { DISCONNECT_OK, // All good. @@ -2787,17 +2797,19 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::set txIds; bool isMainNet = chainparams.GetConsensus().IsMain(); - // Batch Spark proofs within old blocks while syncing or reindexing. + // Defer Spark proof verification for blocks older than a day while syncing or reindexing. BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); batchProofContainer->init(ShouldBatchSparkProofs(pindex)); - struct BatchProofCleanup + struct BatchTempCleanup { BatchProofContainer* container; - ~BatchProofCleanup() + bool merged = false; + ~BatchTempCleanup() { - container->abort(); + if (!merged) + container->discard_temps(); } - } batchProofCleanup{batchProofContainer}; + } batchTempCleanup{batchProofContainer}; std::size_t nSigma = 0; std::size_t nLelantus = 0; @@ -2988,25 +3000,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } } - // Batch within this block, but verify under cs_main before any global or - // persistent block state is updated. This bounds the lock hold to one - // block instead of one accumulated IBD batch. - batchProofContainer->finalize(); - bool batchVerified = false; - try { - batchVerified = batchProofContainer->verify_pending(); - } catch (const std::bad_alloc&) { - return state.Error( - "ConnectBlock(): memory allocation failed during Spark batch verification"); - } - if (!batchVerified) { - return state.DoS( - 100, - error("ConnectBlock(): Spark batch proof verification failed"), - REJECT_INVALID, - "bad-spark-batch-proof"); - } - if (!ProcessSpecialTxsInBlock(block, pindex, state, isVerifyDB ? false : fJustCheck, fScriptChecks, !isVerifyDB)) { return error("ConnectBlock(): ProcessSpecialTxsInBlock for block %s at height %i failed with %s", pindex->GetBlockHash().ToString(), pindex->nHeight, FormatStateMessage(state)); @@ -3131,6 +3124,10 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin // add this block to the view's block chain view.SetBestBlock(pindex->GetBlockHash()); + // Merge this block's collected Spark proofs into the deferred batch. + batchProofContainer->finalize(); + batchTempCleanup.merged = true; + int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4; LogPrint("bench", " - Index writing: %.2fms [%.2fs]\n", 0.001 * (nTime5 - nTime4), nTimeIndex * 0.000001); @@ -3415,6 +3412,26 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara return AbortNode(state, "Failed to read block"); + // retrieve all mints + block.sparkTxInfo = std::make_shared(); + + std::vector sparkTransactionsToRemove; + for (CTransactionRef tx : block.vtx) { + CheckTransaction(*tx, state, false, tx->GetHash(), false, pindexDelete->pprev->nHeight, + false, false, block.sparkTxInfo.get()); + if(GetBoolArg("-batching", true)) { + if (tx->IsSparkSpend()) { + try { + spark::SpendTransaction spendTransaction = spark::ParseSparkSpend(*tx); + sparkTransactionsToRemove.push_back(spendTransaction); + } + catch (CBadTxIn &) { + continue; + } + } + } + } + // Apply the block atomically to the chain state. int64_t nStart = GetTimeMicros(); { @@ -3431,6 +3448,12 @@ bool static DisconnectTip(CValidationState& state, const CChainParams& chainpara spark::DisconnectTipSpark(block, pindexDelete); + BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); + + for (auto& sparkTransaction : sparkTransactionsToRemove) { + batchProofContainer->remove(sparkTransaction); + } + // Roll back MTP state MTPState::GetMTPState()->SetLastBlock(pindexDelete->pprev, chainparams.GetConsensus()); @@ -3977,6 +4000,10 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } } + if (!ShouldBatchSparkProofs(pindexNewTip) && + !VerifyPendingSparkBatch(state, "connecting new tip")) + return false; + // When we reach this point, we switched to a new tip (stored in pindexNewTip). // Notifications/callbacks that can run without cs_main diff --git a/src/validation.h b/src/validation.h index 30b8ef2fb1..a0ffae67db 100644 --- a/src/validation.h +++ b/src/validation.h @@ -303,6 +303,12 @@ 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 pblock = std::shared_ptr()); +/** + * Verify the pending Spark batch when proofs are not being collected. + * On failure the node is aborted, a datadir marker is written so the next + * start disables batching and reindexes, and false is returned (no throw). + */ +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); From 36eb5af32828fcd42731ea9fa17371bdbf16681e Mon Sep 17 00:00:00 2001 From: levonpetrosyan93 Date: Tue, 1 Sep 2026 15:41:17 +0400 Subject: [PATCH 09/12] per-block Spark batch verify for recent blocks --- src/batchproof_container.cpp | 87 ++++++++++++++++++++++++++--------- src/batchproof_container.h | 8 +++- src/test/spark_batch_test.cpp | 14 ++++++ src/test/spark_tests.cpp | 33 +++++++++++-- src/validation.cpp | 37 +++++++++++++-- 5 files changed, 147 insertions(+), 32 deletions(-) diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index 662418e62a..78bbb967d4 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -82,6 +82,40 @@ bool VerifySparkBatch( return true; } +bool VerifySparkBatchSnapshot( + std::vector& sparkTransactions, + const std::vector& sparkTxIds, + std::vector& historicalSparkTransactions, + const std::vector& historicalSparkTxIds) +{ + if (sparkTransactions.empty() && historicalSparkTransactions.empty()) + return true; + + std::set coverSetIds; + for (auto& tx : sparkTransactions) { + for (uint64_t id : tx.getCoinGroupIds()) + coverSetIds.insert(id); + } + for (auto& tx : historicalSparkTransactions) { + for (uint64_t id : tx.getCoinGroupIds()) + coverSetIds.insert(id); + } + std::unordered_map> coverSets; + spark::CSparkState* sparkState = spark::CSparkState::GetState(); + for (uint64_t id : coverSetIds) { + std::vector coins; + sparkState->GetCoinSet(static_cast(id), coins); + coverSets.emplace(id, std::move(coins)); + } + + return VerifySparkBatch( + sparkTransactions, + sparkTxIds, + historicalSparkTransactions, + historicalSparkTxIds, + coverSets); +} + } // namespace std::unique_ptr BatchProofContainer::instance; @@ -122,14 +156,14 @@ BatchProofContainer* BatchProofContainer::get_instance() { } } -void BatchProofContainer::init(bool collectProofs) { +void BatchProofContainer::init(bool collectProofs, bool fDeferredBatch) { LOCK(cs_batch); tempSparkTransactions.clear(); tempSparkTxIds.clear(); tempHistoricalSparkTransactions.clear(); tempHistoricalSparkTxIds.clear(); fCollectProofs = collectProofs; - if (fCollectProofs) + if (fCollectProofs && fDeferredBatch) WriteRecoveryMarker(); } @@ -164,6 +198,33 @@ void BatchProofContainer::discard_temps() fCollectProofs = false; } +bool BatchProofContainer::verify_block_batch() +{ + std::vector snapshotTransactions; + std::vector snapshotTxIds; + std::vector snapshotHistoricalTransactions; + std::vector snapshotHistoricalTxIds; + { + LOCK(cs_batch); + if (tempSparkTransactions.empty() && tempHistoricalSparkTransactions.empty()) { + fCollectProofs = false; + return true; + } + + snapshotTransactions.swap(tempSparkTransactions); + snapshotTxIds.swap(tempSparkTxIds); + snapshotHistoricalTransactions.swap(tempHistoricalSparkTransactions); + snapshotHistoricalTxIds.swap(tempHistoricalSparkTxIds); + fCollectProofs = false; + } + + return VerifySparkBatchSnapshot( + snapshotTransactions, + snapshotTxIds, + snapshotHistoricalTransactions, + snapshotHistoricalTxIds); +} + bool BatchProofContainer::verify_pending() { { LOCK(cs_batch); @@ -195,29 +256,11 @@ bool BatchProofContainer::verify_pending() { snapshotHistoricalTxIds.swap(historicalSparkTxIds); } - std::set 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> coverSets; - spark::CSparkState* sparkState = spark::CSparkState::GetState(); - for (uint64_t id : coverSetIds) { - std::vector coins; - sparkState->GetCoinSet(static_cast(id), coins); - coverSets.emplace(id, std::move(coins)); - } - - const bool passed = VerifySparkBatch( + const bool passed = VerifySparkBatchSnapshot( snapshotTransactions, snapshotTxIds, snapshotHistoricalTransactions, - snapshotHistoricalTxIds, - coverSets); + snapshotHistoricalTxIds); LOCK(cs_batch); if (!sparkTransactions.empty() || !historicalSparkTransactions.empty()) { diff --git a/src/batchproof_container.h b/src/batchproof_container.h index a6f3b9797f..3bca00e601 100644 --- a/src/batchproof_container.h +++ b/src/batchproof_container.h @@ -12,13 +12,19 @@ class BatchProofContainer { public: static BatchProofContainer* get_instance(); - void init(bool collectProofs = false); + void init(bool collectProofs = false, bool fDeferredBatch = true); void finalize(); /** Drop in-flight per-block temps without merging into the deferred batch. */ void discard_temps(); + /** + * Verify Spark proofs collected for the current block only. Clears temps on + * success or failure and does not touch the deferred cross-block batch. + */ + bool verify_block_batch(); + /** * Verify the finalized pending Spark batch when proofs are not being * collected. A no-op while collection is active, so IBD keeps diff --git a/src/test/spark_batch_test.cpp b/src/test/spark_batch_test.cpp index 3f9cef5b3f..9dbad5d55b 100644 --- a/src/test/spark_batch_test.cpp +++ b/src/test/spark_batch_test.cpp @@ -90,6 +90,20 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) container->remove(invalidSpend); BOOST_CHECK(container->verify_pending()); + // Recent blocks verify collected proofs per block without touching the + // deferred cross-block batch. + container->init(true, false); + addValidSpend(); + BOOST_CHECK(container->verify_block_batch()); + BOOST_CHECK(container->verify_block_batch()); + BOOST_CHECK(container->verify_pending()); + + container->init(true, false); + BOOST_REQUIRE(container->add(invalidSpend, spendTxB.GetHash())); + BOOST_CHECK(!container->verify_block_batch()); + BOOST_CHECK(container->verify_block_batch()); + BOOST_CHECK(container->verify_pending()); + // Checking a pending batch while collection is active must not close the // collection window. Otherwise a proof can be skipped without being // enqueued at the old-to-recent batching boundary. diff --git a/src/test/spark_tests.cpp b/src/test/spark_tests.cpp index 6ed108355d..b3a65052ac 100644 --- a/src/test/spark_tests.cpp +++ b/src/test/spark_tests.cpp @@ -2,6 +2,7 @@ #include "../batchproof_container.h" #include "../pow.h" #include "../consensus/consensus.h" +#include "../consensus/merkle.h" #include "../script/sign.h" #include "../script/standard.h" #include "../validation.h" @@ -3153,26 +3154,50 @@ BOOST_AUTO_TEST_CASE(batched_spark_proofs_are_verified_inside_connect_block) CMutableTransaction invalidSpend( GenerateSparkSpend({4 * COIN}, {}, nullptr)); BOOST_REQUIRE(!invalidSpend.vout.empty()); - ++invalidSpend.vout.front().nValue; mempool.clear(); CBlock candidate = CreateBlock({invalidSpend}, script); + ++invalidSpend.vout.front().nValue; + candidate.vtx.back() = MakeTransactionRef(invalidSpend); + candidate.hashMerkleRoot = BlockMerkleRoot(candidate); + if (candidate.IsProgPow()) { + while (!CheckProofOfWork( + progpow_hash_full(candidate.GetProgPowHeader(), candidate.mix_hash), + candidate.nBits, + consensus)) { + ++candidate.nNonce64; + } + } else { + while (!CheckProofOfWork(candidate.GetHash(), candidate.nBits, consensus)) + ++candidate.nNonce; + } + const auto usedLTags = ParseSparkSpend(*candidate.vtx.back()).getUsedLTags(); + BOOST_REQUIRE(!usedLTags.empty()); uint256 candidateHash = candidate.GetHash(); CBlockIndex candidateIndex(candidate); candidateIndex.phashBlock = &candidateHash; candidateIndex.pprev = chainActive.Tip(); candidateIndex.nHeight = chainActive.Height() + 1; - // Use a recent block time so ConnectBlock verifies proofs inline instead - // of deferring them (master IBD batching path). + // Recent blocks batch proofs per block and verify before state is committed. candidateIndex.nTime = GetSystemTimeInSeconds(); CValidationState state; CCoinsViewCache view(pcoinsTip); { LOCK(cs_main); + for (const auto& lTag : usedLTags) + BOOST_REQUIRE(!sparkState->IsUsedLTag(lTag)); BOOST_CHECK(!ConnectBlock( - candidate, state, &candidateIndex, view, ::Params(), true)); + candidate, state, &candidateIndex, view, ::Params(), false)); + BOOST_CHECK(chainActive.Tip() == candidateIndex.pprev); + BOOST_CHECK( + view.GetBestBlock() == candidateIndex.pprev->GetBlockHash()); + BOOST_CHECK(candidateIndex.GetUndoPos().IsNull()); + BOOST_CHECK(!candidateIndex.IsValid(BLOCK_VALID_SCRIPTS)); + for (const auto& lTag : usedLTags) + BOOST_CHECK(!sparkState->IsUsedLTag(lTag)); } + BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-spark-batch-proof"); // VerifyDB must avoid tip-state mutation without skipping the proof. CValidationState verifyState; diff --git a/src/validation.cpp b/src/validation.cpp index 819ee6d34e..e97c5f3e1b 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2307,12 +2307,17 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std return state.Error(strMessage); } -static bool ShouldBatchSparkProofs(const CBlockIndex* pindex) +static bool ShouldDeferSparkBatchVerification(const CBlockIndex* pindex) { // Defer Spark proof verification for blocks older than a day, which means we are syncing or reindexing return ((GetSystemTimeInSeconds() - pindex->GetBlockTime()) > 86400) && GetBoolArg("-batching", true); } +static bool ShouldCollectSparkProofs() +{ + return GetBoolArg("-batching", true); +} + bool VerifyPendingSparkBatch(CValidationState& state, const std::string& reason) { if (!BatchProofContainer::get_instance()->verify_pending()) { @@ -2797,9 +2802,12 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin std::set txIds; bool isMainNet = chainparams.GetConsensus().IsMain(); - // Defer Spark proof verification for blocks older than a day while syncing or reindexing. + // Batch Spark proofs when enabled; old blocks defer verify, recent blocks verify per block. BatchProofContainer* batchProofContainer = BatchProofContainer::get_instance(); - batchProofContainer->init(ShouldBatchSparkProofs(pindex)); + const bool fDeferBatchVerify = ShouldDeferSparkBatchVerification(pindex); + const bool fCollectSparkProofs = + ShouldCollectSparkProofs() && !fJustCheck && !isVerifyDB; + batchProofContainer->init(fCollectSparkProofs, fDeferBatchVerify); struct BatchTempCleanup { BatchProofContainer* container; @@ -3000,6 +3008,24 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } } + if (fCollectSparkProofs && !fDeferBatchVerify) { + batchTempCleanup.merged = true; + bool batchVerified = false; + try { + batchVerified = batchProofContainer->verify_block_batch(); + } catch (const std::bad_alloc&) { + return state.Error( + "ConnectBlock(): memory allocation failed during Spark batch verification"); + } + if (!batchVerified) { + return state.DoS( + 100, + error("ConnectBlock(): Spark batch proof verification failed"), + REJECT_INVALID, + "bad-spark-batch-proof"); + } + } + if (!ProcessSpecialTxsInBlock(block, pindex, state, isVerifyDB ? false : fJustCheck, fScriptChecks, !isVerifyDB)) { return error("ConnectBlock(): ProcessSpecialTxsInBlock for block %s at height %i failed with %s", pindex->GetBlockHash().ToString(), pindex->nHeight, FormatStateMessage(state)); @@ -3125,7 +3151,8 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin view.SetBestBlock(pindex->GetBlockHash()); // Merge this block's collected Spark proofs into the deferred batch. - batchProofContainer->finalize(); + if (fDeferBatchVerify) + batchProofContainer->finalize(); batchTempCleanup.merged = true; int64_t nTime5 = GetTimeMicros(); nTimeIndex += nTime5 - nTime4; @@ -4000,7 +4027,7 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } } - if (!ShouldBatchSparkProofs(pindexNewTip) && + if (!ShouldDeferSparkBatchVerification(pindexNewTip) && !VerifyPendingSparkBatch(state, "connecting new tip")) return false; From 73c4acd8ed7ebc145577075c48ca58df55e960a2 Mon Sep 17 00:00:00 2001 From: levonpetrosyan93 Date: Tue, 1 Sep 2026 17:05:31 +0400 Subject: [PATCH 10/12] rpc test fixed --- qa/rpc-tests/spark_mintspend.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/qa/rpc-tests/spark_mintspend.py b/qa/rpc-tests/spark_mintspend.py index 654dfa3c7f..c0a95d0d4d 100755 --- a/qa/rpc-tests/spark_mintspend.py +++ b/qa/rpc-tests/spark_mintspend.py @@ -53,9 +53,7 @@ def run_test(self): assert tr_type == 'mint', 'Unexpected transaction type: {}'.format(tr_type) # assert(self.wait_for_instantlock(tr, self.nodes[0])) - self.nodes[0].generate(1) - self.sync_all() - + # Unconfirmed mints cannot be spent (wallet treats nHeight < 1 as immature). res = False firoAddress = self.nodes[0].getnewaddress() try: @@ -65,7 +63,8 @@ def run_test(self): assert not res, 'Did not raise spend exception, but should be.' - # generate last confirmation block - now all transactions should be confimed + # One confirmation is enough to spend (ZC_MINT_CONFIRMATIONS == 1), and + # two minted coins make a valid cover set. self.nodes[0].generate(1) self.sync_all() From 72a909d91f0a18679407dec0525d9d099f7f1d9c Mon Sep 17 00:00:00 2001 From: levonpetrosyan93 Date: Tue, 1 Sep 2026 18:40:41 +0400 Subject: [PATCH 11/12] rpc tests fixed, ai review comments resolved --- qa/rpc-tests/llmq-is-cl-conflicts.py | 2 +- qa/rpc-tests/llmq-is-retroactive.py | 6 ++- qa/rpc-tests/spark_mintspend.py | 4 +- qa/rpc-tests/test_framework/test_framework.py | 2 +- src/batchproof_container.cpp | 24 ++++------ src/batchproof_container.h | 2 +- src/init.cpp | 12 ++--- src/test/spark_batch_test.cpp | 21 +++++++-- src/test/spark_tests.cpp | 4 +- src/validation.cpp | 47 ++++++++++--------- 10 files changed, 68 insertions(+), 56 deletions(-) diff --git a/qa/rpc-tests/llmq-is-cl-conflicts.py b/qa/rpc-tests/llmq-is-cl-conflicts.py index fc07ca5e78..001acfb4fe 100755 --- a/qa/rpc-tests/llmq-is-cl-conflicts.py +++ b/qa/rpc-tests/llmq-is-cl-conflicts.py @@ -96,7 +96,7 @@ def test_chainlock_overrides_islock(self, test_block_conflict): block = self.create_block(self.nodes[0], [rawtx2_obj]) if test_block_conflict: submit_result = self.nodes[0].submitblock(ToHex(block)) - assert(submit_result == "conflict-tx-lock") + assert submit_result == "conflict-tx-lock", submit_result cl = self.create_chainlock(self.nodes[0].getblockcount() + 1, block.sha256) self.test_node.send_clsig(cl) diff --git a/qa/rpc-tests/llmq-is-retroactive.py b/qa/rpc-tests/llmq-is-retroactive.py index 3fe60d314c..eb9364a109 100755 --- a/qa/rpc-tests/llmq-is-retroactive.py +++ b/qa/rpc-tests/llmq-is-retroactive.py @@ -33,8 +33,10 @@ def run_test(self): self.log.info("trying normal IS lock") txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) - # 3 nodes should be enough to create an IS lock even if nodes 4 and 5 (which have no tx itself) - # are the only "neighbours" in intra-quorum connections for one of them. + # Nodes 4 and 5 reject the tx (high minrelaytxfee). The remaining 3 + # quorum members must see it before an IS lock can form. + for node in self.nodes[1:4]: + self.wait_for_tx(txid, node) self.wait_for_instantlock(txid, self.nodes[0], do_assert=True) set_mocktime(get_mocktime() + 1) set_node_times(self.nodes, get_mocktime()) diff --git a/qa/rpc-tests/spark_mintspend.py b/qa/rpc-tests/spark_mintspend.py index c0a95d0d4d..1be0b03ff2 100755 --- a/qa/rpc-tests/spark_mintspend.py +++ b/qa/rpc-tests/spark_mintspend.py @@ -72,8 +72,8 @@ def run_test(self): info = self.nodes[0].gettransaction(tr[0]) confrms = info['confirmations'] assert confrms >= 1, \ - 'Confirmations should be 3, ' \ - 'due to 3 blocks was generated after transaction was created,' \ + 'Confirmations should be 1, ' \ + 'due to 1 block was generated after transaction was created,' \ 'but was {}.'.format(confrms) tr_type = info['details'][0]['category'] assert tr_type == 'mint', 'Unexpected transaction type' diff --git a/qa/rpc-tests/test_framework/test_framework.py b/qa/rpc-tests/test_framework/test_framework.py index 2c6d187168..17fe624a9c 100644 --- a/qa/rpc-tests/test_framework/test_framework.py +++ b/qa/rpc-tests/test_framework/test_framework.py @@ -742,7 +742,7 @@ def check_tx(): elif w and not expected: raise AssertionError("waiting unexpectedly succeeded") - def wait_for_instantlock(self, txid, node, expected=True, timeout=15, do_assert=False): + def wait_for_instantlock(self, txid, node, expected=True, timeout=30, do_assert=False): def check_instantlock(): try: return node.getrawtransaction(txid, True)["instantlock"] diff --git a/src/batchproof_container.cpp b/src/batchproof_container.cpp index 78bbb967d4..a2ec3fc30b 100644 --- a/src/batchproof_container.cpp +++ b/src/batchproof_container.cpp @@ -156,15 +156,13 @@ BatchProofContainer* BatchProofContainer::get_instance() { } } -void BatchProofContainer::init(bool collectProofs, bool fDeferredBatch) { +void BatchProofContainer::init(bool collectProofs) { LOCK(cs_batch); tempSparkTransactions.clear(); tempSparkTxIds.clear(); tempHistoricalSparkTransactions.clear(); tempHistoricalSparkTxIds.clear(); fCollectProofs = collectProofs; - if (fCollectProofs && fDeferredBatch) - WriteRecoveryMarker(); } void BatchProofContainer::finalize() { @@ -186,6 +184,8 @@ void BatchProofContainer::finalize() { tempHistoricalSparkTransactions.clear(); tempHistoricalSparkTxIds.clear(); fCollectProofs = false; + if (!sparkTransactions.empty() || !historicalSparkTransactions.empty()) + WriteRecoveryMarker(); } void BatchProofContainer::discard_temps() @@ -226,13 +226,6 @@ bool BatchProofContainer::verify_block_batch() } bool BatchProofContainer::verify_pending() { - { - LOCK(cs_batch); - if (fCollectProofs) { - return true; - } - } - while (true) { std::vector snapshotTransactions; std::vector snapshotTxIds; @@ -240,13 +233,13 @@ bool BatchProofContainer::verify_pending() { std::vector snapshotHistoricalTxIds; { LOCK(cs_batch); - init(); - if (fBatchFailed) { - fCollectProofs = false; + if (fCollectProofs) + return true; + if (fBatchFailed) return false; - } if (sparkTransactions.empty() && historicalSparkTransactions.empty()) { - fCollectProofs = false; + if (!fReindex) + RemoveRecoveryMarker(); return true; } @@ -283,7 +276,6 @@ bool BatchProofContainer::verify_pending() { continue; } - fCollectProofs = false; if (passed) { if (!fReindex) RemoveRecoveryMarker(); diff --git a/src/batchproof_container.h b/src/batchproof_container.h index 3bca00e601..290b59233d 100644 --- a/src/batchproof_container.h +++ b/src/batchproof_container.h @@ -12,7 +12,7 @@ class BatchProofContainer { public: static BatchProofContainer* get_instance(); - void init(bool collectProofs = false, bool fDeferredBatch = true); + void init(bool collectProofs = false); void finalize(); diff --git a/src/init.cpp b/src/init.cpp index d2e65827b0..4446129022 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1973,12 +1973,12 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler) // ********************************************************* Step 7b: load block chain - // Deferred Spark batching writes sparkbatchfailed when collection starts and - // removes it after a successful verify. A failed batch verify or crash while - // proofs are still pending leaves the marker; force -reindex with -batching=0 - // so chainstate is rebuilt with per-block verification. Checked here rather - // than in LoadBlockIndexDB() because a run restarted with -reindex wipes the - // block tree database and never calls LoadBlockIndexDB(). + // Deferred Spark batching writes sparkbatchfailed when a non-empty batch is + // finalized and removes it after a successful verify. A failed batch verify + // or crash while proofs are still pending leaves the marker; force -reindex + // with -batching=0 so chainstate is rebuilt with per-block verification. + // Checked here rather than in LoadBlockIndexDB() because a run restarted + // with -reindex wipes the block tree database and never calls LoadBlockIndexDB(). if (BatchProofContainer::HasRecoveryMarker()) { LogPrintf("Previous run did not finish Spark batch verification, disabling -batching and forcing -reindex for this run\n"); ForceSetArg("-batching", "0"); diff --git a/src/test/spark_batch_test.cpp b/src/test/spark_batch_test.cpp index 9dbad5d55b..f702787042 100644 --- a/src/test/spark_batch_test.cpp +++ b/src/test/spark_batch_test.cpp @@ -92,22 +92,35 @@ BOOST_AUTO_TEST_CASE(spark_batch_fail_closed) // Recent blocks verify collected proofs per block without touching the // deferred cross-block batch. - container->init(true, false); + container->init(true); addValidSpend(); BOOST_CHECK(container->verify_block_batch()); BOOST_CHECK(container->verify_block_batch()); + BOOST_CHECK(!BatchProofContainer::HasRecoveryMarker()); BOOST_CHECK(container->verify_pending()); - container->init(true, false); + container->init(true); BOOST_REQUIRE(container->add(invalidSpend, spendTxB.GetHash())); BOOST_CHECK(!container->verify_block_batch()); BOOST_CHECK(container->verify_block_batch()); + BOOST_CHECK(!BatchProofContainer::HasRecoveryMarker()); + BOOST_CHECK(container->verify_pending()); + + // Empty deferred finalize must not write sparkbatchfailed. + container->init(true); + container->finalize(); + BOOST_CHECK(!BatchProofContainer::HasRecoveryMarker()); BOOST_CHECK(container->verify_pending()); + collectSpend(); + BOOST_CHECK(BatchProofContainer::HasRecoveryMarker()); + BOOST_CHECK(container->verify_pending()); + BOOST_CHECK(!BatchProofContainer::HasRecoveryMarker()); + // Checking a pending batch while collection is active must not close the - // collection window. Otherwise a proof can be skipped without being - // enqueued at the old-to-recent batching boundary. + // collection window or drop temps already enqueued. container->init(true); + BOOST_REQUIRE(container->add(invalidSpend, spendTxB.GetHash())); BOOST_CHECK(container->verify_pending()); BOOST_CHECK(container->add(invalidSpend, spendTxB.GetHash())); container->finalize(); diff --git a/src/test/spark_tests.cpp b/src/test/spark_tests.cpp index b3a65052ac..a603b8c592 100644 --- a/src/test/spark_tests.cpp +++ b/src/test/spark_tests.cpp @@ -3179,7 +3179,7 @@ BOOST_AUTO_TEST_CASE(batched_spark_proofs_are_verified_inside_connect_block) candidateIndex.pprev = chainActive.Tip(); candidateIndex.nHeight = chainActive.Height() + 1; // Recent blocks batch proofs per block and verify before state is committed. - candidateIndex.nTime = GetSystemTimeInSeconds(); + candidateIndex.nTime = GetTime(); CValidationState state; CCoinsViewCache view(pcoinsTip); @@ -3254,7 +3254,7 @@ BOOST_AUTO_TEST_CASE(abandoned_connect_block_clears_batched_spark_proofs) candidateIndex.nHeight = chainActive.Height() + 1; // Old enough to enable deferred batching while ConnectBlock still fails // on the missing transparent input before finalize. - candidateIndex.nTime = GetSystemTimeInSeconds() - 86401; + candidateIndex.nTime = GetTime() - 86401; CValidationState state; CCoinsViewCache view(pcoinsTip); diff --git a/src/validation.cpp b/src/validation.cpp index e97c5f3e1b..c4951eea32 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2309,8 +2309,10 @@ bool AbortNode(CValidationState& state, const std::string& strMessage, const std static bool ShouldDeferSparkBatchVerification(const CBlockIndex* pindex) { - // Defer Spark proof verification for blocks older than a day, which means we are syncing or reindexing - return ((GetSystemTimeInSeconds() - pindex->GetBlockTime()) > 86400) && GetBoolArg("-batching", true); + // Defer Spark proof verification for blocks older than a day (IBD/reindex). + // GetTime() is mockable so -mocktime RPC tests use the same recent vs deferred + // split as a live node; wall-clock time would treat 2014 mocktime chains as IBD. + return ((GetTime() - pindex->GetBlockTime()) > 86400) && GetBoolArg("-batching", true); } static bool ShouldCollectSparkProofs() @@ -2807,7 +2809,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin const bool fDeferBatchVerify = ShouldDeferSparkBatchVerification(pindex); const bool fCollectSparkProofs = ShouldCollectSparkProofs() && !fJustCheck && !isVerifyDB; - batchProofContainer->init(fCollectSparkProofs, fDeferBatchVerify); + batchProofContainer->init(fCollectSparkProofs); struct BatchTempCleanup { BatchProofContainer* container; @@ -3008,24 +3010,6 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } } - if (fCollectSparkProofs && !fDeferBatchVerify) { - batchTempCleanup.merged = true; - bool batchVerified = false; - try { - batchVerified = batchProofContainer->verify_block_batch(); - } catch (const std::bad_alloc&) { - return state.Error( - "ConnectBlock(): memory allocation failed during Spark batch verification"); - } - if (!batchVerified) { - return state.DoS( - 100, - error("ConnectBlock(): Spark batch proof verification failed"), - REJECT_INVALID, - "bad-spark-batch-proof"); - } - } - if (!ProcessSpecialTxsInBlock(block, pindex, state, isVerifyDB ? false : fJustCheck, fScriptChecks, !isVerifyDB)) { return error("ConnectBlock(): ProcessSpecialTxsInBlock for block %s at height %i failed with %s", pindex->GetBlockHash().ToString(), pindex->nHeight, FormatStateMessage(state)); @@ -3058,6 +3042,27 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin } } + // After InstantSend filtering so a conflicting block still returns + // conflict-tx-lock instead of a Spark-batch reject, and collection stays + // open through ProcessSpecialTxsInBlock as it did on master. + if (fCollectSparkProofs && !fDeferBatchVerify) { + batchTempCleanup.merged = true; + bool batchVerified = false; + try { + batchVerified = batchProofContainer->verify_block_batch(); + } catch (const std::bad_alloc&) { + return state.Error( + "ConnectBlock(): memory allocation failed during Spark batch verification"); + } + if (!batchVerified) { + return state.DoS( + 100, + error("ConnectBlock(): Spark batch proof verification failed"), + REJECT_INVALID, + "bad-spark-batch-proof"); + } + } + int64_t nTime5_1 = GetTimeMicros(); nTimeISFilter += nTime5_1 - nTime4; LogPrint("bench", " - IS filter: %.2fms [%.2fs]\n", 0.001 * (nTime5_1 - nTime4), nTimeISFilter * 0.000001); From f491e6cd7bec1e4d60d4b69962e36aa2cb4219c0 Mon Sep 17 00:00:00 2001 From: Reuben Yap Date: Thu, 3 Sep 2026 14:06:42 +0200 Subject: [PATCH 12/12] Tests: age Spark mints before mining --- qa/rpc-tests/spark_mintspend.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/rpc-tests/spark_mintspend.py b/qa/rpc-tests/spark_mintspend.py index 1be0b03ff2..e501e913f5 100755 --- a/qa/rpc-tests/spark_mintspend.py +++ b/qa/rpc-tests/spark_mintspend.py @@ -63,6 +63,10 @@ def run_test(self): assert not res, 'Did not raise spend exception, but should be.' + # This test has no quorum to InstantLock the mints, so age them past + # WAIT_FOR_ISLOCK_TIMEOUT before mining their first confirmation. + set_node_times(self.nodes, int(time()) + 10 * 60 + 1) + # One confirmation is enough to spend (ZC_MINT_CONFIRMATIONS == 1), and # two minted coins make a valid cover set. self.nodes[0].generate(1)