Send funding_signed messages out-of-band to ensure ordered delivery

diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs
index 8b900880f87382805fdd72145daebd55eff8a599..6f18d071b95a95adb09cd3507c31a2085c2a1c84 100644 (file)
--- a/fuzz/fuzz_targets/full_stack_target.rs
+++ b/fuzz/fuzz_targets/full_stack_target.rs
@@ -804,7 +804,7 @@ mod tests {
 
                let log_entries = logger.lines.lock().unwrap();
                assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendAcceptChannel event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000000 for channel ff4f00f805273c1b203bb5ebf8436bfde57b3be8c2f5e95d9491dbb181909679".to_string())), Some(&1)); // 1
-               assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Encoding and sending message of type 35 to 030000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 2
+               assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendFundingSigned event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000000 for channel 3d00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 2
                assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendFundingLocked event in peer_handler for node 030000000000000000000000000000000000000000000000000000000000000000 for channel 3d00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 3
                assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Handling SendFundingLocked event in peer_handler for node 030200000000000000000000000000000000000000000000000000000000000000 for channel 3f00000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&1)); // 4
                assert_eq!(log_entries.get(&("lightning::ln::peer_handler".to_string(), "Encoding and sending message of type 133 to 030000000000000000000000000000000000000000000000000000000000000000".to_string())), Some(&4)); // 5

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 0b302e60b2885f48e0c8f4c9338202614ec498be..91bfec9600c6bdbaa7b8b80f8421b6c357c8d9b1 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -1666,7 +1666,7 @@ impl ChannelManager {
                Ok(())
        }
 
-       fn internal_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, MsgHandleErrInternal> {
+       fn internal_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), MsgHandleErrInternal> {
                let (chan, funding_msg, monitor_update) = {
                        let mut channel_state = self.channel_state.lock().unwrap();
                        match channel_state.by_id.entry(msg.temporary_channel_id.clone()) {
@@ -1692,16 +1692,21 @@ impl ChannelManager {
                if let Err(_e) = self.monitor.add_update_monitor(monitor_update.get_funding_txo().unwrap(), monitor_update) {
                        unimplemented!();
                }
-               let mut channel_state = self.channel_state.lock().unwrap();
+               let mut channel_state_lock = self.channel_state.lock().unwrap();
+               let channel_state = channel_state_lock.borrow_parts();
                match channel_state.by_id.entry(funding_msg.channel_id) {
                        hash_map::Entry::Occupied(_) => {
                                return Err(MsgHandleErrInternal::send_err_msg_no_close("Already had channel with the new channel_id", funding_msg.channel_id))
                        },
                        hash_map::Entry::Vacant(e) => {
+                               channel_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
+                                       node_id: their_node_id.clone(),
+                                       msg: funding_msg,
+                               });
                                e.insert(chan);
                        }
                }
-               Ok(funding_msg)
+               Ok(())
        }
 
        fn internal_funding_signed(&self, their_node_id: &PublicKey, msg: &msgs::FundingSigned) -> Result<(), MsgHandleErrInternal> {
@@ -2467,7 +2472,7 @@ impl ChannelMessageHandler for ChannelManager {
                handle_error!(self, self.internal_accept_channel(their_node_id, msg), their_node_id)
        }
 
-       fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, HandleError> {
+       fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &msgs::FundingCreated) -> Result<(), HandleError> {
                handle_error!(self, self.internal_funding_created(their_node_id, msg), their_node_id)
        }
 
@@ -2893,22 +2898,15 @@ mod tests {
                        _ => panic!("Unexpected event"),
                }
 
-               let events_3 = node_a.node.get_and_clear_pending_msg_events();
-               assert_eq!(events_3.len(), 1);
-               let funding_signed = match events_3[0] {
-                       MessageSendEvent::SendFundingCreated { ref node_id, ref msg } => {
-                               assert_eq!(*node_id, node_b.node.get_our_node_id());
-                               let res = node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), msg).unwrap();
-                               let mut added_monitors = node_b.chan_monitor.added_monitors.lock().unwrap();
-                               assert_eq!(added_monitors.len(), 1);
-                               assert_eq!(added_monitors[0].0, funding_output);
-                               added_monitors.clear();
-                               res
-                       },
-                       _ => panic!("Unexpected event"),
-               };
+               node_b.node.handle_funding_created(&node_a.node.get_our_node_id(), &get_event_msg!(node_a, MessageSendEvent::SendFundingCreated, node_b.node.get_our_node_id())).unwrap();
+               {
+                       let mut added_monitors = node_b.chan_monitor.added_monitors.lock().unwrap();
+                       assert_eq!(added_monitors.len(), 1);
+                       assert_eq!(added_monitors[0].0, funding_output);
+                       added_monitors.clear();
+               }
 
-               node_a.node.handle_funding_signed(&node_b.node.get_our_node_id(), &funding_signed).unwrap();
+               node_a.node.handle_funding_signed(&node_b.node.get_our_node_id(), &get_event_msg!(node_b, MessageSendEvent::SendFundingSigned, node_a.node.get_our_node_id())).unwrap();
                {
                        let mut added_monitors = node_a.chan_monitor.added_monitors.lock().unwrap();
                        assert_eq!(added_monitors.len(), 1);

diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs
index 89c95b3b917e895d81194a283c1f88f1aa9a1235..3ed0cde1f5d6bb7f641a30bd3a3d96687e5d26ef 100644 (file)
--- a/src/ln/msgs.rs
+++ b/src/ln/msgs.rs
@@ -212,6 +212,7 @@ pub struct AcceptChannel {
 }
 
 /// A funding_created message to be sent or received from a peer
+#[derive(Clone)]
 pub struct FundingCreated {
        pub(crate) temporary_channel_id: [u8; 32],
        pub(crate) funding_txid: Sha256dHash,
@@ -220,6 +221,7 @@ pub struct FundingCreated {
 }
 
 /// A funding_signed message to be sent or received from a peer
+#[derive(Clone)]
 pub struct FundingSigned {
        pub(crate) channel_id: [u8; 32],
        pub(crate) signature: Signature,
@@ -530,7 +532,7 @@ pub trait ChannelMessageHandler : events::MessageSendEventsProvider + Send + Syn
        /// Handle an incoming accept_channel message from the given peer.
        fn handle_accept_channel(&self, their_node_id: &PublicKey, msg: &AcceptChannel) -> Result<(), HandleError>;
        /// Handle an incoming funding_created message from the given peer.
-       fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &FundingCreated) -> Result<FundingSigned, HandleError>;
+       fn handle_funding_created(&self, their_node_id: &PublicKey, msg: &FundingCreated) -> Result<(), HandleError>;
        /// Handle an incoming funding_signed message from the given peer.
        fn handle_funding_signed(&self, their_node_id: &PublicKey, msg: &FundingSigned) -> Result<(), HandleError>;
        /// Handle an incoming funding_locked message from the given peer.

diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs
index 85a353a2e332492133fcbc950a02fc7e9ba69ab5..15cda5342900fe474571ef3a5584084ee056c10b 100644 (file)
--- a/src/ln/peer_handler.rs
+++ b/src/ln/peer_handler.rs
@@ -568,8 +568,7 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
 
                                                                                        34 => {
                                                                                                let msg = try_potential_decodeerror!(msgs::FundingCreated::read(&mut reader));
-                                                                                               let resp = try_potential_handleerror!(self.message_handler.chan_handler.handle_funding_created(&peer.their_node_id.unwrap(), &msg));
-                                                                                               encode_and_send_msg!(resp, 35);
+                                                                                               try_potential_handleerror!(self.message_handler.chan_handler.handle_funding_created(&peer.their_node_id.unwrap(), &msg));
                                                                                        },
                                                                                        35 => {
                                                                                                let msg = try_potential_decodeerror!(msgs::FundingSigned::read(&mut reader));
@@ -818,6 +817,17 @@ impl<Descriptor: SocketDescriptor> PeerManager<Descriptor> {
                                                peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 34)));
                                                Self::do_attempt_write_data(&mut descriptor, peer);
                                        },
+                                       MessageSendEvent::SendFundingSigned { ref node_id, ref msg } => {
+                                               log_trace!(self, "Handling SendFundingSigned event in peer_handler for node {} for channel {}",
+                                                               log_pubkey!(node_id),
+                                                               log_bytes!(msg.channel_id));
+                                               let (mut descriptor, peer) = get_peer_for_forwarding!(node_id, {
+                                                               //TODO: generate a DiscardFunding event indicating to the wallet that
+                                                               //they should just throw away this funding transaction
+                                                       });
+                                               peer.pending_outbound_buffer.push_back(peer.channel_encryptor.encrypt_message(&encode_msg!(msg, 35)));
+                                               Self::do_attempt_write_data(&mut descriptor, peer);
+                                       },
                                        MessageSendEvent::SendFundingLocked { ref node_id, ref msg, ref announcement_sigs } => {
                                                log_trace!(self, "Handling SendFundingLocked event in peer_handler for node {}{} for channel {}",
                                                                log_pubkey!(node_id),

diff --git a/src/util/events.rs b/src/util/events.rs
index ce6e5d99fab8c6f773473da96a66bc6493becc13..1ba3f68d12e9f909bf540cdc3e13783cca18fd1e 100644 (file)
--- a/src/util/events.rs
+++ b/src/util/events.rs
@@ -125,6 +125,13 @@ pub enum MessageSendEvent {
                /// The message which should be sent.
                msg: msgs::FundingCreated,
        },
+       /// Used to indicate that a funding_signed message should be sent to the peer with the given node_id.
+       SendFundingSigned {
+               /// The node_id of the node which should receive this message
+               node_id: PublicKey,
+               /// The message which should be sent.
+               msg: msgs::FundingSigned,
+       },
        /// Used to indicate that a funding_locked message should be sent to the peer with the given node_id.
        SendFundingLocked {
                /// The node_id of the node which should receive these message(s)

diff --git a/src/util/test_utils.rs b/src/util/test_utils.rs
index 0795162e228e6b6ebd5170bcbe971a019fd4b244..31aa77be1b4f6f896e9b2bde1b85f1ddc39dd6fa 100644 (file)
--- a/src/util/test_utils.rs
+++ b/src/util/test_utils.rs
@@ -92,7 +92,7 @@ impl msgs::ChannelMessageHandler for TestChannelMessageHandler {
        fn handle_accept_channel(&self, _their_node_id: &PublicKey, _msg: &msgs::AcceptChannel) -> Result<(), HandleError> {
                Err(HandleError { err: "", action: None })
        }
-       fn handle_funding_created(&self, _their_node_id: &PublicKey, _msg: &msgs::FundingCreated) -> Result<msgs::FundingSigned, HandleError> {
+       fn handle_funding_created(&self, _their_node_id: &PublicKey, _msg: &msgs::FundingCreated) -> Result<(), HandleError> {
                Err(HandleError { err: "", action: None })
        }
        fn handle_funding_signed(&self, _their_node_id: &PublicKey, _msg: &msgs::FundingSigned) -> Result<(), HandleError> {