From 7c0afd06a8669d145ae1d09c95aa82e2afeca476 Mon Sep 17 00:00:00 2001 From: DanS Date: Sun, 11 Oct 2026 06:05:06 -0500 Subject: [PATCH] fix(lite-backend): record the mined height of mempool-first transactions When the mempool monitor saw a transaction first, its wallet entry kept the monitor's height (often 0) after the transaction was mined, because scanning the block reused the entry without updating its block. A later reorg between the two heights then left a phantom spend of our own send and a change note with an empty witness list until a rescan. Scanning a block now sets the entry's block to the block height. This applies to every monitor-first transaction, including payments received from others: their reported block height, the verified/spendable timing that depends on it, and the wallet birthday now use the real mined height instead of the monitor's. Co-Authored-By: Claude Opus 5.5 --- .../lib/src/lightwallet.rs | 4 + .../lib/src/lightwallet/oracle_tests.rs | 201 ++++++++++++++++-- .../lib/src/lightwallet/tests.rs | 38 ++++ 3 files changed, 231 insertions(+), 12 deletions(-) diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs index cec1bac..452a486 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs @@ -2398,6 +2398,10 @@ pub fn scan_full_mempool_tx(&self, tx: &Transaction, height: i32, _datetime: u64 txs.insert(tx.txid, tx_entry); } let tx_entry = txs.get_mut(&tx.txid).unwrap(); + // An existing entry can come from the mempool monitor (scan_full_mempool_tx), stamped with the + // height the server reported before the tx was mined. Record the block that mined it: a reorg of + // this block must remove the tx (invalidate_block and the witness clearing key on this field). + tx_entry.block = block_data.height; tx_entry.total_shielded_value_spent = total_shielded_value_spent; // Save notes. 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 5ccc766..af2caac 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 @@ -44,7 +44,10 @@ //! - reloads, which drop `unconfirmed_spent` (the send may still be mined afterwards; until it //! is, the generator never spends its notes); //! - probes on throwaway copies, which never know the flag, so they may spend a pending note; -//! - after the send is mined at M, reorgs with T + 2 <= at <= M that un-spend its notes. +//! - after the send is mined at M, reorgs with T + 2 <= at <= M that un-spend its notes; +//! - the mempool monitor (`scan_full_mempool_tx`) recording the send in `txs` at a server height h_m +//! (0, lagging, the tip or tip + 1) before it is mined. The block M that mines it must become its +//! height, and reorgs with h_m < at <= M must remove it (R3). //! //! A run that leaves a send unmined orphans it at once. Pending notes' witnesses are advanced //! and compared like any unspent note's (F5b); a stale one fails the next check. @@ -384,6 +387,12 @@ struct Stats { pending_probes: usize, /// Reorgs that un-mined our send from a height >= sent tip + 2 (its notes become spendable). unmine_reorgs: usize, + /// Real sends that the mempool monitor recorded in `txs` before they were mined (R3). + monitor_feeds: usize, + /// Fed sends mined at a height other than the monitor's (the entry must move to the mining block). + monitor_moved: usize, + /// Reorgs with h_m < at <= M of a fed send mined at M (the send must go with its mining block). + r3_reorgs: usize, t_gen: f64, t_scan: f64, t_ref: f64, @@ -417,6 +426,12 @@ struct Sim { pending: Option, /// (txid, tip when sent, anchor height) of the last real send. last_send: Option<(TxId, i32, i32)>, + /// The last real send's tx (kept after it is mined or orphaned; the mempool monitor may still see it). + last_gtx: Option, + /// (txid, h_m): the mempool monitor recorded the last real send in `txs` at server height h_m (R3). + fed: Option<(TxId, i32)>, + /// (txid, height) of the last wallet-built tx mined on the current branch. + last_mined: Option<(TxId, i32)>, /// Overrides the coin flip that orphans a pending send whose anchor block was popped. force_orphan: Option, /// Blocks still to be paid to us (setup helper). @@ -500,6 +515,9 @@ impl Sim { mempool: HashMap::new(), pending: None, last_send: None, + last_gtx: None, + fed: None, + last_mined: None, force_orphan: None, force_pay: 0, force_output: false, @@ -792,6 +810,10 @@ impl Sim { // Full-tx scan of mined wallet-built txs (memos, outgoing metadata). for g in &txs { if let Some(raw) = &g.raw { + self.last_mined = Some((g.txid(), h)); + if self.fed.map_or(false, |(t, hm)| t == g.txid() && hm != h) { + self.stats.monitor_moved += 1; + } let tx = Transaction::read(&raw[..]).unwrap(); self.w.scan_full_tx(&tx, h, 0); let txs_w = self.w.txs.read().unwrap(); @@ -918,7 +940,9 @@ impl Sim { } let ids: HashSet = txs.iter().map(|g| g.txid()).collect(); - self.mempool.retain(|t, b| !ids.contains(t) && h < *b + TX_EXPIRY_DELTA); + // 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); let rel_txs: Vec = txs.iter().filter(|g| self.rel.get(&g.txid()).map_or(false, |r| r.0 == h)) .cloned().collect(); @@ -945,10 +969,15 @@ impl Sim { let class = if at > t { 0 } else if at > a { 1 } else { 2 }; self.stats.pending_reorgs[class] += 1; } - if at >= t + 2 && self.rel.get(&txid).map_or(false, |r| r.0 >= at) { + if at >= t + 2 && self.last_mined.map_or(false, |(t2, m)| t2 == txid && m >= at) { self.stats.unmine_reorgs += 1; } } + if let (Some((ftx, hm)), Some((mtx, m))) = (self.fed, self.last_mined) { + if ftx == mtx && hm < at && at <= m { + self.stats.r3_reorgs += 1; + } + } let n = if stepwise { (0..k).map(|_| self.w.invalidate_block(self.w.last_scanned_height())).sum::() } else { @@ -1011,6 +1040,12 @@ impl Sim { } } self.rel.retain(|_, r| r.0 < at); + if self.last_mined.map_or(false, |(_, m)| m >= at) { + self.last_mined = None; + } + if self.fed.map_or(false, |(t, _)| !self.rel.contains_key(&t)) { + self.fed = None; + } // The new branch may re-mine the dropped spends at other heights, or drop them. let mut dropped = vec![]; @@ -1085,7 +1120,10 @@ impl Sim { /// its notes become spendable again with the witnesses of the blocks where they were pending. fn unmine_reorg_depth(&mut self) -> Option { let (txid, t, _) = self.last_send?; - let m = self.rel.get(&txid)?.0; + let (mtx, m) = self.last_mined?; + if mtx != txid { + return None; + } let tip = self.tip(); let lo = cmp::max(t + 2, *self.win.front().unwrap() + 1); let hi = cmp::min(m, tip); @@ -1096,6 +1134,24 @@ impl Sim { Some((tip - at + 1) as usize) } + /// A reorg depth with `at` in (h_m, M]: it un-mines the last real send, which the mempool monitor recorded + /// at h_m and block M mined (R3: the tx must go with its mining block). + fn r3_reorg_depth(&mut self) -> Option { + let (ftx, hm) = self.fed?; + let (mtx, m) = self.last_mined?; + if ftx != mtx { + return None; + } + let tip = self.tip(); + let lo = cmp::max(hm + 1, *self.win.front().unwrap() + 1); + let hi = cmp::min(m, tip); + if lo > hi { + return None; + } + let at = self.rng.gen_range(lo, hi + 1); + Some((tip - at + 1) as usize) + } + /// Orphan the pending send: it is never mined, and its notes stay pending until a reload. fn orphan_pending(&mut self) { if self.pending.take().is_some() { @@ -1103,6 +1159,33 @@ impl Sim { } } + /// The mempool monitor sees the last real send at server height `h_m` (scan_full_mempool_tx). The wallet + /// records it in `txs` at h_m when one of its outputs is ours, and drops it from `mempool_txs` at the next + /// block. MUST be followed by `mine` before any check: the feed also fills `incoming_mempool_txs` + /// (unconfirmed_zbalance), which that block's cleanup clears. + fn monitor_feed(&mut self, h_m: i32) -> bool { + self.op = "monitor"; + let g = match &self.last_gtx { + Some(g) => g.clone(), + None => return false, + }; + let txid = g.txid(); + if self.rel.contains_key(&txid) { + return false; // already recorded, or mined on this branch + } + let tx = Transaction::read(&g.raw.as_ref().unwrap()[..]).unwrap(); + self.w.scan_full_mempool_tx(&tx, h_m, 0, true); + let ours = g.ours.iter().any(|o| o.is_some()); + let got = self.w.txs.read().unwrap().get(&txid).map(|w| (w.block, w.notes.len(), w.full_tx_scanned)); + assert!(got == if ours { Some((h_m, 0, true)) } else { None }, "{} monitor entry {:?}", self.ctx(), got); + if ours { + self.rel.insert(txid, (h_m, 0)); + self.fed = Some((txid, h_m)); + self.stats.monitor_feeds += 1; + } + ours + } + fn reload(&mut self) { self.op = "reload"; self.reload_raw(); @@ -1316,6 +1399,8 @@ impl Sim { raw: Some(raw), anchor_height: Some(tip - off), }; + self.last_gtx = Some(gtx.clone()); + self.fed = None; self.pending = Some(Pending { gtx, txid }); self.last_send = Some((txid, tip, tip - off)); self.stats.sends += 1; @@ -1511,9 +1596,10 @@ impl Sim { impl Stats { fn pending_summary(&self) -> String { format!("pending: reorgs above/across/popped {}/{}/{}, orphaned {}, anchor-root reuse {}, reloads {}, \ - probes {}, un-mining reorgs {}", + probes {}, un-mining reorgs {}, monitor feeds {}, monitor moved {}, R3 reorgs {}", self.pending_reorgs[0], self.pending_reorgs[1], self.pending_reorgs[2], self.orphaned, - self.anchor_root_reuse, self.pending_reloads, self.pending_probes, self.unmine_reorgs) + self.anchor_root_reuse, self.pending_reloads, self.pending_probes, self.unmine_reorgs, + self.monitor_feeds, self.monitor_moved, self.r3_reorgs) } /// Add the pending-state counters of one seed. @@ -1526,12 +1612,16 @@ impl Stats { self.pending_reloads += o.pending_reloads; self.pending_probes += o.pending_probes; self.unmine_reorgs += o.unmine_reorgs; + self.monitor_feeds += o.monitor_feeds; + self.monitor_moved += o.monitor_moved; + self.r3_reorgs += o.r3_reorgs; } /// Every pending-state class was hit (asserted by the full runs). fn assert_pending_cover(&self) { let all = [self.pending_reorgs[0], self.pending_reorgs[1], self.pending_reorgs[2], self.orphaned, - self.anchor_root_reuse, self.pending_reloads, self.pending_probes, self.unmine_reorgs]; + self.anchor_root_reuse, self.pending_reloads, self.pending_probes, self.unmine_reorgs, + self.monitor_feeds, self.monitor_moved, self.r3_reorgs]; assert!(all.iter().all(|&c| c > 0), "a pending-state class was never hit: {}", self.pending_summary()); } } @@ -1649,16 +1739,19 @@ fn run_seed(seed: u64, n_blocks: usize, proofs: Proofs, v: &Verifier) -> Stats { }, } // Rates, per real send, of (a) a reorg, (b) a reload and (c) a probe inside its pending window, - // and (d) a reorg that un-mines it again. - let (p_reorg, p_reload, p_probe, p_unmine) = match proofs { - Proofs::Three => (0.4, 0.3, 0.3, 0.4), - Proofs::One => (0.9, 0.5, 0.7, 0.8), + // (d) a reorg that un-mines it again, (e) a mempool-monitor feed of it before it is mined, and + // (f) once a fed send is mined at M, a reorg with h_m < at <= M (R3). + let (p_reorg, p_reload, p_probe, p_unmine, p_feed, p_r3) = match proofs { + Proofs::Three => (0.4, 0.3, 0.3, 0.4, 0.5, 0.5), + Proofs::One => (0.9, 0.5, 0.7, 0.8, 0.8, 0.8), }; let mut insufficient_at = Some(sim.rng.gen_range(0, n)); let mut deep_reorg_at = if n > 150 && sim.rng.gen_bool(0.4) { Some(sim.rng.gen_range(120, n)) } else { None }; let mut mine_at: Option = None; let (mut pending_reorg_at, mut pending_reload_at, mut pending_probe_at, mut unmine_at): (Option, Option, Option, Option) = (None, None, None, None); + let mut feed_at: Option = None; + let mut r3_at: Option = None; for step in 0..n { sim.step = step; @@ -1669,6 +1762,9 @@ fn run_seed(seed: u64, n_blocks: usize, proofs: Proofs, v: &Verifier) -> Stats { } else if unmine_at == Some(step) { unmine_at = None; sim.unmine_reorg_depth() + } else if r3_at == Some(step) { + r3_at = None; + sim.r3_reorg_depth() } else if deep_reorg_at.map_or(false, |s| step >= s) && sim.win.len() > 91 { deep_reorg_at = None; Some(cmp::min(sim.rint(90, 100) as usize, sim.win.len() - 1)) @@ -1726,19 +1822,37 @@ fn run_seed(seed: u64, n_blocks: usize, proofs: Proofs, v: &Verifier) -> Stats { if sim.rng.gen_bool(p_probe) { pending_probe_at = Some(step + sim.rint(1, span) as usize); } + if sim.rng.gen_bool(p_feed) { + feed_at = Some(if unmined { step } else { step + sim.rint(0, span) as usize }); + } } } if insufficient_at.map_or(false, |s| step >= s) { insufficient_at = None; sim.insufficient_check(); } + if feed_at.map_or(false, |s| step >= s) { + feed_at = None; + // The height the server streams: its tip, one above ours, a lagging one, or 0. + let tip = sim.tip(); + let hm = match sim.rng.gen_range(0, 10) { + 0 => 0, + 1..=2 => cmp::max(sim.last_send.map_or(tip, |(_, t, _)| t), tip - sim.rint(1, 3) as i32), + 3..=5 => tip + 1, + _ => tip, + }; + sim.monitor_feed(hm); // the mine below follows at once + } if mine_at.map_or(false, |s| step >= s) { mine_at = None; // A reorg may have orphaned it. if let Some(p) = sim.pending.take() { + let fed = sim.fed.map_or(false, |(t, _)| t == p.txid); sim.mine(vec![p.gtx], true); - if sim.rng.gen_bool(p_unmine) { + if fed && sim.rng.gen_bool(p_r3) { + r3_at = Some(step + sim.rint(1, 6) as usize); + } else if sim.rng.gen_bool(p_unmine) { unmine_at = Some(step + sim.rint(1, 6) as usize); } } else { @@ -2160,3 +2274,66 @@ fn oracle_f5a_reorg_pops_anchor_then_reload_send() { assert!(spent == want, "the second send spends exactly the formerly pending note q"); sim.mine(vec![], true); } + +/// R3 (regression test). The mempool monitor records our send in `txs` at the server's height h_m +/// (scan_full_mempool_tx). scan_block must move the entry to the block that mines it (M): a reorg in +/// (h_m, M] removes the send, un-spends its notes and drops its change note. Before the fix the entry kept +/// h_m, so such a reorg left a phantom spend and a change note with no witnesses. +fn r3_case(seed: u64, hm_of: fn(i32) -> i32) { + let v = Verifier::new(); + let mut sim = Sim::new(seed); + sim.force_pay = 12; + for _ in 0..12 { + sim.mine(vec![], true); + } + // One input and a change output of 1, so the monitor can decrypt an output. + let (_, off) = sim.anchor(sim.cfg.anchor_offset); + let fee = u64::from(DEFAULT_FEE); + let vq = sim.notes.iter().filter(|n| n.live() && n.has_sk && n.len >= off as usize + 1 && n.value > fee + 1) + .map(|n| n.value).min().expect("setup: a note to spend"); + let spent = sim.send_with(&v, SendKind::Real, Some(vq - fee - 1)).expect("setup: the send must be built"); + let txid = sim.pending.as_ref().unwrap().txid; + let hm = hm_of(sim.tip()); + assert!(sim.monitor_feed(hm), "setup: the monitor recorded the send"); + sim.mine(vec![], true); // a block without it: M >= h_m + 2 when h_m = T + let g = sim.pending.take().unwrap().gtx; + let mined = panic::catch_unwind(AssertUnwindSafe(|| sim.mine(vec![g.clone()], true))); + let m = sim.tip(); + let blk = sim.w.txs.read().unwrap().get(&txid).map(|w| w.block); + assert_eq!(blk, Some(m), "R3 (regression): our send, recorded by the mempool monitor at {}, was mined at {} \ + but its WalletTx keeps the monitor's height", hm, m); + assert!(mined.is_ok(), "mining the monitor-recorded send failed a check"); + sim.mine(vec![], true); + + // A reorg of the mining block, above h_m: the send must go with it. + let k = (sim.tip() - m + 1) as usize; + assert_eq!(sim.w.invalidate_block(m), k as u64); + sim.ref_invalidate(m, k); + sim.requeue.clear(); + { + let txs = sim.w.txs.read().unwrap(); + assert!(txs.get(&txid).is_none(), "R3 (regression): a reorg of its mining block kept the send"); + let all: Vec<_> = txs.values().flat_map(|w| w.notes.iter()).collect(); + assert!(all.iter().filter(|nd| spent.contains(&nd.nullifier)).all(|nd| nd.spent.is_none()), + "R3 (regression): phantom spend: the inputs stay spent by a tx that is no longer on chain"); + assert!(!all.iter().any(|nd| nd.spent.is_none() && nd.witnesses.is_empty()), + "R3 (regression): an unspent note (the send's change) has no witnesses"); + } + sim.check(true); + + // The new branch mines it again (its anchor block is kept): one entry, at the new height. + sim.mine(vec![g], true); + assert_eq!(sim.w.txs.read().unwrap().get(&txid).map(|w| w.block), Some(sim.tip()), "re-mined send's block"); + sim.mine(vec![], true); + sim.check(true); +} + +#[test] +fn oracle_r3_monitor_entry_mined_then_reorg() { + r3_case(0x3A0, |t| t); +} + +#[test] +fn oracle_r3_monitor_entry_at_height_zero() { + r3_case(0x3A1, |_| 0); +} 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 2d80ee4..a657dd3 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs @@ -917,6 +917,44 @@ fn test_self_txns_ttoz_nomemo() { } } +/// R3 (regression test). The mempool monitor (scan_full_mempool_tx) records our send in `txs` at the height +/// the server reports. The block that mines it must become its height: a reorg of that block removes the +/// send, un-spends its note and drops its change. Before the fix the entry kept the monitor's height. +#[test] +fn test_r3_mempool_seen_send_takes_mined_height() { + const AMOUNT1: u64 = 50000; + let (wallet, txid1, block_hash) = get_test_wallet(AMOUNT1); // note at block 0, tip 1 + let fvk = ExtendedFullViewingKey::from(&ExtendedSpendingKey::master(&[1u8; 32])); + let ext_address = encode_payment_address(wallet.config.hrp_sapling_address(), &fvk.default_address().unwrap().1); + let branch_id = u32::from_str_radix("76b809bb", 16).unwrap(); + let (ss, so) = get_sapling_params().unwrap(); + let (_, raw_tx) = wallet.send_to_address(branch_id, &ss, &so, false, + vec![(&ext_address, 20, None)], &default_fee(), |_| Ok(' '.to_string())).unwrap(); + let sent_tx = Transaction::read(&raw_tx[..]).unwrap(); + let sent_txid = sent_tx.txid(); + + wallet.scan_full_mempool_tx(&sent_tx, 1, 0, true); // decrypts the change: entry at the server height 1 + assert_eq!(wallet.txs.read().unwrap().get(&sent_txid).map(|t| t.block), Some(1), "setup: monitor entry"); + + let cb2 = FakeCompactBlock::new(2, block_hash); // not in block 2 + wallet.scan_block(&cb2.as_bytes()).unwrap(); + let mut cb3 = FakeCompactBlock::new(3, cb2.hash()); // mined in block 3 + cb3.add_tx(&sent_tx); + wallet.scan_block(&cb3.as_bytes()).unwrap(); + assert_eq!(wallet.txs.read().unwrap()[&sent_txid].block, 3, + "R3 (regression): mined at 3, but the WalletTx keeps the mempool monitor's height 1"); + + assert_eq!(wallet.invalidate_block(3), 1); + { + let txs = wallet.txs.read().unwrap(); + assert!(txs.get(&sent_txid).is_none(), "R3 (regression): a reorg of its mining block kept the send"); + assert_eq!(txs[&txid1].notes[0].spent, None, "R3 (regression): phantom spend"); + assert_eq!(txs[&txid1].notes[0].unconfirmed_spent, None); + assert!(!txs[&txid1].notes[0].witnesses.is_empty()); + } + assert_eq!(wallet.zbalance(None), AMOUNT1); // guard dropped first: zbalance takes txs.read itself +} + #[test] fn test_self_txns_ztoz() { const AMOUNT1: u64 = 50000;