Disconnect peer when force closing a funded channel with an error

We do this to ensure that the counterparty will always broadcast their
latest state when we broadcast ours. Usually, they'll do this with the
`error` message alone, but if they don't receive it or ignore it, then
we'll force them to broadcast by sending them a bogus
`channel_reestablish` upon reconnecting. Note that this doesn't apply to
unfunded channels as there is no commitment transaction to broadcast.
This commit is contained in:
Wilmer Paulino 2023-10-13 09:28:35 -07:00
parent 9f7de472fb
commit 94e0ecec68
No known key found for this signature in database
GPG key ID: 634FE5FC544DCA31
4 changed files with 58 additions and 33 deletions

View file

@ -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<msgs::ChannelUpdate>, 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() })
},
}
);
@ -6928,8 +6929,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() })
},
});
}
@ -7713,10 +7714,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;
}

View file

@ -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<Node<'a, '
nodes[b].node.handle_error(&nodes[a].node.get_our_node_id(), msg);
}
},
MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::DisconnectPeer { ref msg } } => {
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<Node<'a, '
assert_eq!(node_id, nodes[a].node.get_our_node_id());
assert_eq!(msg.data, expected_error);
},
MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::DisconnectPeer { ref msg } } => {
assert_eq!(node_id, nodes[a].node.get_our_node_id());
assert_eq!(msg.as_ref().unwrap().data, expected_error);
},
_ => panic!("Unexpected event"),
}
}

View file

@ -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);

View file

@ -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!"),
}