Negotiate `scid_alias` for private channels based on a new config
authorMatt Corallo <git@bluematt.me>
Thu, 17 Feb 2022 22:13:54 +0000 (22:13 +0000)
committerMatt Corallo <git@bluematt.me>
Sun, 27 Mar 2022 17:12:17 +0000 (17:12 +0000)
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
lightning/src/ln/channelmanager.rs
lightning/src/util/config.rs

index 0bba70e5e20ef306a3ba649eb29bf7ad769a3f21..0792164e20d588a043de6700e0c23ca3956eff9d 100644 (file)
@@ -810,6 +810,31 @@ impl<Signer: Sign> Channel<Signer> {
                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<msgs::OpenChannel, ()> {
+               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<K: Deref, F: Deref>(
                fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
@@ -967,10 +992,7 @@ impl<Signer: Sign> Channel<Signer> {
                        #[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),
                })
        }
 
index f524cda84c8ad19d4ecbd924ebc89f775e3e8e1b..7cc0cd380b3b4b4b078559cec5a0d7fad842701f 100644 (file)
@@ -5972,6 +5972,23 @@ impl<Signer: Sign, M: Deref , T: Deref , K: Deref , F: Deref , L: Deref >
                                }
                        }
                } 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));
                }
index 55d506e79f145a386919ecce11f54ea7adbb432f..bd8b40b66567e8a5346e8e551568aaf1d539eae5 100644 (file)
@@ -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,
                }
        }
 }