From 2eb6e1f741be3a794ffbf275b8c5b1665e5c16b8 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 17 Feb 2022 22:13:54 +0000 Subject: [PATCH] Negotiate `scid_alias` for private channels based on a new config Because negotiating `scid_alias` for all of our channels will cause us to create channels which LDK versions prior to 0.0.106 do not understand, we disable `scid_alias` negotiation by default. --- lightning/src/ln/channel.rs | 30 ++++++++++++++++++++++++++---- lightning/src/ln/channelmanager.rs | 17 +++++++++++++++++ lightning/src/util/config.rs | 23 +++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0bba70e5..0792164e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -810,6 +810,31 @@ impl Channel { self.channel_transaction_parameters.opt_anchors.is_some() } + fn get_initial_channel_type(config: &UserConfig) -> ChannelTypeFeatures { + // The default channel type (ie the first one we try) depends on whether the channel is + // public - if it is, we just go with `only_static_remotekey` as it's the only option + // available. If it's private, we first try `scid_privacy` as it provides better privacy + // with no other changes, and fall back to `only_static_remotekey` + let mut ret = ChannelTypeFeatures::only_static_remote_key(); + if !config.channel_options.announced_channel && config.own_channel_config.negotiate_scid_privacy { + ret.set_scid_privacy_required(); + } + ret + } + + /// If we receive an error message, it may only be a rejection of the channel type we tried, + /// not of our ability to open any channel at all. Thus, on error, we should first call this + /// and see if we get a new `OpenChannel` message, otherwise the channel is failed. + pub(crate) fn maybe_handle_error_without_close(&mut self, chain_hash: BlockHash) -> Result { + if !self.is_outbound() || self.channel_state != ChannelState::OurInitSent as u32 { return Err(()); } + if self.channel_type == ChannelTypeFeatures::only_static_remote_key() { + // We've exhausted our options + return Err(()); + } + self.channel_type = ChannelTypeFeatures::only_static_remote_key(); // We only currently support two types + Ok(self.get_open_channel(chain_hash)) + } + // Constructors: pub fn new_outbound( fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures, @@ -967,10 +992,7 @@ impl Channel { #[cfg(any(test, fuzzing))] historical_inbound_htlc_fulfills: HashSet::new(), - // We currently only actually support one channel type, so don't retry with new types - // on error messages. When we support more we'll need fallback support (assuming we - // want to support old types). - channel_type: ChannelTypeFeatures::only_static_remote_key(), + channel_type: Self::get_initial_channel_type(&config), }) } diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f524cda8..7cc0cd38 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -5972,6 +5972,23 @@ impl } } } else { + { + // First check if we can advance the channel type and try again. + let mut channel_state = self.channel_state.lock().unwrap(); + if let Some(chan) = channel_state.by_id.get_mut(&msg.channel_id) { + if chan.get_counterparty_node_id() != *counterparty_node_id { + return; + } + if let Ok(msg) = chan.maybe_handle_error_without_close(self.genesis_hash) { + channel_state.pending_msg_events.push(events::MessageSendEvent::SendOpenChannel { + node_id: *counterparty_node_id, + msg, + }); + return; + } + } + } + // Untrusted messages from peer, we throw away the error if id points to a non-existent channel let _ = self.force_close_channel_with_peer(&msg.channel_id, Some(counterparty_node_id), Some(&msg.data)); } diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 55d506e7..bd8b40b6 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -47,6 +47,28 @@ pub struct ChannelHandshakeConfig { /// Default value: 1. If the value is less than 1, it is ignored and set to 1, as is required /// by the protocol. pub our_htlc_minimum_msat: u64, + /// If set, we attempt to negotiate the `scid_privacy` (referred to as `scid_alias` in the + /// BOLTs) option for outbound private channels. This provides better privacy by not including + /// our real on-chain channel UTXO in each invoice and requiring that our counterparty only + /// relay HTLCs to us using the channel's SCID alias. + /// + /// If this option is set, channels may be created that will not be readable by LDK versions + /// prior to 0.0.106, causing [`ChannelManager`]'s read method to return a + /// [`DecodeError:InvalidValue`]. + /// + /// Note that setting this to true does *not* prevent us from opening channels with + /// counterparties that do not support the `scid_alias` option; we will simply fall back to a + /// private channel without that option. + /// + /// Ignored if the channel is negotiated to be announced, see + /// [`ChannelConfig::announced_channel`] and + /// [`ChannelHandshakeLimits::force_announced_channel_preference`] for more. + /// + /// Default value: false. This value is likely to change to true in the future. + /// + /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager + /// [`DecodeError:InvalidValue`]: crate::ln::msgs::DecodeError::InvalidValue + pub negotiate_scid_privacy: bool, } impl Default for ChannelHandshakeConfig { @@ -55,6 +77,7 @@ impl Default for ChannelHandshakeConfig { minimum_depth: 6, our_to_self_delay: BREAKDOWN_TIMEOUT, our_htlc_minimum_msat: 1, + negotiate_scid_privacy: false, } } } -- 2.30.2