diff --git a/crates/bitcoind_rpc/tests/test_emitter.rs b/crates/bitcoind_rpc/tests/test_emitter.rs index 7eefbc049..2c8b1e11a 100644 --- a/crates/bitcoind_rpc/tests/test_emitter.rs +++ b/crates/bitcoind_rpc/tests/test_emitter.rs @@ -392,7 +392,6 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { get_balance(&recv_chain, &recv_graph)?, Balance { confirmed: SEND_AMOUNT * (ADDITIONAL_COUNT - reorg_count) as u64, - trusted_pending: SEND_AMOUNT * reorg_count as u64, ..Balance::default() }, "reorg_count: {}", diff --git a/crates/chain/src/tx_graph.rs b/crates/chain/src/tx_graph.rs index 13eefd66b..4bc1552bf 100644 --- a/crates/chain/src/tx_graph.rs +++ b/crates/chain/src/tx_graph.rs @@ -110,7 +110,7 @@ use core::{ #[derive(Clone, Debug, PartialEq)] pub struct TxGraph { // all transactions that the graph is aware of in format: `(tx_node, tx_anchors, tx_last_seen)` - txs: HashMap, u64)>, + txs: HashMap, Option)>, spends: BTreeMap>, anchors: BTreeSet<(A, Txid)>, @@ -140,7 +140,7 @@ pub struct TxNode<'a, T, A> { /// The blocks that the transaction is "anchored" in. pub anchors: &'a BTreeSet, /// The last-seen unix timestamp of the transaction as unconfirmed. - pub last_seen_unconfirmed: u64, + pub last_seen_unconfirmed: Option, } impl<'a, T, A> Deref for TxNode<'a, T, A> { @@ -504,7 +504,7 @@ impl TxGraph { ( TxNodeInternal::Partial([(outpoint.vout, txout)].into()), BTreeSet::new(), - 0, + None, ), ); self.apply_update(update) @@ -518,7 +518,7 @@ impl TxGraph { let mut update = Self::default(); update.txs.insert( tx.compute_txid(), - (TxNodeInternal::Whole(tx), BTreeSet::new(), 0), + (TxNodeInternal::Whole(tx), BTreeSet::new(), None), ); self.apply_update(update) } @@ -560,7 +560,7 @@ impl TxGraph { pub fn insert_seen_at(&mut self, txid: Txid, seen_at: u64) -> ChangeSet { let mut update = Self::default(); let (_, _, update_last_seen) = update.txs.entry(txid).or_default(); - *update_last_seen = seen_at; + *update_last_seen = Some(seen_at); self.apply_update(update) } @@ -669,7 +669,7 @@ impl TxGraph { None => { self.txs.insert( txid, - (TxNodeInternal::Whole(wrapped_tx), BTreeSet::new(), 0), + (TxNodeInternal::Whole(wrapped_tx), BTreeSet::new(), None), ); } } @@ -696,8 +696,8 @@ impl TxGraph { for (txid, new_last_seen) in changeset.last_seen { let (_, _, last_seen) = self.txs.entry(txid).or_default(); - if new_last_seen > *last_seen { - *last_seen = new_last_seen; + if Some(new_last_seen) > *last_seen { + *last_seen = Some(new_last_seen); } } } @@ -710,10 +710,10 @@ impl TxGraph { let mut changeset = ChangeSet::::default(); for (&txid, (update_tx_node, _, update_last_seen)) in &update.txs { - let prev_last_seen: u64 = match (self.txs.get(&txid), update_tx_node) { + let prev_last_seen = match (self.txs.get(&txid), update_tx_node) { (None, TxNodeInternal::Whole(update_tx)) => { changeset.txs.insert(update_tx.clone()); - 0 + None } (None, TxNodeInternal::Partial(update_txos)) => { changeset.txouts.extend( @@ -721,7 +721,7 @@ impl TxGraph { .iter() .map(|(&vout, txo)| (OutPoint::new(txid, vout), txo.clone())), ); - 0 + None } (Some((TxNodeInternal::Whole(_), _, last_seen)), _) => *last_seen, ( @@ -746,7 +746,10 @@ impl TxGraph { }; if *update_last_seen > prev_last_seen { - changeset.last_seen.insert(txid, *update_last_seen); + changeset.last_seen.insert( + txid, + update_last_seen.expect("checked is greater, so we must have a last_seen"), + ); } } @@ -798,6 +801,13 @@ impl TxGraph { } } + // If no anchors are in best chain and we don't have a last_seen, we can return + // early because by definition the tx doesn't have a chain position. + let last_seen = match last_seen { + Some(t) => *t, + None => return Ok(None), + }; + // The tx is not anchored to a block in the best chain, which means that it // might be in mempool, or it might have been dropped already. // Let's check conflicts to find out! @@ -884,7 +894,7 @@ impl TxGraph { if conflicting_tx.last_seen_unconfirmed > tx_last_seen { return Ok(None); } - if conflicting_tx.last_seen_unconfirmed == *last_seen + if conflicting_tx.last_seen_unconfirmed == Some(last_seen) && conflicting_tx.as_ref().compute_txid() > tx.as_ref().compute_txid() { // Conflicting tx has priority if txid of conflicting tx > txid of original tx @@ -893,7 +903,7 @@ impl TxGraph { } } - Ok(Some(ChainPosition::Unconfirmed(*last_seen))) + Ok(Some(ChainPosition::Unconfirmed(last_seen))) } /// Get the position of the transaction in `chain` with tip `chain_tip`. diff --git a/crates/chain/tests/common/tx_template.rs b/crates/chain/tests/common/tx_template.rs index 13bce01ba..3337fb436 100644 --- a/crates/chain/tests/common/tx_template.rs +++ b/crates/chain/tests/common/tx_template.rs @@ -131,9 +131,7 @@ pub fn init_graph<'a, A: Anchor + Clone + 'a>( for anchor in tx_tmp.anchors.iter() { let _ = graph.insert_anchor(tx.compute_txid(), anchor.clone()); } - if let Some(seen_at) = tx_tmp.last_seen { - let _ = graph.insert_seen_at(tx.compute_txid(), seen_at); - } + let _ = graph.insert_seen_at(tx.compute_txid(), tx_tmp.last_seen.unwrap_or(0)); } (graph, spk_index, tx_ids) } diff --git a/crates/chain/tests/test_indexed_tx_graph.rs b/crates/chain/tests/test_indexed_tx_graph.rs index 4266e184a..4014829e2 100644 --- a/crates/chain/tests/test_indexed_tx_graph.rs +++ b/crates/chain/tests/test_indexed_tx_graph.rs @@ -116,8 +116,8 @@ fn insert_relevant_txs() { /// tx1: A Coinbase, sending 70000 sats to "trusted" address. [Block 0] /// tx2: A external Receive, sending 30000 sats to "untrusted" address. [Block 1] /// tx3: Internal Spend. Spends tx2 and returns change of 10000 to "trusted" address. [Block 2] -/// tx4: Mempool tx, sending 20000 sats to "trusted" address. -/// tx5: Mempool tx, sending 15000 sats to "untested" address. +/// tx4: Mempool tx, sending 20000 sats to "untrusted" address. +/// tx5: Mempool tx, sending 15000 sats to "trusted" address. /// tx6: Complete unrelated tx. [Block 3] /// /// Different transactions are added via `insert_relevant_txs`. @@ -160,7 +160,7 @@ fn test_list_owned_txouts() { let mut untrusted_spks: Vec = Vec::new(); { - // we need to scope here to take immutanble reference of the graph + // we need to scope here to take immutable reference of the graph for _ in 0..10 { let ((_, script), _) = graph .index @@ -226,7 +226,7 @@ fn test_list_owned_txouts() { ..common::new_tx(0) }; - // tx5 is spending tx3 and receiving change at trusted keychain, unconfirmed. + // tx5 is an external transaction receiving at trusted keychain, unconfirmed. let tx5 = Transaction { output: vec![TxOut { value: Amount::from_sat(15000), @@ -239,7 +239,7 @@ fn test_list_owned_txouts() { let tx6 = common::new_tx(0); // Insert transactions into graph with respective anchors - // For unconfirmed txs we pass in `None`. + // Insert unconfirmed txs with a last_seen timestamp let _ = graph.batch_insert_relevant([&tx1, &tx2, &tx3, &tx6].iter().enumerate().map(|(i, tx)| { @@ -291,9 +291,6 @@ fn test_list_owned_txouts() { |_, spk: &Script| trusted_spks.contains(&spk.to_owned()), ); - assert_eq!(txouts.len(), 5); - assert_eq!(utxos.len(), 4); - let confirmed_txouts_txid = txouts .iter() .filter_map(|(_, full_txout)| { @@ -359,29 +356,25 @@ fn test_list_owned_txouts() { balance, ) = fetch(0, &graph); + // tx1 is a confirmed txout and is unspent + // tx4, tx5 are unconfirmed assert_eq!(confirmed_txouts_txid, [tx1.compute_txid()].into()); assert_eq!( unconfirmed_txouts_txid, - [ - tx2.compute_txid(), - tx3.compute_txid(), - tx4.compute_txid(), - tx5.compute_txid() - ] - .into() + [tx4.compute_txid(), tx5.compute_txid()].into() ); assert_eq!(confirmed_utxos_txid, [tx1.compute_txid()].into()); assert_eq!( unconfirmed_utxos_txid, - [tx3.compute_txid(), tx4.compute_txid(), tx5.compute_txid()].into() + [tx4.compute_txid(), tx5.compute_txid()].into() ); assert_eq!( balance, Balance { immature: Amount::from_sat(70000), // immature coinbase - trusted_pending: Amount::from_sat(25000), // tx3 + tx5 + trusted_pending: Amount::from_sat(15000), // tx5 untrusted_pending: Amount::from_sat(20000), // tx4 confirmed: Amount::ZERO // Nothing is confirmed yet } @@ -405,23 +398,26 @@ fn test_list_owned_txouts() { ); assert_eq!( unconfirmed_txouts_txid, - [tx3.compute_txid(), tx4.compute_txid(), tx5.compute_txid()].into() + [tx4.compute_txid(), tx5.compute_txid()].into() ); - // tx2 doesn't get into confirmed utxos set - assert_eq!(confirmed_utxos_txid, [tx1.compute_txid()].into()); + // tx2 gets into confirmed utxos set + assert_eq!( + confirmed_utxos_txid, + [tx1.compute_txid(), tx2.compute_txid()].into() + ); assert_eq!( unconfirmed_utxos_txid, - [tx3.compute_txid(), tx4.compute_txid(), tx5.compute_txid()].into() + [tx4.compute_txid(), tx5.compute_txid()].into() ); assert_eq!( balance, Balance { immature: Amount::from_sat(70000), // immature coinbase - trusted_pending: Amount::from_sat(25000), // tx3 + tx5 + trusted_pending: Amount::from_sat(15000), // tx5 untrusted_pending: Amount::from_sat(20000), // tx4 - confirmed: Amount::ZERO // Nothing is confirmed yet + confirmed: Amount::from_sat(30_000) // tx2 got confirmed } ); } @@ -477,6 +473,7 @@ fn test_list_owned_txouts() { balance, ) = fetch(98, &graph); + // no change compared to block 2 assert_eq!( confirmed_txouts_txid, [tx1.compute_txid(), tx2.compute_txid(), tx3.compute_txid()].into() @@ -502,14 +499,14 @@ fn test_list_owned_txouts() { immature: Amount::from_sat(70000), // immature coinbase trusted_pending: Amount::from_sat(15000), // tx5 untrusted_pending: Amount::from_sat(20000), // tx4 - confirmed: Amount::from_sat(10000) // tx1 got matured + confirmed: Amount::from_sat(10000) // tx3 is confirmed } ); } // AT Block 99 { - let (_, _, _, _, balance) = fetch(100, &graph); + let (_, _, _, _, balance) = fetch(99, &graph); // Coinbase maturity hits assert_eq!( diff --git a/crates/chain/tests/test_tx_graph.rs b/crates/chain/tests/test_tx_graph.rs index 21b955d21..7533f6428 100644 --- a/crates/chain/tests/test_tx_graph.rs +++ b/crates/chain/tests/test_tx_graph.rs @@ -977,16 +977,6 @@ fn test_chain_spends() { })) ); - // Even if unconfirmed tx has a last_seen of 0, it can still be part of a chain spend. - assert_eq!( - graph.get_chain_spend( - &local_chain, - tip.block_id(), - OutPoint::new(tx_0.compute_txid(), 1) - ), - Some((ChainPosition::Unconfirmed(0), tx_2.compute_txid())), - ); - // Mark the unconfirmed as seen and check correct ObservedAs status is returned. let _ = graph.insert_seen_at(tx_2.compute_txid(), 1234567); @@ -1099,10 +1089,10 @@ fn update_last_seen_unconfirmed() { let txid = tx.compute_txid(); // insert a new tx - // initially we have a last_seen of 0, and no anchors + // initially we have a last_seen of None and no anchors let _ = graph.insert_tx(tx); let tx = graph.full_txs().next().unwrap(); - assert_eq!(tx.last_seen_unconfirmed, 0); + assert_eq!(tx.last_seen_unconfirmed, None); assert!(tx.anchors.is_empty()); // higher timestamp should update last seen @@ -1117,7 +1107,15 @@ fn update_last_seen_unconfirmed() { let _ = graph.insert_anchor(txid, ()); let changeset = graph.update_last_seen_unconfirmed(4); assert!(changeset.is_empty()); - assert_eq!(graph.full_txs().next().unwrap().last_seen_unconfirmed, 2); + assert_eq!( + graph + .full_txs() + .next() + .unwrap() + .last_seen_unconfirmed + .unwrap(), + 2 + ); } #[test] diff --git a/crates/electrum/tests/test_electrum.rs b/crates/electrum/tests/test_electrum.rs index 1d520df17..9788b2514 100644 --- a/crates/electrum/tests/test_electrum.rs +++ b/crates/electrum/tests/test_electrum.rs @@ -198,17 +198,14 @@ fn tx_can_become_unconfirmed_after_reorg() -> anyhow::Result<()> { .apply_update(update.chain_update) .map_err(|err| anyhow::anyhow!("LocalChain update error: {:?}", err))?; - // Check to see if a new anchor is added during current reorg. - if !initial_anchors.is_superset(update.graph_update.all_anchors()) { - println!("New anchor added at reorg depth {}", depth); - } + // Check that no new anchors are added during current reorg. + assert!(initial_anchors.is_superset(update.graph_update.all_anchors())); let _ = recv_graph.apply_update(update.graph_update); assert_eq!( get_balance(&recv_chain, &recv_graph)?, Balance { confirmed: SEND_AMOUNT * (REORG_COUNT - depth) as u64, - trusted_pending: SEND_AMOUNT * depth as u64, ..Balance::default() }, "reorg_count: {}", diff --git a/crates/wallet/tests/wallet.rs b/crates/wallet/tests/wallet.rs index bf5075e88..0870375bb 100644 --- a/crates/wallet/tests/wallet.rs +++ b/crates/wallet/tests/wallet.rs @@ -1187,7 +1187,16 @@ fn test_create_tx_add_utxo() { version: transaction::Version::non_standard(0), lock_time: absolute::LockTime::ZERO, }; - wallet.insert_tx(small_output_tx.clone()); + let txid = small_output_tx.compute_txid(); + wallet.insert_tx(small_output_tx); + insert_anchor_from_conf( + &mut wallet, + txid, + ConfirmationTime::Confirmed { + height: 2000, + time: 200, + }, + ); let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX") .unwrap() @@ -1195,10 +1204,7 @@ fn test_create_tx_add_utxo() { let mut builder = wallet.build_tx(); builder .add_recipient(addr.script_pubkey(), Amount::from_sat(30_000)) - .add_utxo(OutPoint { - txid: small_output_tx.compute_txid(), - vout: 0, - }) + .add_utxo(OutPoint { txid, vout: 0 }) .unwrap(); let psbt = builder.finish().unwrap(); let sent_received = @@ -1231,8 +1237,16 @@ fn test_create_tx_manually_selected_insufficient() { version: transaction::Version::non_standard(0), lock_time: absolute::LockTime::ZERO, }; - + let txid = small_output_tx.compute_txid(); wallet.insert_tx(small_output_tx.clone()); + insert_anchor_from_conf( + &mut wallet, + txid, + ConfirmationTime::Confirmed { + height: 2000, + time: 200, + }, + ); let addr = Address::from_str("2N1Ffz3WaNzbeLFBb51xyFMHYSEUXcbiSoX") .unwrap() @@ -1240,10 +1254,7 @@ fn test_create_tx_manually_selected_insufficient() { let mut builder = wallet.build_tx(); builder .add_recipient(addr.script_pubkey(), Amount::from_sat(30_000)) - .add_utxo(OutPoint { - txid: small_output_tx.compute_txid(), - vout: 0, - }) + .add_utxo(OutPoint { txid, vout: 0 }) .unwrap() .manually_selected_only(); builder.finish().unwrap(); @@ -1278,7 +1289,9 @@ fn test_create_tx_policy_path_no_csv() { value: Amount::from_sat(50_000), }], }; + let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let external_policy = wallet.policies(KeychainKind::External).unwrap().unwrap(); let root_id = external_policy.id; @@ -1670,6 +1683,7 @@ fn test_bump_fee_irreplaceable_tx() { let tx = psbt.extract_tx().expect("failed to extract tx"); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); wallet.build_fee_bump(txid).unwrap().finish().unwrap(); } @@ -1713,6 +1727,7 @@ fn test_bump_fee_low_fee_rate() { let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_rate(FeeRate::BROADCAST_MIN); @@ -1744,6 +1759,7 @@ fn test_bump_fee_low_abs() { let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_absolute(Amount::from_sat(10)); @@ -1764,6 +1780,7 @@ fn test_bump_fee_zero_abs() { let tx = psbt.extract_tx().expect("failed to extract tx"); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_absolute(Amount::ZERO); @@ -1788,6 +1805,7 @@ fn test_bump_fee_reduce_change() { let tx = psbt.extract_tx().expect("failed to extract tx"); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let feerate = FeeRate::from_sat_per_kwu(625); // 2.5 sat/vb let mut builder = wallet.build_fee_bump(txid).unwrap(); @@ -1884,6 +1902,7 @@ fn test_bump_fee_reduce_single_recipient() { let original_fee = check_fee!(wallet, psbt); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let feerate = FeeRate::from_sat_per_kwu(625); // 2.5 sat/vb let mut builder = wallet.build_fee_bump(txid).unwrap(); @@ -1930,6 +1949,7 @@ fn test_bump_fee_absolute_reduce_single_recipient() { let original_sent_received = wallet.sent_and_received(&tx); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder @@ -2004,6 +2024,7 @@ fn test_bump_fee_drain_wallet() { let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); assert_eq!(original_sent_received.0, Amount::from_sat(25_000)); // for the new feerate, it should be enough to reduce the output, but since we specify @@ -2068,6 +2089,7 @@ fn test_bump_fee_remove_output_manually_selected_only() { let original_sent_received = wallet.sent_and_received(&tx); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); assert_eq!(original_sent_received.0, Amount::from_sat(25_000)); let mut builder = wallet.build_fee_bump(txid).unwrap(); @@ -2114,6 +2136,7 @@ fn test_bump_fee_add_input() { let original_details = wallet.sent_and_received(&tx); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_rate(FeeRate::from_sat_per_vb_unchecked(50)); @@ -2169,6 +2192,7 @@ fn test_bump_fee_absolute_add_input() { let original_sent_received = wallet.sent_and_received(&tx); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_absolute(Amount::from_sat(6_000)); @@ -2233,6 +2257,7 @@ fn test_bump_fee_no_change_add_input_and_change() { let tx = psbt.extract_tx().expect("failed to extract tx"); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); // Now bump the fees, the wallet should add an extra input and a change output, and leave // the original output untouched. @@ -2301,6 +2326,7 @@ fn test_bump_fee_add_input_change_dust() { assert_eq!(tx.output.len(), 2); let txid = tx.compute_txid(); wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); // We set a fee high enough that during rbf we are forced to add @@ -2370,6 +2396,7 @@ fn test_bump_fee_force_add_input() { txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature } wallet.insert_tx(tx.clone()); + wallet.insert_seen_at(txid, 0); // the new fee_rate is low enough that just reducing the change would be fine, but we force // the addition of an extra input with `add_utxo()` let mut builder = wallet.build_fee_bump(txid).unwrap(); @@ -2435,6 +2462,7 @@ fn test_bump_fee_absolute_force_add_input() { txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature } wallet.insert_tx(tx.clone()); + wallet.insert_seen_at(txid, 0); // the new fee_rate is low enough that just reducing the change would be fine, but we force // the addition of an extra input with `add_utxo()` @@ -2512,6 +2540,7 @@ fn test_bump_fee_unconfirmed_inputs_only() { txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature } wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder.fee_rate(FeeRate::from_sat_per_vb_unchecked(25)); builder.finish().unwrap(); @@ -2543,6 +2572,7 @@ fn test_bump_fee_unconfirmed_input() { txin.witness.push([0x00; P2WPKH_FAKE_WITNESS_SIZE]); // fake signature } wallet.insert_tx(tx); + wallet.insert_seen_at(txid, 0); let mut builder = wallet.build_fee_bump(txid).unwrap(); builder