From 9539d342cf7a6b20b3b27ebc9947350009fd57fd Mon Sep 17 00:00:00 2001 From: jwinterm Date: Mon, 27 Jul 2026 18:17:42 -0400 Subject: [PATCH] Contracts: cancel every tx log entry for a slate in one batch cancel_tx committed each entry separately, so a failure part way through left some entries cancelled and the rest untouched, and a retry then stopped on the entry that was already cancelled. Collect the entries and their outputs first and cancel them in a single commit, so the partial state cannot arise. Cancelling an already cancelled transaction still reports an error, as db_wallet_tx_rollback expects. --- libwallet/src/contract/slate.rs | 2 +- libwallet/src/internal/tx.rs | 7 ++++-- libwallet/src/internal/updater.rs | 42 +++++++++++++++++-------------- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/libwallet/src/contract/slate.rs b/libwallet/src/contract/slate.rs index bed2118..495e428 100644 --- a/libwallet/src/contract/slate.rs +++ b/libwallet/src/contract/slate.rs @@ -296,7 +296,7 @@ where #[cfg(test)] mod tests { use super::*; - use crate::grin_keychain::{ExtKeychain, Keychain as _, SwitchCommitmentType}; + use crate::grin_keychain::{ExtKeychain, SwitchCommitmentType}; use crate::slate::ParticipantData; fn keychain_and_context() -> (ExtKeychain, Context) { diff --git a/libwallet/src/internal/tx.rs b/libwallet/src/internal/tx.rs index da2b6dd..0b5c075 100644 --- a/libwallet/src/internal/tx.rs +++ b/libwallet/src/internal/tx.rs @@ -354,6 +354,9 @@ where if tx_vec.len() == 0 { return Err(Error::TransactionDoesntExist(tx_id_string)); } + // Collect the entries and their outputs first: the batch that cancels them borrows the + // wallet, and a slate can have several entries which must all be cancelled together. + let mut to_cancel = vec![]; for tx in tx_vec { debug!("cancel_tx: tx: {}", tx.tx_type); match tx.tx_type { @@ -375,9 +378,9 @@ where Some(&parent_key_id), )?; let outputs = res.iter().map(|m| m.output.clone()).collect(); - updater::cancel_tx_and_outputs(wallet, keychain_mask, tx, outputs, parent_key_id)?; + to_cancel.push((tx, outputs)); } - Ok(()) + updater::cancel_txs_and_outputs(wallet, keychain_mask, to_cancel, parent_key_id) } /// Update the stored transaction (this update needs to happen when the TX is finalised) diff --git a/libwallet/src/internal/updater.rs b/libwallet/src/internal/updater.rs index 501c7a5..9049a9e 100644 --- a/libwallet/src/internal/updater.rs +++ b/libwallet/src/internal/updater.rs @@ -484,38 +484,42 @@ where } /// Cancel transaction and associated outputs -pub fn cancel_tx_and_outputs( +pub fn cancel_txs_and_outputs( wallet: &mut WalletBackend, keychain_mask: Option<&SecretKey>, - mut tx: TxLogEntry, - outputs: Vec, + txs: Vec<(TxLogEntry, Vec)>, parent_key_id: &Identifier, ) -> Result<(), Error> where C: NodeClient, K: Keychain, { + // A single slate can have several tx log entries, so they are all cancelled in one + // batch: committing per entry would let a failure part way through leave some of them + // cancelled and the rest untouched. let mut batch = wallet.batch(keychain_mask)?; - for mut o in outputs { - // unlock locked outputs - if o.status == OutputStatus::Unconfirmed || o.status == OutputStatus::Reverted { - batch.delete(&o.key_id, &o.mmr_index)?; + for (mut tx, outputs) in txs { + for mut o in outputs { + // unlock locked outputs + if o.status == OutputStatus::Unconfirmed || o.status == OutputStatus::Reverted { + batch.delete(&o.key_id, &o.mmr_index)?; + } + if o.status == OutputStatus::Locked { + o.status = OutputStatus::Unspent; + batch.save(o)?; + } } - if o.status == OutputStatus::Locked { - o.status = OutputStatus::Unspent; - batch.save(o)?; + match tx.tx_type { + TxLogEntryType::TxSent => tx.tx_type = TxLogEntryType::TxSentCancelled, + TxLogEntryType::TxReceived | TxLogEntryType::TxReverted => { + tx.tx_type = TxLogEntryType::TxReceivedCancelled + } + TxLogEntryType::TxSelfSpend => tx.tx_type = TxLogEntryType::TxSelfSpendCancelled, + _ => {} } + batch.save_tx_log_entry(tx, parent_key_id)?; } - match tx.tx_type { - TxLogEntryType::TxSent => tx.tx_type = TxLogEntryType::TxSentCancelled, - TxLogEntryType::TxReceived | TxLogEntryType::TxReverted => { - tx.tx_type = TxLogEntryType::TxReceivedCancelled - } - TxLogEntryType::TxSelfSpend => tx.tx_type = TxLogEntryType::TxSelfSpendCancelled, - _ => {} - } - batch.save_tx_log_entry(tx, parent_key_id)?; batch.commit()?; Ok(()) }