feat(wallet): try batching multiple wallet db operations when possibl…

…e, avoid wasting cpu cycles in AddToWalletIfInvolvingMe (#5452)

## Issue being fixed or feature implemented
It's super slow for wallets with 100.000s of keys and txes to reindex
and to rescan. Batching multiple operations fixes it. In my case (300K+
keys and 500k+ txes testnet wallet) `rescanblockchain` time is down from
6+ hours to ~10 minutes.

Re-calculating `block_time` over and over again inside of the loop in
`AddToWalletIfInvolvingMe` is wasteful, move it out.

## What was done?
batch what's possible, optimize `AddToWalletIfInvolvingMe`

## How Has This Been Tested?
running on top of #5451 , wiping and rescanning w/ and w/out this patch.

## Breaking Changes
should be none

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

src/wallet/wallet.cpp

@@ -1001,7 +1001,7 @@ void CWallet::LoadToWallet(CWalletTx& wtxIn)
     }
 }
 
-bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate)
+bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate)
 {
     const CTransaction& tx = *ptx;
     {
@@ -1030,17 +1030,16 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co
              * the mostly recently created transactions from newer versions of the wallet.
              */
 
-            WalletBatch batch(*database);
+            std::optional<int64_t> block_time;
+            if (!confirm.hashBlock.IsNull()) {
+                int64_t block_time_tmp;
+                bool found_block = chain().findBlock(confirm.hashBlock, FoundBlock().maxTime(block_time_tmp));
+                assert(found_block);
+                block_time = block_time_tmp;
+            }
             // loop though all outputs
             for (const CTxOut& txout: tx.vout) {
                 for (const auto& spk_man_pair : m_spk_managers) {
-                    std::optional<int64_t> block_time;
-                    if (!confirm.hashBlock.IsNull()) {
-                        int64_t block_time_tmp;
-                        bool found_block = chain().findBlock(confirm.hashBlock, FoundBlock().maxTime(block_time_tmp));
-                        assert(found_block);
-                        block_time = block_time_tmp;
-                    }
                     spk_man_pair.second->MarkUnusedAddresses(batch, txout.scriptPubKey, block_time);
                 }
             }
@@ -1205,9 +1204,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c
     fAnonymizableTallyCachedNonDenom = false;
 }
 
-void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx)
+void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool update_tx)
 {
-    if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx))
+    if (!AddToWalletIfInvolvingMe(ptx, confirm, batch, update_tx))
         return; // Not one of ours
 
     // If a transaction changes 'conflicted' state, that changes the balance
@@ -1222,7 +1221,8 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio
 void CWallet::transactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcceptTime) {
     LOCK(cs_wallet);
     CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
-    SyncTransaction(ptx, confirm);
+    WalletBatch batch(*database);
+    SyncTransaction(ptx, confirm, batch);
 
     auto it = mapWallet.find(ptx->GetHash());
     if (it != mapWallet.end()) {
@@ -1247,9 +1247,10 @@ void CWallet::blockConnected(const CBlock& block, int height)
 
     m_last_block_processed_height = height;
     m_last_block_processed = block_hash;
+    WalletBatch batch(*database);
     for (size_t index = 0; index < block.vtx.size(); index++) {
         CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, index);
-        SyncTransaction(block.vtx[index], confirm);
+        SyncTransaction(block.vtx[index], confirm, batch);
         transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::MANUAL);
     }
 
@@ -1268,9 +1269,10 @@ void CWallet::blockDisconnected(const CBlock& block, int height)
     // future with a stickier abandoned state or even removing abandontransaction call.
     m_last_block_processed_height = height - 1;
     m_last_block_processed = block.hashPrevBlock;
+    WalletBatch batch(*database);
     for (const CTransactionRef& ptx : block.vtx) {
         CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0);
-        SyncTransaction(ptx, confirm);
+        SyncTransaction(ptx, confirm, batch);
     }
 
     // reset cache to make sure no longer mature coins are excluded
@@ -1926,6 +1928,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
     double progress_end = chain().guessVerificationProgress(end_hash);
     double progress_current = progress_begin;
     int block_height = start_height;
+    WalletBatch batch(*database);
     while (!fAbortRescan && !chain().shutdownRequested()) {
         if (progress_end - progress_begin > 0.0) {
             m_scanning_progress = (progress_current - progress_begin) / (progress_end - progress_begin);
@@ -1962,7 +1965,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc
             }
             for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
                 CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block_height, block_hash, posInBlock);
-                SyncTransaction(block.vtx[posInBlock], confirm, fUpdate);
+                SyncTransaction(block.vtx[posInBlock], confirm, batch, fUpdate);
             }
             // scan succeeded, record block as most recent successfully scanned
             result.last_scanned_block = block_hash;

src/wallet/wallet.h

@@ -716,7 +716,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
      * Abandoned state should probably be more carefully tracked via different
      * posInBlock signals or by checking mempool presence when necessary.
      */
-    bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */
     void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx);
@@ -728,7 +728,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
 
     /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions.
      * Should be called with non-zero block_hash and posInBlock if this is for a transaction that is included in a block. */
-    void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
+    void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, WalletBatch& batch, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
 
     std::atomic<uint64_t> m_wallet_flags{0};