mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-04-04 04:45:10 +02:00
Merge bitcoin/bitcoin#28987: wallet: simplify and batch zap wallet txes process
9a3c5c8697scripted-diff: rename ZapSelectTx to RemoveTxs (furszy)83b762845fwallet: batch and simplify ZapSelectTx process (furszy)595d50a103wallet: migration, remove extra NotifyTransactionChanged call (furszy)a2b071f992wallet: ZapSelectTx, remove db rewrite code (furszy) Pull request description: Work decoupled from #28574. Brother of #28894. Includes two different, yet interconnected, performance and code improvements to the zap wallet transactions process. 1) As the goal of the `ZapSelectTx` function is to erase tx records that match any of the inputted hashes. There is no need to traverse the whole database record by record. We could just check if the tx exist, and remove it directly by calling `EraseTx()`. 2) Instead of performing single write operations per removed tx record, this PR batches them all within a single atomic db txn. Moreover, these changes will enable us to consolidate all individual write operations that take place during the wallet migration process into a single db txn in the future. ACKs for top commit: achow101: ACK9a3c5c8697josibake: ACK9a3c5c8697Tree-SHA512: fb2ecc48224c400ab3b1fbb32e174b5b13bf03794717727f80f01f55fb183883b067a68c0a127b2de8885564da15425d021a96541953bf38a72becc2e9929ccf
This commit is contained in:
@@ -2320,12 +2320,41 @@ DBErrors CWallet::LoadWallet()
|
||||
return nLoadWalletRet;
|
||||
}
|
||||
|
||||
DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256>& vHashOut)
|
||||
util::Result<void> CWallet::RemoveTxs(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);
|
||||
@@ -2333,22 +2362,9 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256
|
||||
NotifyTransactionChanged(hash, CT_DELETED);
|
||||
}
|
||||
|
||||
if (nZapSelectTxRet == DBErrors::NEED_REWRITE)
|
||||
{
|
||||
if (GetDatabase().Rewrite("\x04pool"))
|
||||
{
|
||||
for (const auto& spk_man_pair : m_spk_managers) {
|
||||
spk_man_pair.second->RewriteDB();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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)
|
||||
@@ -4025,19 +4041,10 @@ 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");
|
||||
if (auto res = RemoveTxs(txids_to_delete); !res) {
|
||||
error = _("Error: Could not delete watchonly transactions. ") + util::ErrorString(res);
|
||||
return false;
|
||||
}
|
||||
if (deleted_txids != txids_to_delete) {
|
||||
error = _("Error: Not all watchonly txs could be deleted");
|
||||
return false;
|
||||
}
|
||||
// Tell the GUI of each tx
|
||||
for (const uint256& txid : deleted_txids) {
|
||||
NotifyTransactionChanged(txid, CT_UPDATED);
|
||||
}
|
||||
}
|
||||
|
||||
// Pair external wallets with their corresponding db handler
|
||||
|
||||
Reference in New Issue
Block a user