fix(lite-backend): release the pending lock when a send expires; un-spend reorged-out UTXOs

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 <noreply@anthropic.com>
2026-10-19 18:13:33 -05:00
parent 16a1ca02e8
commit 8a6d523062
3 changed files with 69 additions and 8 deletions
@@ -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<TxId> = 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, _| {
@@ -94,7 +94,6 @@ pub struct SaplingNoteData {
    pub unconfirmed_spent: Option<TxId>, // If this note was spent in a send, but has not yet been confirmed.
    pub memo:  Option<Memo>,
    pub is_change: bool,
-    // TODO: We need to remove the unconfirmed_spent (i.e., set it to None) if the Tx has expired
 }


@@ -987,7 +987,28 @@ impl Sim {
        let ids: HashSet<TxId> = 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<TxId> = 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<GenTx> = txs.iter().filter(|g| self.rel.get(&g.txid()).map_or(false, |r| r.0 == h))
            .cloned().collect();