diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs index 452a486..365c2f0 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs @@ -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); + } } } 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 af2caac..9b2a3a6 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 @@ -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); + } +} diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs index a657dd3..7c71622 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs @@ -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;