Skip to content

Commit 1f0928f

Browse files
wpaulinowpaulino
committed
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.
1 parent f4601e3 commit 1f0928f

File tree

4 files changed

+55
-33
lines changed

4 files changed

+55
-33
lines changed

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -447,16 +447,14 @@ impl MsgHandleErrInternal {
447447
}
448448
#[inline]
449449
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 {
450+
let err_msg = msgs::ErrorMessage { channel_id, data: err.clone() };
451+
let action = if channel_update.is_some() {
452+
msgs::ErrorAction::DisconnectPeer { msg: Some(err_msg) }
453+
} else {
454+
msgs::ErrorAction::SendErrorMessage { msg: err_msg }
455+
};
450456
Self {
451-
err: LightningError {
452-
err: err.clone(),
453-
action: msgs::ErrorAction::SendErrorMessage {
454-
msg: msgs::ErrorMessage {
455-
channel_id,
456-
data: err
457-
},
458-
},
459-
},
457+
err: LightningError { err, action },
460458
chan_id: Some((channel_id, user_channel_id)),
461459
shutdown_finish: Some((shutdown_res, channel_update)),
462460
channel_capacity: Some(channel_capacity)
@@ -2812,8 +2810,8 @@ where
28122810
peer_state.pending_msg_events.push(
28132811
events::MessageSendEvent::HandleError {
28142812
node_id: counterparty_node_id,
2815-
action: msgs::ErrorAction::SendErrorMessage {
2816-
msg: msgs::ErrorMessage { channel_id: *channel_id, data: "Channel force-closed".to_owned() }
2813+
action: msgs::ErrorAction::DisconnectPeer {
2814+
msg: Some(msgs::ErrorMessage { channel_id: *channel_id, data: "Channel force-closed".to_owned() })
28172815
},
28182816
}
28192817
);
@@ -6930,8 +6928,8 @@ where
69306928
self.issue_channel_close_events(&chan.context, ClosureReason::HolderForceClosed);
69316929
pending_msg_events.push(events::MessageSendEvent::HandleError {
69326930
node_id: chan.context.get_counterparty_node_id(),
6933-
action: msgs::ErrorAction::SendErrorMessage {
6934-
msg: msgs::ErrorMessage { channel_id: chan.context.channel_id(), data: "Channel force-closed".to_owned() }
6931+
action: msgs::ErrorAction::DisconnectPeer {
6932+
msg: Some(msgs::ErrorMessage { channel_id: chan.context.channel_id(), data: "Channel force-closed".to_owned() })
69356933
},
69366934
});
69376935
}
@@ -7715,10 +7713,12 @@ where
77157713
self.issue_channel_close_events(&channel.context, reason);
77167714
pending_msg_events.push(events::MessageSendEvent::HandleError {
77177715
node_id: channel.context.get_counterparty_node_id(),
7718-
action: msgs::ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage {
7719-
channel_id: channel.context.channel_id(),
7720-
data: reason_message,
7721-
} },
7716+
action: msgs::ErrorAction::DisconnectPeer {
7717+
msg: Some(msgs::ErrorMessage {
7718+
channel_id: channel.context.channel_id(),
7719+
data: reason_message,
7720+
})
7721+
},
77227722
});
77237723
return false;
77247724
}

‎lightning/src/ln/functional_test_utils.rs‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,12 @@ pub fn get_err_msg(node: &Node, recipient: &PublicKey) -> msgs::ErrorMessage {
665665
assert_eq!(node_id, recipient);
666666
(*msg).clone()
667667
},
668+
MessageSendEvent::HandleError {
669+
action: msgs::ErrorAction::DisconnectPeer { ref msg }, ref node_id
670+
} => {
671+
assert_eq!(node_id, recipient);
672+
msg.as_ref().unwrap().clone()
673+
},
668674
_ => panic!("Unexpected event"),
669675
}
670676
}
@@ -1446,10 +1452,15 @@ pub fn check_closed_broadcast(node: &Node, num_channels: usize, with_error_msg:
14461452
assert_eq!(msg.contents.flags & 2, 2);
14471453
None
14481454
},
1449-
MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { ref msg }, node_id: _ } => {
1455+
MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { msg }, node_id: _ } => {
14501456
assert!(with_error_msg);
14511457
// TODO: Check node_id
1452-
Some(msg.clone())
1458+
Some(msg)
1459+
},
1460+
MessageSendEvent::HandleError { action: msgs::ErrorAction::DisconnectPeer { msg }, node_id: _ } => {
1461+
assert!(with_error_msg);
1462+
// TODO: Check node_id
1463+
Some(msg.unwrap())
14531464
},
14541465
_ => panic!("Unexpected event"),
14551466
}
@@ -2921,6 +2932,13 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec<Node<'a, '
29212932
nodes[b].node.handle_error(&nodes[a].node.get_our_node_id(), msg);
29222933
}
29232934
},
2935+
MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::DisconnectPeer { ref msg } } => {
2936+
assert_eq!(node_id, nodes[b].node.get_our_node_id());
2937+
assert_eq!(msg.as_ref().unwrap().data, expected_error);
2938+
if needs_err_handle {
2939+
nodes[b].node.handle_error(&nodes[a].node.get_our_node_id(), msg.as_ref().unwrap());
2940+
}
2941+
},
29242942
_ => panic!("Unexpected event"),
29252943
}
29262944

@@ -2938,6 +2956,10 @@ pub fn handle_announce_close_broadcast_events<'a, 'b, 'c>(nodes: &Vec<Node<'a, '
29382956
assert_eq!(node_id, nodes[a].node.get_our_node_id());
29392957
assert_eq!(msg.data, expected_error);
29402958
},
2959+
MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::DisconnectPeer { ref msg } } => {
2960+
assert_eq!(node_id, nodes[a].node.get_our_node_id());
2961+
assert_eq!(msg.as_ref().unwrap().data, expected_error);
2962+
},
29412963
_ => panic!("Unexpected event"),
29422964
}
29432965
}

‎lightning/src/ln/functional_tests.rs‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1338,9 +1338,9 @@ fn test_duplicate_htlc_different_direction_onchain() {
13381338
for e in events {
13391339
match e {
13401340
MessageSendEvent::BroadcastChannelUpdate { .. } => {},
1341-
MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::SendErrorMessage { ref msg } } => {
1341+
MessageSendEvent::HandleError { node_id, action: msgs::ErrorAction::DisconnectPeer { ref msg } } => {
13421342
assert_eq!(node_id, nodes[1].node.get_our_node_id());
1343-
assert_eq!(msg.data, "Channel closed because commitment or closing transaction was confirmed on chain.");
1343+
assert_eq!(msg.as_ref().unwrap().data, "Channel closed because commitment or closing transaction was confirmed on chain.");
13441344
},
13451345
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, .. } } => {
13461346
assert!(update_add_htlcs.is_empty());
@@ -2369,7 +2369,7 @@ fn channel_monitor_network_test() {
23692369
_ => panic!("Unexpected event"),
23702370
};
23712371
match events[1] {
2372-
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => {
2372+
MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id } => {
23732373
assert_eq!(node_id, nodes[4].node.get_our_node_id());
23742374
},
23752375
_ => panic!("Unexpected event"),
@@ -2401,7 +2401,7 @@ fn channel_monitor_network_test() {
24012401
_ => panic!("Unexpected event"),
24022402
};
24032403
match events[1] {
2404-
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id } => {
2404+
MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id } => {
24052405
assert_eq!(node_id, nodes[3].node.get_our_node_id());
24062406
},
24072407
_ => panic!("Unexpected event"),
@@ -2913,7 +2913,7 @@ fn test_htlc_on_chain_success() {
29132913
let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut events);
29142914

29152915
match nodes_2_event {
2916-
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
2916+
MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id: _ } => {},
29172917
_ => panic!("Unexpected event"),
29182918
}
29192919

@@ -3358,7 +3358,7 @@ fn do_test_commitment_revoked_fail_backward_exhaustive(deliver_bs_raa: bool, use
33583358

33593359
let nodes_2_event = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events);
33603360
match nodes_2_event {
3361-
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { msg: msgs::ErrorMessage { channel_id, ref data } }, node_id: _ } => {
3361+
MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { msg: Some(msgs::ErrorMessage { channel_id, ref data }) }, node_id: _ } => {
33623362
assert_eq!(channel_id, chan_2.2);
33633363
assert_eq!(data.as_str(), "Channel closed because commitment or closing transaction was confirmed on chain.");
33643364
},
@@ -4920,7 +4920,7 @@ fn test_onchain_to_onchain_claim() {
49204920
let nodes_0_event = remove_first_msg_event_to_node(&nodes[0].node.get_our_node_id(), &mut msg_events);
49214921

49224922
match nodes_2_event {
4923-
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { .. }, node_id: _ } => {},
4923+
MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { .. }, node_id: _ } => {},
49244924
_ => panic!("Unexpected event"),
49254925
}
49264926

@@ -7860,9 +7860,9 @@ fn test_channel_conf_timeout() {
78607860
let close_ev = nodes[1].node.get_and_clear_pending_msg_events();
78617861
assert_eq!(close_ev.len(), 1);
78627862
match close_ev[0] {
7863-
MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, ref node_id } => {
7863+
MessageSendEvent::HandleError { action: ErrorAction::DisconnectPeer { ref msg }, ref node_id } => {
78647864
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
7865-
assert_eq!(msg.data, "Channel closed because funding transaction failed to confirm within 2016 blocks");
7865+
assert_eq!(msg.as_ref().unwrap().data, "Channel closed because funding transaction failed to confirm within 2016 blocks");
78667866
},
78677867
_ => panic!("Unexpected event"),
78687868
}
@@ -9212,8 +9212,8 @@ fn test_invalid_funding_tx() {
92129212
assert_eq!(events_2.len(), 1);
92139213
if let MessageSendEvent::HandleError { node_id, action } = &events_2[0] {
92149214
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
9215-
if let msgs::ErrorAction::SendErrorMessage { msg } = action {
9216-
assert_eq!(msg.data, "Channel closed because of an exception: ".to_owned() + expected_err);
9215+
if let msgs::ErrorAction::DisconnectPeer { msg } = action {
9216+
assert_eq!(msg.as_ref().unwrap().data, "Channel closed because of an exception: ".to_owned() + expected_err);
92179217
} else { panic!(); }
92189218
} else { panic!(); }
92199219
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
1065210652
let mut msg_events = closing_node.node.get_and_clear_pending_msg_events();
1065310653
assert_eq!(msg_events.len(), 1);
1065410654
match msg_events.pop().unwrap() {
10655-
MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage { .. }, .. } => {},
10655+
MessageSendEvent::HandleError { action: msgs::ErrorAction::DisconnectPeer { .. }, .. } => {},
1065610656
_ => panic!("Unexpected event"),
1065710657
}
1065810658
check_added_monitors(closing_node, 1);

‎lightning/src/ln/reload_tests.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,8 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
566566
if let MessageSendEvent::BroadcastChannelUpdate { .. } = msg {
567567
} else if let MessageSendEvent::HandleError { ref action, .. } = msg {
568568
match action {
569-
&ErrorAction::SendErrorMessage { ref msg } => {
570-
assert_eq!(msg.data, "Channel force-closed");
569+
&ErrorAction::DisconnectPeer { ref msg } => {
570+
assert_eq!(msg.as_ref().unwrap().data, "Channel force-closed");
571571
},
572572
_ => panic!("Unexpected event!"),
573573
}

0 commit comments

Comments
 (0)