feat(wallet): try batching multiple wallet db operations when possible, 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)_
This commit is contained in:
UdjinM6 2023-06-28 18:59:49 +03:00 committed by GitHub
parent 78fa019952
commit 9138ff738a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 17 deletions

View File

@ -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; 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. * 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 // loop though all outputs
for (const CTxOut& txout: tx.vout) { for (const CTxOut& txout: tx.vout) {
for (const auto& spk_man_pair : m_spk_managers) { 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); 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; 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 return; // Not one of ours
// If a transaction changes 'conflicted' state, that changes the balance // 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) { void CWallet::transactionAddedToMempool(const CTransactionRef& ptx, int64_t nAcceptTime) {
LOCK(cs_wallet); LOCK(cs_wallet);
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); 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()); auto it = mapWallet.find(ptx->GetHash());
if (it != mapWallet.end()) { 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_height = height;
m_last_block_processed = block_hash; m_last_block_processed = block_hash;
WalletBatch batch(*database);
for (size_t index = 0; index < block.vtx.size(); index++) { for (size_t index = 0; index < block.vtx.size(); index++) {
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, block_hash, 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); 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. // future with a stickier abandoned state or even removing abandontransaction call.
m_last_block_processed_height = height - 1; m_last_block_processed_height = height - 1;
m_last_block_processed = block.hashPrevBlock; m_last_block_processed = block.hashPrevBlock;
WalletBatch batch(*database);
for (const CTransactionRef& ptx : block.vtx) { for (const CTransactionRef& ptx : block.vtx) {
CWalletTx::Confirmation confirm(CWalletTx::Status::UNCONFIRMED, /* block_height */ 0, {}, /* nIndex */ 0); 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 // 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_end = chain().guessVerificationProgress(end_hash);
double progress_current = progress_begin; double progress_current = progress_begin;
int block_height = start_height; int block_height = start_height;
WalletBatch batch(*database);
while (!fAbortRescan && !chain().shutdownRequested()) { while (!fAbortRescan && !chain().shutdownRequested()) {
if (progress_end - progress_begin > 0.0) { if (progress_end - progress_begin > 0.0) {
m_scanning_progress = (progress_current - progress_begin) / (progress_end - progress_begin); 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) { for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) {
CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, block_height, block_hash, 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 // scan succeeded, record block as most recent successfully scanned
result.last_scanned_block = block_hash; result.last_scanned_block = block_hash;

View File

@ -716,7 +716,7 @@ private:
* Abandoned state should probably be more carefully tracked via different * Abandoned state should probably be more carefully tracked via different
* posInBlock signals or by checking mempool presence when necessary. * 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. */ /* 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); void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx);
@ -728,7 +728,7 @@ private:
/* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions. /* 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. */ * 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}; std::atomic<uint64_t> m_wallet_flags{0};