Wallet: identify block mints outside cs_main

src/spark/sparkwallet.cpp

@@ -859,49 +859,55 @@ CAmount CSparkWallet::getMySpendAmount(const std::vector<GroupElement>& lTags) c
     return result;
 }
 
-void CSparkWallet::UpdateMintState(const std::vector<spark::Coin>& coins, const uint256& txHash, CWalletDB& walletdb) {
-    spark::CSparkState *sparkState = spark::CSparkState::GetState();
-    for (auto coin : coins) {
-        try {
-            spark::IdentifiedCoinData identifiedCoinData = coin.identify(this->viewKey);
-            spark::RecoveredCoinData recoveredCoinData = coin.recover(this->fullViewKey, identifiedCoinData);
-            CSparkMintMeta mintMeta;
-            auto mintedCoinHeightAndId = sparkState->GetMintedCoinHeightAndId(coin);
-            mintMeta.nHeight = mintedCoinHeightAndId.first;
-            mintMeta.nId = mintedCoinHeightAndId.second;
-            mintMeta.isUsed = false;
-            mintMeta.txid = txHash;
-            mintMeta.i = identifiedCoinData.i;
-            mintMeta.d = identifiedCoinData.d;
-            mintMeta.v = identifiedCoinData.v;
-            mintMeta.k = identifiedCoinData.k;
-            mintMeta.memo = identifiedCoinData.memo;
-            mintMeta.serial_context = coin.serial_context;
-            mintMeta.coin = coin;
-            mintMeta.type = coin.type;
-            //! Check whether this mint has been spent and is considered 'pending' or 'confirmed'
-            {
-                LOCK(mempool.cs);
-                mintMeta.isUsed = mempool.sparkState.HasLTag(recoveredCoinData.T);
-            }
+CSparkWallet::IdentifiedMint CSparkWallet::IdentifyMint(spark::Coin coin, const uint256& txHash) const
+{
+    // These keys are initialized before the wallet worker starts and never change.
+    const auto identified = coin.identify(viewKey);
+    const auto recovered = coin.recover(fullViewKey, identified);
+    IdentifiedMint mint{};
+    mint.meta.txid = txHash;
+    mint.meta.i = identified.i;
+    mint.meta.d = identified.d;
+    mint.meta.v = identified.v;
+    mint.meta.k = identified.k;
+    mint.meta.memo = identified.memo;
+    mint.meta.serial_context = coin.serial_context;
+    mint.meta.type = coin.type;
+    mint.meta.coin = std::move(coin);
+    mint.lTag = recovered.T;
+    return mint;
+}
+
+void CSparkWallet::RecordMint(IdentifiedMint mint, CWalletDB& walletdb)
+{
+    auto& mintMeta = mint.meta;
+    const auto heightAndId = spark::CSparkState::GetState()->GetMintedCoinHeightAndId(mintMeta.coin);
+    mintMeta.nHeight = heightAndId.first;
+    mintMeta.nId = heightAndId.second;
+    mintMeta.isUsed = false;
+    uint256 spendTxHash;
+    for (auto* pool : {&mempool, &txpools.getStemTxPool()}) {
+        LOCK(pool->cs);
+        if (pool->sparkState.HasLTag(mint.lTag)) {
+            mintMeta.isUsed = true;
+            spendTxHash = pool->sparkState.GetMempoolConflictingTxHash(mint.lTag);
+            break;
+        }
+    }
 
-            uint256 lTagHash = primitives::GetLTagHash(recoveredCoinData.T);
-            addOrUpdateMint(mintMeta, lTagHash, walletdb);
+    const uint256 lTagHash = primitives::GetLTagHash(mint.lTag);
+    addOrUpdateMint(mintMeta, lTagHash, walletdb);
 
-            if (mintMeta.isUsed) {
-                uint256 spendTxHash;
-                {
-                    LOCK(mempool.cs);
-                    spendTxHash = mempool.sparkState.GetMempoolConflictingTxHash(recoveredCoinData.T);
-                }
-                UpdateSpendState(recoveredCoinData.T, lTagHash, spendTxHash, false);
-            }
+    if (mintMeta.isUsed) {
+        UpdateSpendState(mint.lTag, lTagHash, spendTxHash, false);
+    }
+}
 
-//            pwalletMain->NotifyZerocoinChanged(
-//                    pwalletMain,
-//                    lTagHash.GetHex(),
-//                    std::string("Update (") + std::to_string((double)mintMeta.v / COIN) + "mint)",
-//                    CT_UPDATED);
+void CSparkWallet::UpdateMintState(const std::vector<spark::Coin>& coins, const uint256& txHash, CWalletDB& walletdb)
+{
+    for (auto coin : coins) {
+        try {
+            RecordMint(IdentifyMint(std::move(coin), txHash), walletdb);
         } catch (const std::runtime_error& e) {
             continue;
         }
@@ -922,16 +928,33 @@ void CSparkWallet::UpdateMintStateFromBlock(const CBlock& block) {
     std::vector<CTransactionRef> vtxCopy = block.vtx;
     const uint256 blockHash = block.GetHash();
     ((ParallelOpThreadPool<void>*)threadPool)->PostTask([=, this]() mutable {
+        std::vector<IdentifiedMint> mints;
+        for (const auto& tx : vtxCopy) {
+            if (tx->IsSparkTransaction()) {
+                auto coins = spark::GetSparkMintCoins(*tx);
+                for (auto& coin : coins) {
+                    try {
+                        mints.push_back(IdentifyMint(std::move(coin), tx->GetHash()));
+                    } catch (const std::runtime_error&) {
+                        // Most outputs belong to other wallets.
+                    }
+                }
+            }
+        }
+        if (mints.empty())
+            return;
+
+        // Identification can overlap a reorg. Only record mints still on the active chain.
         LOCK2(cs_main, cs_spark_wallet);
         auto it = mapBlockIndex.find(blockHash);
         if (it == mapBlockIndex.end() || !chainActive.Contains(it->second))
             return;
         CWalletDB walletdb(strWalletFile);
-        for (const auto& tx : vtxCopy) {
-            if (tx->IsSparkTransaction()) {
-                auto coins = spark::GetSparkMintCoins(*tx);
-                uint256 txHash = tx->GetHash();
-                UpdateMintState(coins, txHash, walletdb);
+        for (auto& mint : mints) {
+            try {
+                RecordMint(std::move(mint), walletdb);
+            } catch (const std::runtime_error&) {
+                continue;
             }
         }
     });

src/spark/sparkwallet.h

@@ -228,6 +228,15 @@ class CSparkWallet  {
     mutable CCriticalSection cs_spark_wallet;
 
 private:
+    struct IdentifiedMint
+    {
+        CSparkMintMeta meta;
+        GroupElement lTag;
+    };
+
+    IdentifiedMint IdentifyMint(spark::Coin coin, const uint256& txHash) const;
+    void RecordMint(IdentifiedMint mint, CWalletDB& walletdb);
+
     std::string strWalletFile;
     // this is latest used diversifier
     int32_t lastDiversifier GUARDED_BY(cs_spark_wallet);

src/wallet/test/spark_wallet_tests.cpp

@@ -190,6 +190,126 @@ BOOST_AUTO_TEST_CASE(list_spark_mints)
     sparkState->Reset();
 }
 
+BOOST_AUTO_TEST_CASE(block_mint_scan_and_queued_reorg)
+{
+    GenerateBlocks(1001);
+    auto* wallet = pwalletMain->sparkWallet.get();
+    const spark::SpendKey foreignSpendKey(params);
+    const spark::FullViewKey foreignFullViewKey(foreignSpendKey);
+    const spark::IncomingViewKey foreignViewKey(foreignFullViewKey);
+    const std::vector<spark::MintedCoinData> outputs = {
+        {wallet->getDefaultAddress(), 2 * COIN, "wallet mint"},
+        {spark::Address(foreignViewKey, 0), 3 * COIN, "foreign mint"},
+    };
+    std::vector<std::pair<CWalletTx, CAmount>> transactions;
+    BOOST_REQUIRE_EQUAL(pwalletMain->MintAndStoreSpark(outputs, transactions, false, true), "");
+    BOOST_REQUIRE_EQUAL(transactions.size(), 1);
+    const auto* index = GenerateBlock({CMutableTransaction(*transactions[0].first.tx)});
+    BOOST_REQUIRE(index);
+    const CBlock block = GetCBlock(index);
+    wallet->FinishTasks();
+
+    CWalletDB walletdb(pwalletMain->strWalletFile);
+    const auto initial = wallet->getMintMap();
+    BOOST_REQUIRE_EQUAL(initial.size(), 1);
+    const auto lTagHash = initial.begin()->first;
+    const auto expected = initial.begin()->second;
+
+    // Rediscover the owned output through the block worker, without cached metadata.
+    wallet->eraseMint(lTagHash, walletdb);
+    wallet->UpdateMintStateFromBlock(block);
+    wallet->FinishTasks();
+    const auto scanned = wallet->getMintMap();
+    BOOST_REQUIRE_EQUAL(scanned.size(), 1);
+    BOOST_REQUIRE(scanned.count(lTagHash));
+    const auto& actual = scanned.at(lTagHash);
+    BOOST_CHECK_EQUAL(actual.nHeight, index->nHeight);
+    BOOST_CHECK_EQUAL(actual.nId, expected.nId);
+    BOOST_CHECK_EQUAL(actual.v, 2 * COIN);
+    BOOST_CHECK_EQUAL(actual.memo, "wallet mint");
+    BOOST_CHECK(actual.txid == expected.txid);
+    BOOST_CHECK(actual.coin == expected.coin);
+    BOOST_CHECK(actual.serial_context == expected.serial_context);
+    BOOST_CHECK(!actual.isUsed);
+    BOOST_CHECK(wallet->validateLookupIndexes());
+    CSparkMintMeta persisted;
+    BOOST_REQUIRE(walletdb.ReadSparkMint(lTagHash, persisted));
+    BOOST_CHECK_EQUAL(persisted.nHeight, actual.nHeight);
+    BOOST_CHECK_EQUAL(persisted.v, actual.v);
+    BOOST_CHECK(persisted.coin == actual.coin);
+
+    // The worker may identify coins now, but cannot record them until cs_main is released.
+    // Disconnect first: a queued scan must not resurrect the removed wallet mint.
+    {
+        LOCK(cs_main);
+        wallet->UpdateMintStateFromBlock(block);
+        BOOST_REQUIRE(DisconnectBlocks(1));
+        // Discard the resurrected mempool mint as well, leaving only the stale block job.
+        mempool.clear();
+        txpools.getStemTxPool().clear();
+        wallet->eraseMint(lTagHash, walletdb);
+    }
+    wallet->FinishTasks();
+    BOOST_CHECK(wallet->getMintMap().empty());
+    BOOST_CHECK(!walletdb.ReadSparkMint(lTagHash, persisted));
+    BOOST_CHECK(wallet->validateLookupIndexes());
+
+    CValidationState state;
+    BOOST_REQUIRE(ActivateBestChain(state, ::Params(), std::make_shared<CBlock>(block)));
+    wallet->FinishTasks();
+    BOOST_CHECK_EQUAL(wallet->getMintMap().size(), 1);
+    BOOST_REQUIRE(walletdb.ReadSparkMint(lTagHash, persisted));
+    BOOST_CHECK_EQUAL(persisted.nHeight, index->nHeight);
+    spark::CSparkState::GetState()->Reset();
+}
+
+
+BOOST_AUTO_TEST_CASE(block_mint_scan_preserves_pending_spends)
+{
+    GenerateBlocks(500);
+    std::vector<CMutableTransaction> mintTransactions;
+    GenerateMints({5 * COIN, COIN}, mintTransactions);
+    txpools.clear();
+    const auto* index = GenerateBlock(mintTransactions);
+    BOOST_REQUIRE(index);
+    const CBlock block = GetCBlock(index);
+    GenerateBlocks(10);
+
+    auto* wallet = pwalletMain->sparkWallet.get();
+    wallet->FinishTasks();
+    const CTransaction spend = GenerateSparkSpend({4 * COIN}, {}, nullptr);
+    const auto lTags = spark::GetSparkUsedTags(spend);
+    BOOST_REQUIRE_EQUAL(lTags.size(), 1);
+    const uint256 lTagHash = primitives::GetLTagHash(lTags[0]);
+    CWalletDB walletdb(pwalletMain->strWalletFile);
+
+    for (auto* pool : {&mempool, &txpools.getStemTxPool()}) {
+        txpools.clear();
+        {
+            LOCK(cs_main);
+            CValidationState state;
+            BOOST_REQUIRE(AcceptToMemoryPool(*pool, state, MakeTransactionRef(spend), false, nullptr));
+        }
+        wallet->FinishTasks();
+
+        // Rediscover the mint after its spend notification has already been processed.
+        wallet->eraseMint(lTagHash, walletdb);
+        walletdb.EraseSparkSpendEntry(lTags[0]);
+        wallet->UpdateMintStateFromBlock(block);
+        wallet->FinishTasks();
+
+        BOOST_CHECK(wallet->getMintMeta(lTagHash).isUsed);
+        CSparkMintMeta persisted;
+        BOOST_REQUIRE(walletdb.ReadSparkMint(lTagHash, persisted));
+        BOOST_CHECK(persisted.isUsed);
+        CSparkSpendEntry entry;
+        BOOST_REQUIRE(walletdb.ReadSparkSpendEntry(lTags[0], entry));
+        BOOST_CHECK(entry.hashTx == spend.GetHash());
+        BOOST_CHECK_EQUAL(entry.amount, persisted.v);
+    }
+    txpools.clear();
+    spark::CSparkState::GetState()->Reset();
+}
 
 BOOST_AUTO_TEST_CASE(spend)
 {