fix(lite-backend): don't panic when a reorg lands during a send

After broadcasting, the send path looked up the spent notes' and UTXOs' transactions with unwrap. A reorg while the transaction was being proved could remove one of them, so the backend panicked and poisoned the wallet lock although the transaction was already broadcast. The lookups are now `if let` with a warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-11 06:05:06 -05:00
parent 7c0afd06a8
commit 22e3f104d6
3 changed files with 110 additions and 10 deletions
@@ -2747,22 +2747,31 @@ pub fn scan_full_mempool_tx(&self, tx: &Transaction, height: i32, _datetime: u64

        // Mark notes as spent.
        {
-            // Mark sapling notes as unconfirmed spent
+            // Mark sapling notes as unconfirmed spent. The tx is already broadcast, so never panic here: a
+            // reorg or a rescan (clear_blocks) while it was proved can have removed the tx that holds a
+            // selected note or utxo. Skip those with a warning (a panic would poison `txs`; the send went out).
            let mut txs = self.txs.write().unwrap();
            for selected in notes {
-                let mut spent_note = txs.get_mut(&selected.txid).unwrap()
-                                        .notes.iter_mut()
-                                        .find(|nd| &nd.nullifier[..] == &selected.nullifier[..])
-                                        .unwrap();
-                spent_note.unconfirmed_spent = Some(tx.txid());
+                if let Some(spent_note) = txs.get_mut(&selected.txid)
+                    .and_then(|wtx| wtx.notes.iter_mut().find(|nd| &nd.nullifier[..] == &selected.nullifier[..]))
+                {
+                    spent_note.unconfirmed_spent = Some(tx.txid());
+                } else {
+                    warn!("Sent {}, but its input note from tx {} is no longer in the wallet (reorg or rescan); not marked pending",
+                          tx.txid(), selected.txid);
+                }
            }

            // Mark this utxo as unconfirmed spent
            for utxo in tinputs {
-                let mut spent_utxo = txs.get_mut(&utxo.txid).unwrap().utxos.iter_mut()
-                                        .find(|u| utxo.txid == u.txid && utxo.output_index == u.output_index)
-                                        .unwrap();
-                spent_utxo.unconfirmed_spent = Some(tx.txid());
+                if let Some(spent_utxo) = txs.get_mut(&utxo.txid)
+                    .and_then(|wtx| wtx.utxos.iter_mut().find(|u| utxo.txid == u.txid && utxo.output_index == u.output_index))
+                {
+                    spent_utxo.unconfirmed_spent = Some(tx.txid());
+                } else {
+                    warn!("Sent {}, but its input utxo {}:{} is no longer in the wallet (reorg or rescan); not marked pending",
+                          tx.txid(), utxo.txid, utxo.output_index);
+                }
            }
        }

@@ -2337,3 +2337,59 @@ fn oracle_r3_monitor_entry_mined_then_reorg() {
 fn oracle_r3_monitor_entry_at_height_zero() {
    r3_case(0x3A1, |_| 0);
 }
+
+/// U1 (regression test). A reorg that lands while a send is proved can remove the tx that holds a selected
+/// note. send_to_address marks its inputs pending after the broadcast and unwrapped those lookups, so it
+/// panicked with the tx already broadcast and poisoned `txs`.
+#[test]
+fn oracle_u1_reorg_during_send() {
+    let v = Verifier::new();
+    let mut sim = Sim::new(0x51A);
+    sim.force_pay = 12;
+    for _ in 0..12 {
+        sim.mine(vec![], true);
+    }
+    let (tip, off) = sim.anchor(sim.cfg.anchor_offset);
+    let fee = u64::from(DEFAULT_FEE);
+    let cands: Vec<(u64, [u8; 32])> = sim.notes.iter()
+        .filter(|n| n.live() && n.has_sk && n.len >= off as usize + 1).map(|n| (n.value, n.nf)).collect();
+    // The newest eligible note that the selection rule spends alone for amount = its value - fee.
+    let (q, vq, at) = sim.notes.iter()
+        .filter(|n| n.live() && n.has_sk && n.len >= off as usize + 1 && n.value > fee)
+        .filter(|n| expected_selection(&cands, n.value) == Some(vec![n.nf]))
+        .max_by_key(|n| n.recv).map(|n| (n.nf, n.value, n.recv)).expect("setup: a note spent alone");
+    let k = (tip - at + 1) as usize;
+    assert!(k < sim.win.len());
+    let anchor = Fr::from(sim.block_at(tip - off).root);
+    let eligible: HashSet<[u8; 32]> = [q].iter().cloned().collect();
+    let to = encode_payment_address(sim.cfg.hrp_sapling_address(), &sim.foreign[0].1);
+    let (ss, so) = (SaplingParams::get("sapling-spend.params").unwrap(),
+                    SaplingParams::get("sapling-output.params").unwrap());
+    let popped = Cell::new(0u64);
+    let r = {
+        let w = &sim.w;
+        panic::catch_unwind(AssertUnwindSafe(|| {
+            w.send_to_address(BRANCH_ID, &ss, &so, false, vec![(to.as_str(), vq - fee, None)], &fee, |_raw| {
+                popped.set(w.invalidate_block(at)); // the reorg lands between proving and the bookkeeping
+                Ok(NOT_BROADCAST.to_string())
+            })
+        }))
+    };
+    assert!(r.is_ok() && !sim.w.txs.is_poisoned(),
+            "U1 (regression): a reorg removed the tx of a selected note while the send was proved; \
+             send_to_address panicked after the broadcast and poisoned the wallet's txs lock");
+    let (_, raw) = r.unwrap().expect("the broadcast send returns Ok");
+    assert_eq!(popped.get(), k as u64);
+    let tx = verify_tx(&v, &raw, anchor, &eligible, fee as i64, (tip + TX_EXPIRY_DELTA) as u32, "u1");
+    assert!(tx.shielded_spends.len() == 1 && tx.shielded_spends[0].nullifier == q);
+
+    // Reference: the reorg removed q (the send can never be mined); the wallet still records the send.
+    sim.ref_invalidate(at, k);
+    sim.requeue.clear();
+    sim.mempool.insert(tx.txid(), tip);
+    assert!(sim.w.txs.read().unwrap().values().flat_map(|w| w.notes.iter()).all(|nd| nd.unconfirmed_spent.is_none()));
+    sim.check(true);
+    for _ in 0..3 {
+        sim.mine(vec![], true);
+    }
+}
@@ -955,6 +955,41 @@ fn test_r3_mempool_seen_send_takes_mined_height() {
    assert_eq!(wallet.zbalance(None), AMOUNT1); // guard dropped first: zbalance takes txs.read itself
 }

+/// U1 (regression test). A reorg that lands while a send is proved can remove the tx that holds a selected
+/// utxo. send_to_address marks its inputs pending after the broadcast and unwrapped that lookup, so it
+/// panicked with the tx already broadcast and poisoned the wallet's `txs` lock.
+#[test]
+fn test_u1_reorg_during_send_removes_utxo_tx() {
+    let mut rng = OsRng;
+    let secp = Secp256k1::new();
+    let (wallet, _txid0, _hash) = get_test_wallet(0); // zero-value note at block 0: never selected
+    let pk = PublicKey::from_secret_key(&secp, &wallet.tkeys.read().unwrap()[0].tkey.unwrap());
+    let mut tx = FakeTransaction::new(&mut rng);
+    tx.add_t_output(&pk, 50000);
+    let txid1 = tx.get_tx().txid();
+    wallet.scan_full_tx(&tx.get_tx(), 1, 0); // utxo tx at block 1
+    assert_eq!(wallet.txs.read().unwrap()[&txid1].utxos.len(), 1);
+
+    let zaddr = wallet.add_zaddr();
+    let branch_id = u32::from_str_radix("2bb40e60", 16).unwrap();
+    let (ss, so) = get_sapling_params().unwrap();
+    let popped = std::cell::Cell::new(0u64);
+    let r = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
+        wallet.send_to_address(branch_id, &ss, &so, false, vec![(&zaddr, 20, None)], &default_fee(), |_| {
+            popped.set(wallet.invalidate_block(1)); // lands between proving and the bookkeeping
+            Ok(' '.to_string())
+        })
+    }));
+    assert!(r.is_ok() && !wallet.txs.is_poisoned(),
+            "U1 (regression): a reorg removed the tx of a selected utxo while the send was proved; \
+             send_to_address panicked after the broadcast and poisoned the wallet's txs lock");
+    let (_, raw_tx) = r.unwrap().expect("the broadcast send returns Ok");
+    let sent = Transaction::read(&raw_tx[..]).unwrap();
+    assert_eq!(popped.get(), 1);
+    assert!(wallet.txs.read().unwrap().get(&txid1).is_none());
+    assert!(wallet.mempool_txs.read().unwrap().contains_key(&sent.txid()));
+}
+
 #[test]
 fn test_self_txns_ztoz() {
    const AMOUNT1: u64 = 50000;