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.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 35107f10bee9a47eeb4704b14651c6b24ca8037e..d1f561e266e1b3f6bcfe6ca54d8e5e54e5ea7da3 100644 (file)
--- 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<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;
                                                        }

diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs
index acf1c7dfb70e078c521cc088c880479adf7e420d..42403f89c7d17b267af97633a6e2b3b895315fa7 100644 (file)
--- 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<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"),
                }
        }

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 5e90ee7e92e31d0005dd65b35ac22c68a9eb9c98..cabbba8f3ae06914ffc209f4f1b56c02d5d09ed3 100644 (file)
--- 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/reload_tests.rs b/lightning/src/ln/reload_tests.rs
index 9ea13f13109635174565e650c8cde03b076543ba..58da52eb743c24fcf381c47cc90d9bfbf310b485 100644 (file)
--- 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!"),
                        }