From 8a6d523062ebb08132fb84469b01894f4a5afc99 Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 19 Oct 2026 18:13:33 -0500 Subject: [PATCH] fix(lite-backend): release the pending lock when a send expires; un-spend reorged-out UTXOs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resilience audit, vendored SDXL core (post-hoc wallet bookkeeping only — the send/build/broadcast path, note/UTXO selection, fees, recipients, memo and the post-broadcast pending-marking are byte-identical): - cleanup_mempool never cleared unconfirmed_spent, so a send that was dropped / never mined / reorged out locked its notes and UTXOs until the app was reloaded ("insufficient funds" on a funded wallet). It now releases the lock once the send is provably dead. A send is built with Builder::new(tip) (expiry = tip+20, mineable through tip+20), so the lock is held until the chain reaches tip+21 (MEMPOOL_KEEP = DELTA+1) — one block past the old mempool-drop point, which would have released the notes while the tx could still be mined. A txid already in `txs` was mined (scan settled its inputs) and is skipped. mempool_txs is released before `txs` is taken, so the two locks are never nested. - invalidate_block reset nd.spent on reorged-out spenders but never walked wtx.utxos, so a transparent UTXO whose spender was reorged out kept spent=Some(removed txid) and dropped out of the balance until a rescan. It now resets utxo.spent the same way (unconfirmed_spent left as-is, as for notes — the send may be re-mined on the new branch). - Removed the stale "TODO remove unconfirmed_spent on expiry" in data.rs (now done). Verified with the scratch-mirror rtest.sh (cargo 1.63, scratch HOME): cargo test --release --lib green, 129 passed / 0 failed / 12 ignored, including the oracle differential tests (updated in lockstep to clear the sim's pending flag at the same expiry point) and test_z_mempool_expiry. The C-ABI wrapper and the artifact rebuild / lite_smoke (needs a live server) were not run; they don't touch these functions. Co-Authored-By: Claude Opus 4.8 --- .../lib/src/lightwallet.rs | 53 ++++++++++++++++--- .../lib/src/lightwallet/data.rs | 1 - .../lib/src/lightwallet/oracle_tests.rs | 23 +++++++- 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs index 78f8332..a51a2d2 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs @@ -2023,9 +2023,19 @@ pub fn scan_full_mempool_tx(&self, tx: &Transaction, height: i32, _datetime: u64 } // unconfirmed_spent is left as it is: a pending send can still be mined on the - // new branch. Only a block that spends the note (scan_block) or a reload clears - // it, the same as for a send that expires unmined. - }) + // new branch. Only a block that spends the note (scan_block), the send expiring + // unmined (cleanup_mempool) or a reload clears it. + }); + + // Same for transparent utxos: one whose spender was reorged out is unspent again + // (else it stays spent=Some(removed txid) and drops out of the balance until a rescan). + wtx.utxos.iter_mut() + .for_each(|utxo| { + if utxo.spent.is_some() && txids_to_remove.contains(&utxo.spent.unwrap()) { + utxo.spent = None; + } + // unconfirmed_spent is left as it is, for the same reason as on the notes. + }); }) } @@ -3007,16 +3017,47 @@ pub fn scan_full_mempool_tx(&self, tx: &Transaction, height: i32, _datetime: u64 // 2. The Tx has been added to the wallet via a mined block pub fn cleanup_mempool(&self) { const DEFAULT_TX_EXPIRY_DELTA: i32 = 20; + // A send is built with Builder::new(tip) (wtx.block = tip), so its expiry_height is tip + 20 and it can + // still be mined through block tip+20 (consensus rejects it only once the height exceeds the expiry). It + // is provably dead once the chain reaches tip+21, so the pending lock on its inputs must be held until + // then — one block longer than the old drop point (tip+20), which would release the notes while the tx + // could still be mined. The point is that the lock IS released now; previously unconfirmed_spent never was. + const MEMPOOL_KEEP: i32 = DEFAULT_TX_EXPIRY_DELTA + 1; let current_height = self.blocks.read().unwrap().last().map(|b| b.height).unwrap_or(0); + let mut expired: HashSet = HashSet::new(); { - // Remove all expired Txns - self.mempool_txs.write().unwrap().retain( | _, wtx| { - current_height < (wtx.block + DEFAULT_TX_EXPIRY_DELTA) + // Remove all expired Txns, remembering which ones so their inputs can be released below. + self.mempool_txs.write().unwrap().retain( | txid, wtx| { + let keep = current_height < (wtx.block + MEMPOOL_KEEP); + if !keep { + expired.insert(txid.clone()); + } + keep }); } + if !expired.is_empty() { + // An expired send can never be mined: release the notes/utxos it locked (unconfirmed_spent). The + // mempool_txs lock above is released first, so mempool_txs and txs are never held together. + let mut txs = self.txs.write().unwrap(); + // A txid already in txs was mined; scan_block/scan_full_tx settled its inputs, so skip it. + expired.retain(|txid| !txs.contains_key(txid)); + for wtx in txs.values_mut() { + for nd in wtx.notes.iter_mut() { + if nd.unconfirmed_spent.map_or(false, |t| expired.contains(&t)) { + nd.unconfirmed_spent = None; + } + } + for utxo in wtx.utxos.iter_mut() { + if utxo.unconfirmed_spent.map_or(false, |t| expired.contains(&t)) { + utxo.unconfirmed_spent = None; + } + } + } + } + { // Remove all txns where the txid is added to the wallet directly self.mempool_txs.write().unwrap().retain ( |txid, _| { diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/data.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/data.rs index b5c64a2..3b3ab12 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/data.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/data.rs @@ -94,7 +94,6 @@ pub struct SaplingNoteData { pub unconfirmed_spent: Option, // If this note was spent in a send, but has not yet been confirmed. pub memo: Option, pub is_change: bool, - // TODO: We need to remove the unconfirmed_spent (i.e., set it to None) if the Tx has expired } diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/oracle_tests.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/oracle_tests.rs index 3d572e7..5cd845a 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/oracle_tests.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/oracle_tests.rs @@ -987,7 +987,28 @@ impl Sim { let ids: HashSet = txs.iter().map(|g| g.txid()).collect(); // cleanup_mempool also drops a send that `txs` holds (the mempool monitor may have recorded it). let rel = &self.rel; - self.mempool.retain(|t, b| !ids.contains(t) && !rel.contains_key(t) && h < *b + TX_EXPIRY_DELTA); + // Mirror cleanup_mempool: it keeps a send for MEMPOOL_KEEP (= TX_EXPIRY_DELTA + 1, i.e. one block past the + // tx's expiry height, when it is provably dead) past the sending tip, and for a send dropped purely by + // expiry (never mined, i.e. not in `ids`/`rel`/the wallet's `txs`) it now RELEASES the pending lock on the + // inputs. So clear `pending` for those, exactly as the wallet clears `unconfirmed_spent`. The real tx + // expiry stays TX_EXPIRY_DELTA (used at verify_tx below). + const MEMPOOL_KEEP: i32 = TX_EXPIRY_DELTA + 1; + let mut expired: HashSet = HashSet::new(); + self.mempool.retain(|t, b| { + let mined = ids.contains(t) || rel.contains_key(t); + let keep = !mined && h < *b + MEMPOOL_KEEP; + if !keep && !mined { + expired.insert(*t); + } + keep + }); + if !expired.is_empty() { + for n in self.notes.iter_mut() { + if n.pending.map_or(false, |p| expired.contains(&p)) { + n.pending = None; + } + } + } let rel_txs: Vec = txs.iter().filter(|g| self.rel.get(&g.txid()).map_or(false, |r| r.0 == h)) .cloned().collect();