wallet: batch and simplify ZapSelectTx process

The goal of the function is to erase the wallet transactions that
match the inputted hashes. There is no need to traverse the database,
reading record by record, to then perform single entry removals for
each of them.

To ensure consistency and improve performance, this change-set removes
all tx records within a single atomic db batch operation, as well as
it cleans up code, improves error handling and simplifies the
transactions removal process entirely.

This optimizes the removal of watch-only transactions during the wallet
migration process and the 'removeprunedfunds' RPC command.
This commit is contained in:
furszy
2023-07-16 13:31:38 -03:00
parent 595d50a103
commit 83b762845f
7 changed files with 44 additions and 113 deletions

View File

@@ -2311,12 +2311,41 @@ DBErrors CWallet::LoadWallet()
return nLoadWalletRet;
}
DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut)
util::Result<void> CWallet::ZapSelectTx(std::vector<uint256>& txs_to_remove)
{
AssertLockHeld(cs_wallet);
DBErrors nZapSelectTxRet = WalletBatch(GetDatabase()).ZapSelectTx(vHashIn, vHashOut);
for (const uint256& hash : vHashOut) {
const auto& it = mapWallet.find(hash);
WalletBatch batch(GetDatabase());
if (!batch.TxnBegin()) return util::Error{_("Error starting db txn for wallet transactions removal")};
// Check for transaction existence and remove entries from disk
using TxIterator = std::unordered_map<uint256, CWalletTx, SaltedTxidHasher>::const_iterator;
std::vector<TxIterator> erased_txs;
bilingual_str str_err;
for (const uint256& hash : txs_to_remove) {
auto it_wtx = mapWallet.find(hash);
if (it_wtx == mapWallet.end()) {
str_err = strprintf(_("Transaction %s does not belong to this wallet"), hash.GetHex());
break;
}
if (!batch.EraseTx(hash)) {
str_err = strprintf(_("Failure removing transaction: %s"), hash.GetHex());
break;
}
erased_txs.emplace_back(it_wtx);
}
// Roll back removals in case of an error
if (!str_err.empty()) {
batch.TxnAbort();
return util::Error{str_err};
}
// Dump changes to disk
if (!batch.TxnCommit()) return util::Error{_("Error committing db txn for wallet transactions removal")};
// Update the in-memory state and notify upper layers about the removals
for (const auto& it : erased_txs) {
const uint256 hash{it->first};
wtxOrdered.erase(it->second.m_it_wtxOrdered);
for (const auto& txin : it->second.tx->vin)
mapTxSpends.erase(txin.prevout);
@@ -2324,12 +2353,9 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256
NotifyTransactionChanged(hash, CT_DELETED);
}
if (nZapSelectTxRet != DBErrors::LOAD_OK)
return nZapSelectTxRet;
MarkDirty();
return DBErrors::LOAD_OK;
return {}; // all good
}
bool CWallet::SetAddressBookWithDB(WalletBatch& batch, const CTxDestination& address, const std::string& strName, const std::optional<AddressPurpose>& new_purpose)
@@ -3925,13 +3951,8 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
watchonly_batch.reset(); // Flush
// Do the removes
if (txids_to_delete.size() > 0) {
std::vector<uint256> deleted_txids;
if (ZapSelectTx(txids_to_delete, deleted_txids) != DBErrors::LOAD_OK) {
error = _("Error: Could not delete watchonly transactions");
return false;
}
if (deleted_txids != txids_to_delete) {
error = _("Error: Not all watchonly txs could be deleted");
if (auto res = ZapSelectTx(txids_to_delete); !res) {
error = _("Error: Could not delete watchonly transactions. ") + util::ErrorString(res);
return false;
}
}