From c962a27156c92888f9f3cb0bc6565a73525c8da8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 19 Oct 2018 17:06:40 -0400 Subject: [PATCH] Send funding_signed messages out-of-band to ensure ordered delivery --- fuzz/fuzz_targets/full_stack_target.rs | 2 +- src/ln/channelmanager.rs | 36 ++++++++++++-------------- src/ln/msgs.rs | 4 ++- src/ln/peer_handler.rs | 14 ++++++++-- src/util/events.rs | 7 +++++ src/util/test_utils.rs | 2 +- 6 files changed, 41 insertions(+), 24 deletions(-) diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs index 8b900880f..6f18d071b 100644 --- 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 0b302e60b..91bfec960 100644 --- 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 { + 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 { + 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 89c95b3b9..3ed0cde1f 100644 --- 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; + 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 85a353a2e..15cda5342 100644 --- a/src/ln/peer_handler.rs +++ b/src/ln/peer_handler.rs @@ -568,8 +568,7 @@ impl PeerManager { 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 PeerManager { 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 ce6e5d99f..1ba3f68d1 100644 --- 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 0795162e2..31aa77be1 100644 --- 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 { + 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> { -- 2.39.5