diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 3f83fe98e..df95c11ec 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -1551,7 +1551,54 @@ impl Wallet { let txid = tx.compute_txid(); let (amount_msat, fee_paid_msat, direction) = self.onchain_payment_fields(tx); + // A funding transaction that moves no wallet funds carries nothing to record — e.g. LDK + // re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding path, + // including splices the interactive-funding classification deliberately declined (no + // local contribution, or a splice-out moving no wallet funds). Recording it here would + // mint a zero-amount payment that nothing ever confirms. Skip on the wallet-derived + // amount alone — the condition `classify_interactive_funding` declines on; anything + // declined there must be skipped here, or its re-broadcast resurrects the record. The fee + // is no participation signal: the wallet resolves a splice's shared input whenever the + // previous funding transaction touched it (e.g. it funded the original channel open). + // + // TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The + // re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`: + // the re-offer ought to keep its `InteractiveFunding` classification, or not recur at + // all. `zero_conf_splice_out_funding_rebroadcast_canary` pins the current behavior by + // asserting the log line below; when it fails against a newer LDK, re-evaluate whether + // this skip still sees traffic. + if amount_msat == Some(0) { + log_trace!( + self.logger, + "Not recording channel-funding broadcast {} as a payment: no wallet-level activity", + txid, + ); + return Ok(()); + } + let payment_id = PaymentId(txid.to_byte_array()); + + // A promoted-but-unconfirmed 0conf splice comes back through this generic path re-typed + // and carrying wallet-view figures; `funding_reclassification_update` declines the + // downgrade, leaving no trace that a re-broadcast arrived. Log the arrival so tests can + // observe the traffic. The read cannot go stale: only the broadcast loop writes + // interactive-funding classifications, and it runs this classification too. + if let Some(current) = self.payment_store.get(&payment_id) { + if matches!( + current.kind, + PaymentKind::Onchain { + tx_type: Some(TransactionType::InteractiveFunding { .. }), + .. + } + ) { + log_trace!( + self.logger, + "Keeping interactive-funding classification over funding-typed rebroadcast {}", + txid, + ); + } + } + let details = PaymentDetails::new( payment_id, PaymentKind::Onchain { @@ -2585,6 +2632,28 @@ fn ldk_to_bdk_satisfaction_weight(ldk_satisfaction_weight: u64) -> Weight { fn funding_reclassification_update( details: PaymentDetails, candidates: &[FundingTxCandidate], current: Option<&PaymentDetails>, ) -> PaymentDetailsUpdate { + // A funding-typed classification of a record already classified as interactive funding is a + // downgrade, not news: LDK re-broadcasts a promoted-but-unconfirmed splice through its + // generic funding path, where the figures are wallet-view rather than contribution-derived. + // Keep the record as classified; wallet-sync events own its confirmation state. + // + // TODO(https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/issues/4878): The + // re-typed re-broadcasts are upstream behavior that should be fixed in `rust-lightning`: + // the re-offer ought to keep its `InteractiveFunding` classification, or not recur at all. + // `zero_conf_splice_in_funding_rebroadcast_canary` pins the current behavior via the + // arrival log in `classify_funding`; when it fails against a newer LDK, re-evaluate + // whether this guard still sees traffic. + if let ( + Some(PaymentKind::Onchain { + tx_type: Some(TransactionType::InteractiveFunding { .. }), + .. + }), + PaymentKind::Onchain { tx_type: Some(TransactionType::Funding { .. }), .. }, + ) = (current.map(|payment| &payment.kind), &details.kind) + { + return PaymentDetailsUpdate::new(details.id); + } + let mut update = PaymentDetailsUpdate::funding_reclassification(details); if let Some(PaymentKind::Onchain { txid: confirmed_txid, @@ -3689,6 +3758,35 @@ mod tests { assert_eq!(update.txid, Some(active_txid)); } + /// A funding-typed (re)classification of a record already classified as interactive funding + /// carries nothing the record doesn't have — LDK re-broadcasts a promoted-but-unconfirmed + /// splice through its generic funding path with wallet-view figures — so the update must + /// move nothing. + #[test] + fn funding_reclassification_update_skips_funding_over_interactive_funding() { + let txid = Txid::from_byte_array([1u8; 32]); + let payment_id = PaymentId(txid.to_byte_array()); + let current = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500)); + + let rebroadcast = PaymentDetails::new( + payment_id, + PaymentKind::Onchain { + txid, + status: ConfirmationStatus::Unconfirmed, + tx_type: Some(TransactionType::Funding { channels: vec![] }), + }, + Some(10_000_000), + Some(0), + PaymentDirection::Inbound, + PaymentStatus::Pending, + ); + + let update = funding_reclassification_update(rebroadcast, &[], Some(¤t)); + let mut updated = current.clone(); + assert!(!updated.update(update), "the rebroadcast must not move the record"); + assert_eq!(updated, current); + } + /// Graduation must decide from the live record and write only the status: a pending-store /// snapshot taken before a concurrent classification landed must not roll the record's /// figures back when the payment graduates to `Succeeded`. @@ -3831,6 +3929,148 @@ mod tests { assert_eq!(wallet.find_payment_by_txid(txid2), Some(payment_id)); } + /// A funding-typed broadcast that doesn't touch the on-chain wallet must not be recorded. + /// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding + /// path, so a splice the interactive-funding classification deliberately declined — no local + /// contribution, or none of the moved funds are the wallet's — would otherwise come back as + /// a spurious zero-amount record that nothing ever confirms. + #[tokio::test] + async fn funding_broadcast_without_wallet_activity_is_not_recorded() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + + let counterparty_node_id = PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(); + let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))]; + let tx_type = TransactionType::Funding { channels: vec![] }; + + // No inputs or outputs involve the wallet: nothing to record. + wallet.classify_funding(&dummy_tx(), &channels, tx_type.clone()).await.unwrap(); + assert!(wallet.payment_store.list_filter(|_| true).is_empty()); + assert!(wallet.pending_payment_store.list_filter(|_| true).is_empty()); + + // A computable fee is not wallet participation. The wallet can resolve a splice's shared + // input whenever the previous funding transaction touched it (e.g. it funded the original + // channel open), so it derives the splice's fee even when no wallet funds move. + let prev_funding_outpoint = OutPoint { txid: Txid::from_byte_array([8u8; 32]), vout: 0 }; + wallet.inner.lock().unwrap().insert_txout( + prev_funding_outpoint, + TxOut { value: Amount::from_sat(100_000), script_pubkey: ScriptBuf::new() }, + ); + let splice_tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: LockTime::ZERO, + input: vec![bitcoin::TxIn { + previous_output: prev_funding_outpoint, + ..Default::default() + }], + output: vec![TxOut { + value: Amount::from_sat(99_000), + script_pubkey: ScriptBuf::new(), + }], + }; + wallet.classify_funding(&splice_tx, &channels, tx_type.clone()).await.unwrap(); + assert!(wallet.payment_store.list_filter(|_| true).is_empty()); + + // Control: a funding transaction the wallet participates in is still recorded. + let script_pubkey = wallet + .inner + .lock() + .unwrap() + .reveal_next_address(KeychainKind::External) + .address + .script_pubkey(); + let funded_tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: LockTime::ZERO, + input: Vec::new(), + output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }], + }; + wallet.classify_funding(&funded_tx, &channels, tx_type).await.unwrap(); + let payments = wallet.payment_store.list_filter(|_| true); + assert_eq!(payments.len(), 1); + assert_eq!(payments[0].id, PaymentId(funded_tx.compute_txid().to_byte_array())); + } + + /// LDK re-broadcasts a promoted-but-unconfirmed 0conf splice through its generic funding + /// path: same txid, but typed as a plain funding transaction with wallet-view figures and no + /// contribution metadata. The rebroadcast must not overwrite the contribution-derived + /// figures or the interactive-funding classification — neither while the record is + /// unconfirmed nor once it confirmed under that same txid, where updates naming the + /// confirmed txid may otherwise move figures. + #[tokio::test] + async fn funding_rebroadcast_keeps_interactive_funding_classification() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + + // The rebroadcast passes the wallet-activity guard: a splice-in funds the new channel + // output partly from the wallet, so the wallet sees movement. + let script_pubkey = wallet + .inner + .lock() + .unwrap() + .reveal_next_address(KeychainKind::External) + .address + .script_pubkey(); + let tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: LockTime::ZERO, + input: Vec::new(), + output: vec![TxOut { value: Amount::from_sat(10_000), script_pubkey }], + }; + let txid = tx.compute_txid(); + let payment_id = PaymentId(txid.to_byte_array()); + + let candidates = vec![FundingTxCandidate { + txid, + amount_msat: Some(1_000_000), + fee_paid_msat: Some(500), + }]; + let details = interactive_funding_details(payment_id, txid, Some(1_000_000), Some(500)); + wallet.persist_funding_payment(details, candidates).await.unwrap(); + + let counterparty_node_id = PublicKey::from_str( + "0279be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + .unwrap(); + let channels = vec![(counterparty_node_id, ChannelId([7u8; 32]))]; + let tx_type = TransactionType::Funding { channels: vec![] }; + + let assert_unchanged = |confirmed: bool| { + let payments = wallet.payment_store.list_filter(|_| true); + assert_eq!(payments.len(), 1, "the rebroadcast must not mint a second record"); + let payment = &payments[0]; + assert_eq!(payment.id, payment_id); + assert_eq!(payment.amount_msat, Some(1_000_000)); + assert_eq!(payment.fee_paid_msat, Some(500)); + match &payment.kind { + PaymentKind::Onchain { + status, + tx_type: Some(TransactionType::InteractiveFunding { .. }), + .. + } => assert_eq!(matches!(status, ConfirmationStatus::Confirmed { .. }), confirmed), + kind => panic!("unexpected kind {:?}", kind), + } + }; + + wallet.classify_funding(&tx, &channels, tx_type.clone()).await.unwrap(); + assert_unchanged(false); + + // Confirm the record, then replay the rebroadcast: a monitor-update completion can race + // wallet sync around confirmation. + let event = WalletEvent::TxConfirmed { + txid, + tx: Arc::new(tx.clone()), + block_time: confirmed_block_time(5), + old_block_time: None, + }; + wallet.update_payment_store(vec![event]).await.unwrap(); + wallet.classify_funding(&tx, &channels, tx_type).await.unwrap(); + assert_unchanged(true); + } + /// Barrier test, classification-first ordering: wallet sync's confirmation handling must /// wait for classification's two-store write pair. Classification is parked between its /// payment-store and pending-store writes (the torn window) and only then is the diff --git a/tests/common/logging.rs b/tests/common/logging.rs index 1e3a8a1c2..3b231b3cd 100644 --- a/tests/common/logging.rs +++ b/tests/common/logging.rs @@ -173,3 +173,47 @@ impl LogWriter for MultiNodeLogger { print!("{}", log); } } + +/// Collects every log message a node emits, for tests that assert a specific line was logged. +pub(crate) struct CollectingLogWriter { + logs: Mutex>, +} + +impl CollectingLogWriter { + pub(crate) fn new() -> Self { + Self { logs: Mutex::new(Vec::new()) } + } + + pub(crate) fn contains(&self, text: &str) -> bool { + self.count(text) > 0 + } + + pub(crate) fn count(&self, text: &str) -> usize { + self.logs.lock().unwrap().iter().filter(|message| message.contains(text)).count() + } + + /// Waits up to ten seconds for a logged message containing `text`, returning whether one + /// arrived. Polling beats a fixed sleep: it returns as soon as the line lands and only pays + /// the full timeout when the line never comes. + pub(crate) async fn wait_for(&self, text: &str) -> bool { + self.wait_for_count(text, 1).await + } + + /// Waits up to ten seconds for `occurrences` logged messages containing `text`, returning + /// whether they arrived. + pub(crate) async fn wait_for_count(&self, text: &str, occurrences: usize) -> bool { + for _ in 0..100 { + if self.count(text) >= occurrences { + return true; + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + false + } +} + +impl LogWriter for CollectingLogWriter { + fn log(&self, record: LogRecord) { + self.logs.lock().unwrap().push(record.args.to_string()); + } +} diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 0333fe006..1082a75f7 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -18,7 +18,9 @@ use bitcoin::address::NetworkUnchecked; use bitcoin::hashes::sha256::Hash as Sha256Hash; use bitcoin::hashes::Hash; use bitcoin::{Address, Amount, ScriptBuf, Txid}; -use common::logging::{init_log_logger, validate_log_entry, MultiNodeLogger, TestLogWriter}; +use common::logging::{ + init_log_logger, validate_log_entry, CollectingLogWriter, MultiNodeLogger, TestLogWriter, +}; use common::{ bump_fee_and_broadcast, distribute_funds_unconfirmed, do_channel_full_cycle, expect_channel_pending_event, expect_channel_ready_event, expect_channel_ready_events, @@ -2140,6 +2142,196 @@ async fn splice_channel() { ); } +/// Canary for the upstream behavior the zero-activity skip in `classify_funding` works around: +/// after a 0conf splice is promoted, LDK re-broadcasts the still-unconfirmed funding transaction +/// through its generic funding path — re-typed as a plain funding transaction without its +/// contribution metadata — on every monitor-update completion until it confirms. A splice-out +/// paying an external address moves no wallet funds, so the interactive-funding classification +/// declines to record it and each re-offer then arrives with nothing to record. The re-typing is +/// tracked upstream at . +/// +/// If this test fails, upstream likely stopped re-offering the transaction that way (or now +/// preserves its interactive-funding classification): re-evaluate whether the skip still sees +/// traffic. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn zero_conf_splice_out_funding_rebroadcast_canary() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // The skip leaves no trace in the payment stores — that is its point — so observe it through + // Node A's logs. `setup_two_nodes` wires file loggers, so build the pair manually with a + // collector, Node B trusting Node A for 0conf. + let logger_a = Arc::new(CollectingLogWriter::new()); + let mut config_a = random_config(); + config_a.log_writer = TestLogWriter::Custom(logger_a.clone()); + let node_a = setup_node(&chain_source, config_a); + + let mut config_b = random_config(); + config_b.node_config.trusted_peers_0conf.push(node_a.node_id()); + let node_b = setup_node(&chain_source, config_b); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + let premine_amount_sat = 5_000_000; + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a], + Amount::from_sat(premine_amount_sat), + ) + .await; + node_a.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 2_000_000, false, &electrsd).await; + + // 0conf: the channel is ready without any confirmations. + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Confirm the original funding so the splice below is the only unconfirmed funding. + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + // Splice out to a third-party address: channel funds leave without touching Node A's + // on-chain wallet, so no classification path records the transaction. + let external_address = bitcoind.client.new_address().unwrap(); + node_a.splice_out(&user_channel_id_a, node_b.node_id(), &external_address, 500_000).unwrap(); + let txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + + // The 0conf splice locks without confirmations, re-signaled as `ChannelReady`. + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Locking the splice completed monitor updates that re-offered the unconfirmed funding + // transaction; a payment drives further monitor updates and thus further re-broadcasts. + let amount_msat = 1_000_000; + let payment_id = + node_a.spontaneous_payment().send(amount_msat, node_b.node_id(), None).unwrap(); + expect_payment_successful_event!(node_a, payment_id, None); + expect_payment_received_event!(node_b, amount_msat); + + // Canary: the skip saw a re-offer. When this stops firing, LDK no longer re-offers the + // promoted-but-unconfirmed splice through the generic funding path. The line is also the + // synchronization point: it is the terminal action of classifying a re-offer, so once it + // appears the classification pipeline has demonstrably processed one. + let skipped = format!("Not recording channel-funding broadcast {}", txo.txid); + assert!( + logger_a.wait_for(&skipped).await, + "Node A never skipped a generic-funding re-broadcast of the promoted 0conf splice-out; if \ + upstream stopped re-offering it, re-evaluate the zero-activity skip in classify_funding" + ); + + // The re-offers must not have minted a record for a transaction the wallet has no stake in. + let splice_records = node_a.list_payments_with_filter( + |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == txo.txid), + ); + assert!( + splice_records.is_empty(), + "a zero-activity funding re-broadcast minted a record: {:?}", + splice_records + ); +} + +/// Canary for the upstream behavior the funding-over-interactive-funding guard in +/// `funding_reclassification_update` works around: LDK re-broadcasts a promoted-but-unconfirmed +/// 0conf splice through its generic funding path — re-typed as a plain funding transaction with +/// wallet-view figures and no contribution metadata — on every monitor-update completion until it +/// confirms. On the contributing side those re-offers target the interactive-funding record, +/// which must come through unchanged. The re-typing is tracked upstream at +/// . +/// +/// If this test fails, upstream likely stopped re-offering the transaction that way (or now +/// preserves its interactive-funding classification): re-evaluate whether the guard still sees +/// traffic. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn zero_conf_splice_in_funding_rebroadcast_canary() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = random_chain_source(&bitcoind, &electrsd); + + // The guard leaves no trace in the stores, so observe the re-offers through Node A's logs. + // `setup_two_nodes` wires file loggers, so build the pair manually with a collector, Node B + // trusting Node A for 0conf. + let logger_a = Arc::new(CollectingLogWriter::new()); + let mut config_a = random_config(); + config_a.log_writer = TestLogWriter::Custom(logger_a.clone()); + let node_a = setup_node(&chain_source, config_a); + + let mut config_b = random_config(); + config_b.node_config.trusted_peers_0conf.push(node_a.node_id()); + let node_b = setup_node(&chain_source, config_b); + + let address_a = node_a.onchain_payment().new_address().unwrap(); + let premine_amount_sat = 5_000_000; + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![address_a], + Amount::from_sat(premine_amount_sat), + ) + .await; + node_a.sync_wallets().unwrap(); + + open_channel(&node_a, &node_b, 2_000_000, false, &electrsd).await; + + // 0conf: the channel is ready without any confirmations. + let user_channel_id_a = expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Confirm the original funding so the splice below is the only unconfirmed funding and Node + // A's change from the open is spendable for the splice contribution. + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + node_a.splice_in(&user_channel_id_a, node_b.node_id(), 1_000_000).unwrap(); + let txo = expect_splice_negotiated_event!(node_a, node_b.node_id()); + wait_for_classified_funding_payment(&node_a, txo.txid).await; + + // The 0conf splice locks without confirmations, re-signaled as `ChannelReady`. + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + let splice_payments = |node: &Node| { + node.list_payments_with_filter( + |p| matches!(p.kind, PaymentKind::Onchain { txid, .. } if txid == txo.txid), + ) + }; + let payments = splice_payments(&node_a); + assert_eq!(payments.len(), 1); + let recorded_amount_msat = payments[0].amount_msat; + let recorded_fee_paid_msat = payments[0].fee_paid_msat; + + // Locking the splice completed monitor updates that re-offered the unconfirmed funding + // transaction; a payment drives further monitor updates and thus further re-broadcasts. + let amount_msat = 1_000_000; + let payment_id = + node_a.spontaneous_payment().send(amount_msat, node_b.node_id(), None).unwrap(); + expect_payment_successful_event!(node_a, payment_id, None); + expect_payment_received_event!(node_b, amount_msat); + + // Canary: generic re-offers of the splice reached classification while its record held the + // interactive-funding classification. Waiting for the second occurrence also makes the + // record assertions below deterministic — the broadcast loop classifies sequentially, so by + // the second arrival the first re-offer's store write has completed. + let rebroadcast = format!("funding-typed rebroadcast {}", txo.txid); + assert!( + logger_a.wait_for_count(&rebroadcast, 2).await, + "Node A saw no generic-funding re-broadcast targeting the interactive-funding record; if \ + upstream stopped re-offering it, re-evaluate the guard in funding_reclassification_update" + ); + + // The re-offers must not have disturbed the record's classification or figures. + let payments = splice_payments(&node_a); + assert_eq!(payments.len(), 1); + let payment = &payments[0]; + assert_eq!(payment.amount_msat, recorded_amount_msat); + assert_eq!(payment.fee_paid_msat, recorded_fee_paid_msat); + assert!(matches!( + payment.kind, + PaymentKind::Onchain { tx_type: Some(TransactionType::InteractiveFunding { .. }), .. } + )); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn rbf_splice_channel() { run_rbf_splice_channel_test(false).await;