diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index f49764b657f..1d035b68650 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -3933,6 +3933,7 @@ impl ChannelMonitorImpl { #[rustfmt::skip] fn generate_claimable_outpoints_and_watch_outputs( &mut self, generate_monitor_event_with_reason: Option, + require_funding_seen: bool, ) -> (Vec, Vec) { let funding = get_confirmed_funding_scope!(self); let holder_commitment_tx = &funding.current_holder_commitment_tx; @@ -3960,6 +3961,13 @@ impl ChannelMonitorImpl { // in the claim that is queued to OnchainTxHandler. We set holder_tx_signed here to reject // new channel updates. self.holder_tx_signed = true; + + // In manual-broadcast mode, if we have not yet observed the funding transaction on-chain, + // return empty vectors rather than triggering a broadcast. + if require_funding_seen && self.is_manual_broadcast && !self.funding_seen_onchain { + return (Vec::new(), Vec::new()); + } + let mut watch_outputs = Vec::new(); // In CSV anchor channels, we can't broadcast our HTLC transactions while the commitment transaction is // unconfirmed. @@ -3985,13 +3993,7 @@ impl ChannelMonitorImpl { } claimable_outpoints.append(&mut new_outpoints); } - // In manual-broadcast mode, if we have not yet observed the funding transaction on-chain, - // return empty vectors. - if self.is_manual_broadcast && !self.funding_seen_onchain { - return (Vec::new(), Vec::new()); - } else { - (claimable_outpoints, watch_outputs) - } + (claimable_outpoints, watch_outputs) } #[rustfmt::skip] @@ -4004,7 +4006,8 @@ impl ChannelMonitorImpl { /// /// [`ChannelMonitor::broadcast_latest_holder_commitment_txn`]: crate::chain::channelmonitor::ChannelMonitor::broadcast_latest_holder_commitment_txn pub(crate) fn queue_latest_holder_commitment_txn_for_broadcast( - &mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator, logger: &WithChannelMonitor, require_funding_seen: bool, + &mut self, broadcaster: &B, fee_estimator: &LowerBoundedFeeEstimator, logger: &WithChannelMonitor, + require_funding_seen: bool, ) where B::Target: BroadcasterInterface, @@ -4015,7 +4018,8 @@ impl ChannelMonitorImpl { broadcasted_latest_txn: Some(true), message: "ChannelMonitor-initiated commitment transaction broadcast".to_owned(), }; - let (claimable_outpoints, _) = self.generate_claimable_outpoints_and_watch_outputs(Some(reason)); + let (claimable_outpoints, _) = + self.generate_claimable_outpoints_and_watch_outputs(Some(reason), require_funding_seen); // In manual-broadcast mode, if `require_funding_seen` is true and we have not yet observed // the funding transaction on-chain, do not queue any transactions. if require_funding_seen && self.is_manual_broadcast && !self.funding_seen_onchain { @@ -5618,7 +5622,7 @@ impl ChannelMonitorImpl { if should_broadcast_commitment { let (mut claimables, mut outputs) = - self.generate_claimable_outpoints_and_watch_outputs(None); + self.generate_claimable_outpoints_and_watch_outputs(None, false); claimable_outpoints.append(&mut claimables); watch_outputs.append(&mut outputs); } @@ -5660,9 +5664,13 @@ impl ChannelMonitorImpl { if let Some(payment_hash) = should_broadcast { let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) }; let (mut new_outpoints, mut new_outputs) = - self.generate_claimable_outpoints_and_watch_outputs(Some(reason)); - claimable_outpoints.append(&mut new_outpoints); - watch_outputs.append(&mut new_outputs); + self.generate_claimable_outpoints_and_watch_outputs(Some(reason), false); + if !self.is_manual_broadcast || self.funding_seen_onchain { + claimable_outpoints.append(&mut new_outpoints); + watch_outputs.append(&mut new_outputs); + } else { + log_info!(logger, "Not broadcasting holder commitment for manual-broadcast channel before funding appears on-chain"); + } } } @@ -5969,7 +5977,7 @@ impl ChannelMonitorImpl { /// Filters a block's `txdata` for transactions spending watched outputs or for any child /// transactions thereof. #[rustfmt::skip] - fn filter_block<'a>(&mut self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> { + fn filter_block<'a>(&self, txdata: &TransactionData<'a>) -> Vec<&'a Transaction> { let mut matched_txn = new_hash_set(); txdata.iter().filter(|&&(_, tx)| { let mut matches = self.spends_watched_output(tx); diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 17ac959be64..99b64ed419b 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1576,14 +1576,10 @@ pub fn open_zero_conf_channel<'a, 'b, 'c, 'd>( open_zero_conf_channel_with_value(initiator, receiver, initiator_config, 100_000, 10_001) } -// Receiver must have been initialized with manually_accept_inbound_channels set to true. -pub fn open_zero_conf_channel_with_value<'a, 'b, 'c, 'd>( +pub fn exchange_open_accept_zero_conf_chan<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, receiver: &'a Node<'b, 'c, 'd>, initiator_config: Option, channel_value_sat: u64, push_msat: u64, -) -> (bitcoin::Transaction, ChannelId) { - let initiator_channels = initiator.node.list_usable_channels().len(); - let receiver_channels = receiver.node.list_usable_channels().len(); - +) -> ChannelId { let receiver_node_id = receiver.node.get_our_node_id(); let initiator_node_id = initiator.node.get_our_node_id(); @@ -1617,6 +1613,28 @@ pub fn open_zero_conf_channel_with_value<'a, 'b, 'c, 'd>( assert_eq!(accept_channel.common_fields.minimum_depth, 0); initiator.node.handle_accept_channel(receiver_node_id, &accept_channel); + accept_channel.common_fields.temporary_channel_id +} + +// Receiver must have been initialized with manually_accept_inbound_channels set to true. +pub fn open_zero_conf_channel_with_value<'a, 'b, 'c, 'd>( + initiator: &'a Node<'b, 'c, 'd>, receiver: &'a Node<'b, 'c, 'd>, + initiator_config: Option, channel_value_sat: u64, push_msat: u64, +) -> (bitcoin::Transaction, ChannelId) { + let initiator_channels = initiator.node.list_usable_channels().len(); + let receiver_channels = receiver.node.list_usable_channels().len(); + + let receiver_node_id = receiver.node.get_our_node_id(); + let initiator_node_id = initiator.node.get_our_node_id(); + + exchange_open_accept_zero_conf_chan( + initiator, + receiver, + initiator_config, + channel_value_sat, + push_msat, + ); + let (temporary_channel_id, tx, _) = create_funding_transaction(&initiator, &receiver_node_id, channel_value_sat, 42); initiator @@ -1806,14 +1824,18 @@ pub fn create_chan_between_nodes_with_value_a<'a, 'b, 'c: 'd, 'd>( pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>( nodes: &'a Vec>, initiator: usize, counterparty: usize, channel_value: u64, - push_msat: u64, + push_msat: u64, zero_conf: bool, ) -> (ChannelId, Transaction, OutPoint) { let node_a = &nodes[initiator]; let node_b = &nodes[counterparty]; let node_a_id = node_a.node.get_our_node_id(); let node_b_id = node_b.node.get_our_node_id(); - let temp_channel_id = exchange_open_accept_chan(node_a, node_b, channel_value, push_msat); + let temp_channel_id = if zero_conf { + exchange_open_accept_zero_conf_chan(node_a, node_b, None, channel_value, push_msat) + } else { + exchange_open_accept_chan(node_a, node_b, channel_value, push_msat) + }; let (funding_temp_id, funding_tx, funding_outpoint) = create_funding_transaction(node_a, &node_b_id, channel_value, 42); @@ -1834,12 +1856,39 @@ pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>( check_added_monitors!(node_b, 1); let channel_id_b = expect_channel_pending_event(node_b, &node_a_id); - let funding_signed = get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a_id); - node_a.node.handle_funding_signed(node_b_id, &funding_signed); - check_added_monitors!(node_a, 1); + if zero_conf { + let bs_signed_locked = node_b.node.get_and_clear_pending_msg_events(); + assert_eq!(bs_signed_locked.len(), 2); + match &bs_signed_locked[0] { + MessageSendEvent::SendFundingSigned { node_id, msg } => { + assert_eq!(*node_id, node_a_id); + node_a.node.handle_funding_signed(node_b_id, &msg); + check_added_monitors(node_a, 1); + + assert!(node_a.tx_broadcaster.txn_broadcast().is_empty()); + + let as_channel_ready = + get_event_msg!(node_a, MessageSendEvent::SendChannelReady, node_b_id); + node_b.node.handle_channel_ready(node_a_id, &as_channel_ready); + expect_channel_ready_event(node_b, &node_a_id); + }, + _ => panic!("Unexpected event"), + } + match &bs_signed_locked[1] { + MessageSendEvent::SendChannelReady { node_id, msg } => { + assert_eq!(*node_id, node_a_id); + node_a.node.handle_channel_ready(node_b_id, &msg); + }, + _ => panic!("Unexpected event"), + } + } else { + let funding_signed = get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a_id); + node_a.node.handle_funding_signed(node_b_id, &funding_signed); + check_added_monitors(node_a, 1) + } let events = node_a.node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); + assert_eq!(events.len(), if zero_conf { 3 } else { 2 }); let funding_txid = funding_tx.compute_txid(); let mut channel_id = None; for event in events { @@ -1853,6 +1902,10 @@ pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>( assert_eq!(counterparty_node_id, node_b_id); channel_id = Some(pending_id); }, + Event::ChannelReady { channel_id: pending_id, counterparty_node_id, .. } => { + assert_eq!(counterparty_node_id, node_b_id); + channel_id = Some(pending_id); + }, _ => panic!("Unexpected event"), } } @@ -1861,6 +1914,16 @@ pub fn create_channel_manual_funding<'a, 'b, 'c: 'd, 'd>( assert!(node_a.tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty()); + if zero_conf { + let as_channel_update = + get_event_msg!(node_a, MessageSendEvent::SendChannelUpdate, node_b_id); + let bs_channel_update = + get_event_msg!(node_b, MessageSendEvent::SendChannelUpdate, node_a_id); + + node_a.node.handle_channel_update(node_b_id, &bs_channel_update); + node_b.node.handle_channel_update(node_a_id, &as_channel_update); + } + (channel_id, funding_tx, funding_outpoint) } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index a1863cf32e6..876324737a6 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -20,7 +20,6 @@ use crate::chain::channelmonitor::{ }; use crate::chain::transaction::OutPoint; use crate::chain::{ChannelMonitorUpdateStatus, Confirm, Listen, Watch}; -use crate::events::bump_transaction::BumpTransactionEvent; use crate::events::{ ClosureReason, Event, HTLCHandlingFailureType, PathFailure, PaymentFailureReason, PaymentPurpose, @@ -9629,64 +9628,79 @@ pub fn test_remove_expired_inbound_unfunded_channels() { check_closed_event(&nodes[1], 1, reason, false, &[node_a_id], 100000); } -#[test] -fn test_manual_broadcast_skips_commitment_until_funding_seen() { +fn do_test_manual_broadcast_skips_commitment_until_funding( + force_broadcast: bool, close_by_timeout: bool, zero_conf_open: bool, +) { + // Checks that commitment (and HTLC) transactions will not be broadcast for manual-funded + // channels until either the funding transaction is seen on-chain or the channel is manually + // forced to broadcast using `ChannelMonitor::broadcast_latest_holder_commitment_txn`. let chanmon_cfgs = create_chanmon_cfgs(2); let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let mut chan_config = test_default_channel_config(); + if zero_conf_open { + chan_config.manually_accept_inbound_channels = true; + } + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(chan_config)]); let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let node_a_id = nodes[0].node.get_our_node_id(); let node_b_id = nodes[1].node.get_our_node_id(); let (channel_id, funding_tx, funding_outpoint) = - create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000); - - nodes[0] - .node - .force_close_broadcasting_latest_txn(&channel_id, &node_b_id, "manual close".to_owned()) - .unwrap(); - check_added_monitors!(&nodes[0], 1); - check_added_monitors!(&nodes[1], 0); + create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000, zero_conf_open); - assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty()); - nodes[0].node.get_and_clear_pending_msg_events(); - nodes[1].node.get_and_clear_pending_msg_events(); - let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1); - match &events[0] { - Event::ChannelClosed { - reason: ClosureReason::HolderForceClosed { broadcasted_latest_txn, message }, - counterparty_node_id: Some(id), - .. - } => { - assert_eq!(*broadcasted_latest_txn, Some(true)); - assert_eq!(message, "manual close"); - assert_eq!(id, &node_b_id); - }, - _ => panic!("Unexpected event"), + if close_by_timeout { + if !zero_conf_open { + panic!("Cant send a payment if we didn't open 0-conf"); + } + let (_payment_preimage, payment_hash, _payment_secret, _payment_id) = + route_payment(&nodes[0], &[&nodes[1]], 10_000_000); + nodes[1].node.get_and_clear_pending_events(); + + connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); + let reason = ClosureReason::HTLCsTimedOut { payment_hash: Some(payment_hash) }; + check_closed_event(&nodes[0], 1, reason, false, &[node_b_id], 100_000); + + // On timeout, B will try to fail the HTLC back, but its too late - A has already FC'd. + connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); + let failure = HTLCHandlingFailureType::Receive { payment_hash }; + expect_and_process_pending_htlcs_and_htlc_handling_failed(&nodes[1], &[failure]); + get_htlc_update_msgs(&nodes[1], &node_a_id); + check_added_monitors(&nodes[1], 1); + } else { + let msg = "manual close".to_owned(); + nodes[0] + .node + .force_close_broadcasting_latest_txn(&channel_id, &node_b_id, msg.clone()) + .unwrap(); + let reason = + ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(true), message: msg }; + check_closed_event(&nodes[0], 1, reason, false, &[node_b_id], 100_000); } - nodes[1].node.get_and_clear_pending_events(); + check_added_monitors(&nodes[0], 1); + assert_eq!(get_err_msg(&nodes[0], &node_b_id).channel_id, channel_id); + assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty()); let monitor_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events(); assert!(monitor_events.is_empty()); - confirm_transaction(&nodes[0], &funding_tx); - confirm_transaction(&nodes[1], &funding_tx); - nodes[0].node.get_and_clear_pending_msg_events(); - nodes[1].node.get_and_clear_pending_msg_events(); - - { + // The funding tx should be broadcasted after either a manual broadcast call or if the funding + // transaction appears on chain. + if force_broadcast { let monitor = get_monitor!(&nodes[0], channel_id); - // manual override monitor.broadcast_latest_holder_commitment_txn( &nodes[0].tx_broadcaster, &nodes[0].fee_estimator, &nodes[0].logger, ); + } else { + mine_transaction(&nodes[0], &funding_tx); + mine_transaction(&nodes[1], &funding_tx); } + let funding_txid = funding_tx.compute_txid(); let broadcasts = nodes[0].tx_broadcaster.txn_broadcast(); - assert!(!broadcasts.is_empty()); + assert_eq!(broadcasts.len(), if close_by_timeout { 2 } else { 1 }); let commitment_tx = broadcasts .iter() .find(|tx| { @@ -9702,129 +9716,32 @@ fn test_manual_broadcast_skips_commitment_until_funding_seen() { assert_eq!(commitment_input.previous_output.txid, funding_txid); assert_eq!(commitment_input.previous_output.vout, u32::from(funding_outpoint.index)); - let monitor_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events(); - assert!(monitor_events.iter().all(|event| !matches!(event, Event::BumpTransaction(_)))); - assert!(nodes[0].node.get_and_clear_pending_events().is_empty()); - assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); -} - -#[test] -fn test_manual_broadcast_detects_funding_and_broadcasts_on_timeout() { - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - - let node_b_id = nodes[1].node.get_our_node_id(); - - let (channel_id, funding_tx, funding_outpoint) = - create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000); - - let funding_msgs = - create_chan_between_nodes_with_value_confirm(&nodes[0], &nodes[1], &funding_tx); - let confirmed_channel_id = funding_msgs.1; - assert_eq!(confirmed_channel_id, channel_id); - let _announcements = - create_chan_between_nodes_with_value_b(&nodes[0], &nodes[1], &funding_msgs.0); - - let usable_channels = nodes[0].node.list_usable_channels(); - assert_eq!(usable_channels.len(), 1); - assert_eq!(usable_channels[0].channel_id, channel_id); - - let (_payment_preimage, _payment_hash, _payment_secret, _payment_id) = - route_payment(&nodes[0], &[&nodes[1]], 10_000_000); - nodes[1].node.get_and_clear_pending_events(); - - connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); - connect_blocks(&nodes[1], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1); - - let events = nodes[0].node.get_and_clear_pending_events(); - assert!(events.iter().any(|event| matches!( - event, - Event::ChannelClosed { - reason: ClosureReason::HTLCsTimedOut { .. }, - counterparty_node_id: Some(id), - .. - } if id == &node_b_id - ))); - nodes[1].node.get_and_clear_pending_events(); - nodes[0].node.get_and_clear_pending_msg_events(); - nodes[1].node.get_and_clear_pending_msg_events(); - check_added_monitors!(&nodes[0], 1); - check_added_monitors!(&nodes[1], 0); - - let funding_txid = funding_tx.compute_txid(); - let broadcasts = nodes[0].tx_broadcaster.txn_broadcast(); - assert!(!broadcasts.is_empty()); - let commitment_tx = broadcasts - .iter() - .find(|tx| { - tx.input.iter().any(|input| { - input.previous_output.txid == funding_txid - && input.previous_output.vout == u32::from(funding_outpoint.index) + if close_by_timeout { + let htlc_tx = broadcasts + .iter() + .find(|tx| { + tx.input + .iter() + .any(|input| input.previous_output.txid == commitment_tx.compute_txid()) }) - }) - .expect("commitment transaction not broadcast"); - check_spends!(commitment_tx, funding_tx); - assert_eq!(commitment_tx.input.len(), 1); - - for event in nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events() { - if let Event::BumpTransaction(bump_event) = event { - if let BumpTransactionEvent::ChannelClose { - channel_id: event_channel_id, - counterparty_node_id, - .. - } = &bump_event - { - assert_eq!(*event_channel_id, channel_id); - assert_eq!(*counterparty_node_id, node_b_id); - } - nodes[0].bump_tx_handler.handle_event(&bump_event); - } - } -} - -#[test] -fn test_manual_broadcast_no_bump_events_before_funding_seen() { - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - - let node_b_id = nodes[1].node.get_our_node_id(); - - let (channel_id, _, _) = create_channel_manual_funding(&nodes, 0, 1, 100_000, 10_000); - - nodes[0] - .node - .force_close_broadcasting_latest_txn(&channel_id, &node_b_id, "manual close".to_owned()) - .unwrap(); - check_added_monitors!(&nodes[0], 1); - check_added_monitors!(&nodes[1], 0); - - assert!(nodes[0].tx_broadcaster.txn_broadcast().is_empty()); - nodes[0].node.get_and_clear_pending_msg_events(); - nodes[1].node.get_and_clear_pending_msg_events(); - let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1); - match &events[0] { - Event::ChannelClosed { - reason: ClosureReason::HolderForceClosed { broadcasted_latest_txn, message }, - counterparty_node_id: Some(id), - .. - } => { - assert_eq!(*broadcasted_latest_txn, Some(true)); - assert_eq!(message, "manual close"); - assert_eq!(id, &node_b_id); - }, - _ => panic!("Unexpected event"), + .expect("HTLC claim transaction not broadcast"); + check_spends!(htlc_tx, commitment_tx); } - nodes[1].node.get_and_clear_pending_events(); let monitor_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events(); assert!(monitor_events.iter().all(|event| !matches!(event, Event::BumpTransaction(_)))); } +#[test] +fn test_manual_broadcast_skips_commitment_until_funding() { + do_test_manual_broadcast_skips_commitment_until_funding(true, true, true); + do_test_manual_broadcast_skips_commitment_until_funding(true, false, true); + do_test_manual_broadcast_skips_commitment_until_funding(true, false, false); + do_test_manual_broadcast_skips_commitment_until_funding(false, true, true); + do_test_manual_broadcast_skips_commitment_until_funding(false, false, true); + do_test_manual_broadcast_skips_commitment_until_funding(false, false, false); +} + fn do_test_multi_post_event_actions(do_reload: bool) { // Tests handling multiple post-Event actions at once. // There is specific code in ChannelManager to handle channels where multiple post-Event