From 675cf4ac1d02b2b558a0e041d6cd4bebac0e5108 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 9 Jul 2019 13:00:15 -0400 Subject: [PATCH] Support option_upfront_shutdown_script for local peer Track shutdown_pubkey of peer at open/accept_channel messages Fix encoding_init test --- src/ln/channel.rs | 49 +++++++++++++++++++++++++++++++++++++++--- src/ln/msgs.rs | 17 ++++++++++----- src/ln/peer_handler.rs | 4 ---- 3 files changed, 58 insertions(+), 12 deletions(-) diff --git a/src/ln/channel.rs b/src/ln/channel.rs index cee70e5c..bf5d3576 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -522,7 +522,7 @@ impl Channel { /// Creates a new channel from a remote sides' request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! - pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc, their_node_id: PublicKey, _their_local_features: LocalFeatures, msg: &msgs::OpenChannel, user_id: u64, logger: Arc, config: &UserConfig) -> Result { + pub fn new_from_req(fee_estimator: &FeeEstimator, keys_provider: &Arc, their_node_id: PublicKey, their_local_features: LocalFeatures, msg: &msgs::OpenChannel, user_id: u64, logger: Arc, config: &UserConfig) -> Result { let chan_keys = keys_provider.get_channel_keys(true); let mut local_config = (*config).channel_options.clone(); @@ -625,6 +625,27 @@ impl Channel { channel_monitor.set_their_base_keys(&msg.htlc_basepoint, &msg.delayed_payment_basepoint); channel_monitor.set_their_to_self_delay(msg.to_self_delay); + let their_shutdown_scriptpubkey = if their_local_features.supports_upfront_shutdown_script() { + match &msg.shutdown_scriptpubkey { + &OptionalField::Present(ref script) => { + // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. We enforce it while receiving shutdown msg + if script.is_p2pkh() || script.is_p2sh() || script.is_v0_p2wsh() || script.is_v0_p2wpkh() { + Some(script.clone()) + // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything + } else if script.len() == 0 { + None + // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel + } else { + return Err(ChannelError::Close("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format")); + } + }, + // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel + &OptionalField::Absent => { + return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out")); + } + } + } else { None }; + let mut chan = Channel { user_id: user_id, config: local_config, @@ -692,7 +713,7 @@ impl Channel { their_prev_commitment_point: None, their_node_id: their_node_id, - their_shutdown_scriptpubkey: None, + their_shutdown_scriptpubkey, channel_monitor: channel_monitor, @@ -1341,7 +1362,7 @@ impl Channel { // Message handlers: - pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, _their_local_features: LocalFeatures) -> Result<(), ChannelError> { + pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_local_features: LocalFeatures) -> Result<(), ChannelError> { // Check sanity of message fields: if !self.channel_outbound { return Err(ChannelError::Close("Got an accept_channel message from an inbound peer")); @@ -1400,6 +1421,27 @@ impl Channel { return Err(ChannelError::Close("We consider the minimum depth to be unreasonably large")); } + let their_shutdown_scriptpubkey = if their_local_features.supports_upfront_shutdown_script() { + match &msg.shutdown_scriptpubkey { + &OptionalField::Present(ref script) => { + // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. We enforce it while receiving shutdown msg + if script.is_p2pkh() || script.is_p2sh() || script.is_v0_p2wsh() || script.is_v0_p2wpkh() { + Some(script.clone()) + // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything + } else if script.len() == 0 { + None + // Peer is signaling upfront_shutdown and has provided a non-accepted scriptpubkey format. Fail the channel + } else { + return Err(ChannelError::Close("Peer is signaling upfront_shutdown but has provided a non-accepted scriptpubkey format")); + } + }, + // Peer is signaling upfront shutdown but don't opt-out with correct mechanism (a.k.a 0-length script). Peer looks buggy, we fail the channel + &OptionalField::Absent => { + return Err(ChannelError::Close("Peer is signaling upfront_shutdown but we don't get any script. Use 0-length script to opt-out")); + } + } + } else { None }; + self.channel_monitor.set_their_base_keys(&msg.htlc_basepoint, &msg.delayed_payment_basepoint); self.their_dust_limit_satoshis = msg.dust_limit_satoshis; @@ -1415,6 +1457,7 @@ impl Channel { self.their_delayed_payment_basepoint = Some(msg.delayed_payment_basepoint); self.their_htlc_basepoint = Some(msg.htlc_basepoint); self.their_cur_commitment_point = Some(msg.first_per_commitment_point); + self.their_shutdown_scriptpubkey = their_shutdown_scriptpubkey; let obscure_factor = self.get_commitment_transaction_number_obscure_factor(); self.channel_monitor.set_commitment_obscure_factor(obscure_factor); diff --git a/src/ln/msgs.rs b/src/ln/msgs.rs index 10e83e08..6c910865 100644 --- a/src/ln/msgs.rs +++ b/src/ln/msgs.rs @@ -60,9 +60,16 @@ pub struct LocalFeatures { impl LocalFeatures { /// Create a blank LocalFeatures flags (visibility extended for fuzz tests) + #[cfg(not(feature = "fuzztarget"))] + pub(crate) fn new() -> LocalFeatures { + LocalFeatures { + flags: vec![1 << 4], + } + } + #[cfg(feature = "fuzztarget")] pub fn new() -> LocalFeatures { LocalFeatures { - flags: Vec::new(), + flags: vec![1 << 4], } } @@ -87,8 +94,8 @@ impl LocalFeatures { pub(crate) fn supports_upfront_shutdown_script(&self) -> bool { self.flags.len() > 0 && (self.flags[0] & (3 << 4)) != 0 } - pub(crate) fn requires_upfront_shutdown_script(&self) -> bool { - self.flags.len() > 0 && (self.flags[0] & (1 << 4)) != 0 + pub(crate) fn unset_upfront_shutdown_script(&mut self) { + self.flags[0] ^= 1 << 4; } pub(crate) fn requires_unknown_bits(&self) -> bool { @@ -2011,9 +2018,9 @@ mod tests { target_value.append(&mut hex::decode("0000").unwrap()); } if initial_routing_sync { - target_value.append(&mut hex::decode("000108").unwrap()); + target_value.append(&mut hex::decode("000118").unwrap()); } else { - target_value.append(&mut hex::decode("0000").unwrap()); + target_value.append(&mut hex::decode("000110").unwrap()); } assert_eq!(encoded_value, target_value); } diff --git a/src/ln/peer_handler.rs b/src/ln/peer_handler.rs index a9784327..f544ae23 100644 --- a/src/ln/peer_handler.rs +++ b/src/ln/peer_handler.rs @@ -587,10 +587,6 @@ impl PeerManager { log_info!(self, "Peer local features required data_loss_protect"); return Err(PeerHandleError{ no_connection_possible: true }); } - if msg.local_features.requires_upfront_shutdown_script() { - log_info!(self, "Peer local features required upfront_shutdown_script"); - return Err(PeerHandleError{ no_connection_possible: true }); - } if peer.their_global_features.is_some() { return Err(PeerHandleError{ no_connection_possible: false }); } -- 2.30.2