diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a61a8de82de..2359d1ef0e3 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -3992,7 +3992,7 @@ impl Channel where if msg.next_local_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_remote_commitment_number >= INITIAL_COMMITMENT_NUMBER || msg.next_local_commitment_number == 0 { - return Err(ChannelError::Close("Peer sent a garbage channel_reestablish (usually an lnd node with lost state asking us to force-close for them)".to_owned())); + return Err(ChannelError::Close("Peer sent an invalid channel_reestablish to force close in a non-standard way".to_owned())); } if msg.next_remote_commitment_number > 0 { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 24ca65af008..203d8f09e7d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -447,16 +447,17 @@ impl MsgHandleErrInternal { } #[inline] fn from_finish_shutdown(err: String, channel_id: ChannelId, user_channel_id: u128, shutdown_res: ShutdownResult, channel_update: Option, channel_capacity: u64) -> Self { + let err_msg = msgs::ErrorMessage { channel_id, data: err.clone() }; + let action = if let (Some(_), ..) = &shutdown_res { + // We have a closing `ChannelMonitorUpdate`, which means the channel was funded and we + // should disconnect our peer such that we force them to broadcast their latest + // commitment upon reconnecting. + msgs::ErrorAction::DisconnectPeer { msg: Some(err_msg) } + } else { + msgs::ErrorAction::SendErrorMessage { msg: err_msg } + }; Self { - err: LightningError { - err: err.clone(), - action: msgs::ErrorAction::SendErrorMessage { - msg: msgs::ErrorMessage { - channel_id, - data: err - }, - }, - }, + err: LightningError { err, action }, chan_id: Some((channel_id, user_channel_id)), shutdown_finish: Some((shutdown_res, channel_update)), channel_capacity: Some(channel_capacity) @@ -2812,8 +2813,8 @@ where peer_state.pending_msg_events.push( events::MessageSendEvent::HandleError { node_id: counterparty_node_id, - action: msgs::ErrorAction::SendErrorMessage { - msg: msgs::ErrorMessage { channel_id: *channel_id, data: "Channel force-closed".to_owned() } + action: msgs::ErrorAction::DisconnectPeer { + msg: Some(msgs::ErrorMessage { channel_id: *channel_id, data: "Channel force-closed".to_owned() }) }, } ); @@ -4296,8 +4297,9 @@ where } } } - let (counterparty_node_id, forward_chan_id) = match self.short_to_chan_info.read().unwrap().get(&short_chan_id) { - Some((cp_id, chan_id)) => (cp_id.clone(), chan_id.clone()), + let chan_info_opt = self.short_to_chan_info.read().unwrap().get(&short_chan_id).cloned(); + let (counterparty_node_id, forward_chan_id) = match chan_info_opt { + Some((cp_id, chan_id)) => (cp_id, chan_id), None => { forwarding_channel_not_found!(); continue; @@ -6785,7 +6787,10 @@ where let peer_state_mutex = per_peer_state.get(counterparty_node_id) .ok_or_else(|| { debug_assert!(false); - MsgHandleErrInternal::send_err_msg_no_close(format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), msg.channel_id) + MsgHandleErrInternal::send_err_msg_no_close( + format!("Can't find a peer matching the passed counterparty node_id {}", counterparty_node_id), + msg.channel_id + ) })?; let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; @@ -6829,7 +6834,39 @@ where "Got a channel_reestablish message for an unfunded channel!".into())), chan_phase_entry); } }, - hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.channel_id)) + hash_map::Entry::Vacant(_) => { + log_debug!(self.logger, "Sending bogus ChannelReestablish for unknown channel {} to force channel closure", + log_bytes!(msg.channel_id.0)); + // Unfortunately, lnd doesn't force close on errors + // (https://github.com/lightningnetwork/lnd/blob/abb1e3463f3a83bbb843d5c399869dbe930ad94f/htlcswitch/link.go#L2119). + // One of the few ways to get an lnd counterparty to force close is by + // replicating what they do when restoring static channel backups (SCBs). They + // send an invalid `ChannelReestablish` with `0` commitment numbers and an + // invalid `your_last_per_commitment_secret`. + // + // Since we received a `ChannelReestablish` for a channel that doesn't exist, we + // can assume it's likely the channel closed from our point of view, but it + // remains open on the counterparty's side. By sending this bogus + // `ChannelReestablish` message now as a response to theirs, we trigger them to + // force close broadcasting their latest state. If the closing transaction from + // our point of view remains unconfirmed, it'll enter a race with the + // counterparty's to-be-broadcast latest commitment transaction. + peer_state.pending_msg_events.push(MessageSendEvent::SendChannelReestablish { + node_id: *counterparty_node_id, + msg: msgs::ChannelReestablish { + channel_id: msg.channel_id, + next_local_commitment_number: 0, + next_remote_commitment_number: 0, + your_last_per_commitment_secret: [1u8; 32], + my_current_per_commitment_point: PublicKey::from_slice(&[2u8; 33]).unwrap(), + next_funding_txid: None, + }, + }); + return Err(MsgHandleErrInternal::send_err_msg_no_close( + format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", + counterparty_node_id), msg.channel_id) + ) + } } }; @@ -6893,8 +6930,8 @@ where self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: chan.context.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage { - msg: msgs::ErrorMessage { channel_id: chan.context.channel_id(), data: "Channel force-closed".to_owned() } + action: msgs::ErrorAction::DisconnectPeer { + msg: Some(msgs::ErrorMessage { channel_id: chan.context.channel_id(), data: "Channel force-closed".to_owned() }) }, }); } @@ -7678,10 +7715,12 @@ where self.issue_channel_close_events(&channel.context, reason); pending_msg_events.push(events::MessageSendEvent::HandleError { node_id: channel.context.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { - channel_id: channel.context.channel_id(), - data: reason_message, - } }, + action: msgs::ErrorAction::DisconnectPeer { + msg: Some(msgs::ErrorMessage { + channel_id: channel.context.channel_id(), + data: reason_message, + }) + }, }); return false; } @@ -11219,6 +11258,67 @@ mod tests { let payment_preimage = PaymentPreimage([42; 32]); assert_eq!(format!("{}", &payment_preimage), "2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a2a"); } + + #[test] + fn test_trigger_lnd_force_close() { + let chanmon_cfg = create_chanmon_cfgs(2); + let node_cfg = create_node_cfgs(2, &chanmon_cfg); + let user_config = test_default_channel_config(); + let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config), Some(user_config)]); + let nodes = create_network(2, &node_cfg, &node_chanmgr); + + // Open a channel, immediately disconnect each other, and broadcast Alice's latest state. + let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes(&nodes, 0, 1); + nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id()); + nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id()); + nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id()).unwrap(); + check_closed_broadcast(&nodes[0], 1, true); + check_added_monitors(&nodes[0], 1); + check_closed_event!(nodes[0], 1, ClosureReason::HolderForceClosed, [nodes[1].node.get_our_node_id()], 100000); + { + let txn = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + } + + // Since they're disconnected, Bob won't receive Alice's `Error` message. Reconnect them + // such that Bob sends a `ChannelReestablish` to Alice since the channel is still open from + // their side. + nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { + features: nodes[1].node.init_features(), networks: None, remote_network_address: None + }, true).unwrap(); + nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { + features: nodes[0].node.init_features(), networks: None, remote_network_address: None + }, false).unwrap(); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + let channel_reestablish = get_event_msg!( + nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id() + ); + nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &channel_reestablish); + + // Alice should respond with an error since the channel isn't known, but a bogus + // `ChannelReestablish` should be sent first, such that we actually trigger Bob to force + // close even if it was an lnd node. + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + if let MessageSendEvent::SendChannelReestablish { node_id, msg } = &msg_events[0] { + assert_eq!(*node_id, nodes[1].node.get_our_node_id()); + assert_eq!(msg.next_local_commitment_number, 0); + assert_eq!(msg.next_remote_commitment_number, 0); + nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &msg); + } else { panic!() }; + check_closed_broadcast(&nodes[1], 1, true); + check_added_monitors(&nodes[1], 1); + let expected_close_reason = ClosureReason::ProcessingError { + err: "Peer sent an invalid channel_reestablish to force close in a non-standard way".to_string() + }; + check_closed_event!(nodes[1], 1, expected_close_reason, [nodes[0].node.get_our_node_id()], 100000); + { + let txn = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(txn.len(), 1); + check_spends!(txn[0], funding_tx); + } + } } #[cfg(ldk_bench)] diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index acf1c7dfb70..42403f89c7d 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -665,6 +665,12 @@ pub fn get_err_msg(node: &Node, recipient: &PublicKey) -> msgs::ErrorMessage { assert_eq!(node_id, recipient); (*msg).clone() }, + MessageSendEvent::HandleError { + action: msgs::ErrorAction::DisconnectPeer { ref msg }, ref node_id + } => { + assert_eq!(node_id, recipient); + msg.as_ref().unwrap().clone() + }, _ => panic!("Unexpected event"), } } @@ -1446,10 +1452,15 @@ pub fn check_closed_broadcast(node: &Node, num_channels: usize, with_error_msg: assert_eq!(msg.contents.flags & 2, 2); None }, - MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => { + MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { msg }, node_id: _ } => { assert!(with_error_msg); // TODO: Check node_id - Some(msg.clone()) + Some(msg) + }, + MessageSendEvent::HandleError { action: msgs::ErrorAction::DisconnectPeer { msg }, node_id: _ } => { + assert!(with_error_msg); + // TODO: Check node_id + Some(msg.unwrap()) }, _ => panic!("Unexpected event"), } @@ -2921,6 +2932,13 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec { + assert_eq!(node_id, nodes[b].node.get_our_node_id()); + assert_eq!(msg.as_ref().unwrap().data, expected_error); + if needs_err_handle { + nodes[b].node.handle_error(&nodes[a].node.get_our_node_id(), msg.as_ref().unwrap()); + } + }, _ => panic!("Unexpected event"), } @@ -2938,6 +2956,10 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec { + assert_eq!(node_id, nodes[a].node.get_our_node_id()); + assert_eq!(msg.as_ref().unwrap().data, expected_error); + }, _ => panic!("Unexpected event"), } } diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 5e90ee7e92e..cabbba8f3ae 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -1338,9 +1338,9 @@ fn test_duplicate_htlc_different_direction_onchain() { for e in events { match e { MessageSendEvent::BroadcastChannelUpdate { .. } => {}, - MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { + MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::DisconnectPeer { ref msg } } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()); - assert_eq!(msg.data, "Channel closed because commitment or closing transaction was confirmed on chain."); + assert_eq!(msg.as_ref().unwrap().data, "Channel closed because commitment or closing transaction was confirmed on chain."); }, MessageSendEvent::UpdateHTLCs { ref node_id, updates: msgs::CommitmentUpdate { ref update_add_htlcs, ref update_fulfill_htlcs, ref update_fail_htlcs, ref update_fail_malformed_htlcs, .. } } => { assert!(update_add_htlcs.is_empty()); @@ -2369,7 +2369,7 @@ fn channel_monitor_network_test() { _ => panic!("Unexpected event"), }; match events[1] { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => { + MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id } => { assert_eq!(node_id, nodes[4].node.get_our_node_id()); }, _ => panic!("Unexpected event"), @@ -2401,7 +2401,7 @@ fn channel_monitor_network_test() { _ => panic!("Unexpected event"), }; match events[1] { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => { + MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id } => { assert_eq!(node_id, nodes[3].node.get_our_node_id()); }, _ => panic!("Unexpected event"), @@ -2913,7 +2913,7 @@ fn test_htlc_on_chain_success() { let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events); match nodes_2_event { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {}, + MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id: _ } => {}, _ => panic!("Unexpected event"), } @@ -3358,7 +3358,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); match nodes_2_event { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => { + MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { msg: Some(msgs::ErrorMessage { channel_id, ref data }) }, node_id: _ } => { assert_eq!(channel_id, chan_2.2); assert_eq!(data.as_str(), "Channel closed because commitment or closing transaction was confirmed on chain."); }, @@ -4920,7 +4920,7 @@ fn test_onchain_to_onchain_claim() { let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut msg_events); match nodes_2_event { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {}, + MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id: _ } => {}, _ => panic!("Unexpected event"), } @@ -7860,9 +7860,9 @@ fn test_channel_conf_timeout() { let close_ev = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(close_ev.len(), 1); match close_ev[0] { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, ref node_id } => { + MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { ref msg }, ref node_id } => { assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - assert_eq!(msg.data, "Channel closed because funding transaction failed to confirm within 2016 blocks"); + assert_eq!(msg.as_ref().unwrap().data, "Channel closed because funding transaction failed to confirm within 2016 blocks"); }, _ => panic!("Unexpected event"), } @@ -9212,8 +9212,8 @@ fn test_invalid_funding_tx() { assert_eq!(events_2.len(), 1); if let MessageSendEvent::HandleError { node_id, action } = &events_2[0] { assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - if let msgs::ErrorAction::SendErrorMessage { msg } = action { - assert_eq!(msg.data, "Channel closed because of an exception: ".to_owned() + expected_err); + if let msgs::ErrorAction::DisconnectPeer { msg } = action { + assert_eq!(msg.as_ref().unwrap().data, "Channel closed because of an exception: ".to_owned() + expected_err); } else { panic!(); } } else { panic!(); } assert_eq!(nodes[1].node.list_channels().len(), 0); @@ -10652,7 +10652,7 @@ fn do_test_funding_and_commitment_tx_confirm_same_block(confirm_remote_commitmen let mut msg_events = closing_node.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), 1); match msg_events.pop().unwrap() { - MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { .. }, .. } => {}, + MessageSendEvent::HandleError { action: msgs::ErrorAction::DisconnectPeer { .. }, .. } => {}, _ => panic!("Unexpected event"), } check_added_monitors(closing_node, 1); diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 26ecbb0bd24..f35b67e98f1 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -706,8 +706,8 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) { let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap(); nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish); let as_err = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(as_err.len(), 1); - match as_err[0] { + assert_eq!(as_err.len(), 2); + match as_err[1] { MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()); nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), msg); @@ -881,9 +881,9 @@ fn do_test_completed_payment_not_retryable_on_reload(use_dust: bool) { let bs_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]).pop().unwrap(); nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &bs_reestablish); let as_err = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(as_err.len(), 1); + assert_eq!(as_err.len(), 2); let bs_commitment_tx; - match as_err[0] { + match as_err[1] { MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => { assert_eq!(node_id, nodes[1].node.get_our_node_id()); nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), msg); diff --git a/lightning/src/ln/reload_tests.rs b/lightning/src/ln/reload_tests.rs index 5f4009d6429..58da52eb743 100644 --- a/lightning/src/ln/reload_tests.rs +++ b/lightning/src/ln/reload_tests.rs @@ -566,8 +566,8 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) { if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg { } else if let MessageSendEvent::HandleError { ref action, .. } = msg { match action { - &ErrorAction::SendErrorMessage { ref msg } => { - assert_eq!(msg.data, "Channel force-closed"); + &ErrorAction::DisconnectPeer { ref msg } => { + assert_eq!(msg.as_ref().unwrap().data, "Channel force-closed"); }, _ => panic!("Unexpected event!"), } @@ -589,18 +589,16 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) { nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &retry_reestablish[0]); let mut err_msgs_0 = Vec::with_capacity(1); - for msg in nodes[0].node.get_and_clear_pending_msg_events() { - if let MessageSendEvent::HandleError { ref action, .. } = msg { - match action { - &ErrorAction::SendErrorMessage { ref msg } => { - assert_eq!(msg.data, format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id())); - err_msgs_0.push(msg.clone()); - }, - _ => panic!("Unexpected event!"), - } - } else { - panic!("Unexpected event!"); + if let MessageSendEvent::HandleError { ref action, .. } = nodes[0].node.get_and_clear_pending_msg_events()[1] { + match action { + &ErrorAction::SendErrorMessage { ref msg } => { + assert_eq!(msg.data, format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", &nodes[1].node.get_our_node_id())); + err_msgs_0.push(msg.clone()); + }, + _ => panic!("Unexpected event!"), } + } else { + panic!("Unexpected event!"); } assert_eq!(err_msgs_0.len(), 1); nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &err_msgs_0[0]); diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index 47361693693..9bc133f45a9 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -623,8 +623,8 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) { nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &node_1_2nd_reestablish); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - if let MessageSendEvent::HandleError { ref action, .. } = msg_events[0] { + assert_eq!(msg_events.len(), 2); + if let MessageSendEvent::HandleError { ref action, .. } = msg_events[1] { match action { &ErrorAction::SendErrorMessage { ref msg } => { nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &msg);