Send peers error messages for failures due to invalid batch funding

If we fail to fund a batch open we'll force-close all channels in
the batch, however would previously fail to send error messages to
our peers for any channels we were due to test after the one that
failed.

This commit fixes that issue, sending the required error messages
to allow our peers to clean up their state.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 3a025d069d45e4a1ffcd325fb95e8dd1aa83500d..07fe697a8f6d2ac508ee2bab75973d1b2c6150e5 100644 (file)
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -4726,11 +4726,20 @@ where
                                for (channel_id, counterparty_node_id) in channels_to_remove {
                                        per_peer_state.get(&counterparty_node_id)
                                                .map(|peer_state_mutex| peer_state_mutex.lock().unwrap())
-                                               .and_then(|mut peer_state| peer_state.channel_by_id.remove(&channel_id))
-                                               .map(|mut chan| {
+                                               .and_then(|mut peer_state| peer_state.channel_by_id.remove(&channel_id).map(|chan| (chan, peer_state)))
+                                               .map(|(mut chan, mut peer_state)| {
                                                        update_maps_on_chan_removal!(self, &chan.context());
                                                        let closure_reason = ClosureReason::ProcessingError { err: e.clone() };
                                                        shutdown_results.push(chan.context_mut().force_shutdown(false, closure_reason));
+                                                       peer_state.pending_msg_events.push(events::MessageSendEvent::HandleError {
+                                                               node_id: counterparty_node_id,
+                                                               action: msgs::ErrorAction::SendErrorMessage {
+                                                                       msg: msgs::ErrorMessage {
+                                                                               channel_id,
+                                                                               data: "Failed to fund channel".to_owned(),
+                                                                       }
+                                                               },
+                                                       });
                                                });
                                }
                        }

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 465d6288d9d3e764662edb2276d7bb5bef477cb3..151626f94ab5134f9e7bf16c174f21f48d69b24d 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -10110,14 +10110,9 @@ fn test_non_final_funding_tx() {
                },
                _ => panic!()
        }
-       let events = nodes[0].node.get_and_clear_pending_events();
-       assert_eq!(events.len(), 1);
-       match events[0] {
-               Event::ChannelClosed { channel_id, .. } => {
-                       assert_eq!(channel_id, temp_channel_id);
-               },
-               _ => panic!("Unexpected event"),
-       }
+       let err = "Error in transaction funding: Misuse error: Funding transaction absolute timelock is non-final".to_owned();
+       check_closed_events(&nodes[0], &[ExpectedCloseEvent::from_id_reason(temp_channel_id, false, ClosureReason::ProcessingError { err })]);
+       assert_eq!(get_err_msg(&nodes[0], &nodes[1].node.get_our_node_id()).data, "Failed to fund channel");
 }
 
 #[test]

diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs
index 9a44953f51e3fab76776f323ea91485a59ecc448..e4b69321eb724bb94913c1d403aa21c345e87ed3 100644 (file)
--- a/lightning/src/ln/shutdown_tests.rs
+++ b/lightning/src/ln/shutdown_tests.rs
@@ -1430,23 +1430,32 @@ fn batch_funding_failure() {
        nodes[0].node.batch_funding_transaction_generated(&chans, tx).unwrap_err();
 
        let msgs = nodes[0].node.get_and_clear_pending_msg_events();
-       assert_eq!(msgs.len(), 2);
+       assert_eq!(msgs.len(), 3);
        // We currently spuriously send `FundingCreated` for the first channel and then immediately
        // fail both channels, which isn't ideal but should be fine.
        assert!(msgs.iter().any(|msg| {
                if let MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage {
                        msg: msgs::ErrorMessage { channel_id, .. }, ..
                }, .. } = msg {
-                       assert_eq!(*channel_id, temp_chan_id_b);
-                       true
+                       *channel_id == temp_chan_id_b
                } else { false }
        }));
-       assert!(msgs.iter().any(|msg| {
+       let funding_created_pos = msgs.iter().position(|msg| {
                if let MessageSendEvent::SendFundingCreated { msg: msgs::FundingCreated { temporary_channel_id, .. }, .. } = msg {
                        assert_eq!(*temporary_channel_id, temp_chan_id_a);
                        true
                } else { false }
-       }));
+       }).unwrap();
+       let funded_channel_close_pos = msgs.iter().position(|msg| {
+               if let MessageSendEvent::HandleError { action: msgs::ErrorAction::SendErrorMessage {
+                       msg: msgs::ErrorMessage { channel_id, .. }, ..
+               }, .. } = msg {
+                       *channel_id == post_funding_chan_id_a
+               } else { false }
+       }).unwrap();
+
+       // The error message uses the funded channel_id so must come after the funding_created
+       assert!(funded_channel_close_pos > funding_created_pos);
 
        check_closed_events(&nodes[0], &close);
        assert_eq!(nodes[0].node.list_channels().len(), 0);