From 8fd8966b9a7e798184c89acf7a4a9789c309f735 Mon Sep 17 00:00:00 2001 From: Willem Van Lint Date: Thu, 1 Jun 2023 15:24:42 -0400 Subject: [PATCH] Support atomic partial updates to ChannelConfig --- lightning/src/ln/channelmanager.rs | 100 +++++++++++++++++++++++++++-- lightning/src/util/config.rs | 55 ++++++++++++++++ 2 files changed, 148 insertions(+), 7 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0aa2a8552..fc0cd6d53 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -56,7 +56,7 @@ use crate::ln::outbound_payment; use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment}; use crate::ln::wire::Encode; use crate::sign::{EntropySource, KeysManager, NodeSigner, Recipient, SignerProvider, ChannelSigner, WriteableEcdsaChannelSigner}; -use crate::util::config::{UserConfig, ChannelConfig}; +use crate::util::config::{UserConfig, ChannelConfig, ChannelConfigUpdate}; use crate::util::wakers::{Future, Notifier}; use crate::util::scid_utils::fake_scid; use crate::util::string::UntrustedString; @@ -3228,7 +3228,7 @@ where }) } - /// Atomically updates the [`ChannelConfig`] for the given channels. + /// Atomically applies partial updates to the [`ChannelConfig`] of the given channels. /// /// Once the updates are applied, each eligible channel (advertised with a known short channel /// ID and a change in [`forwarding_fee_proportional_millionths`], [`forwarding_fee_base_msat`], @@ -3250,10 +3250,10 @@ where /// [`ChannelUpdate`]: msgs::ChannelUpdate /// [`ChannelUnavailable`]: APIError::ChannelUnavailable /// [`APIMisuseError`]: APIError::APIMisuseError - pub fn update_channel_config( - &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config: &ChannelConfig, + pub fn update_partial_channel_config( + &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config_update: &ChannelConfigUpdate, ) -> Result<(), APIError> { - if config.cltv_expiry_delta < MIN_CLTV_EXPIRY_DELTA { + if config_update.cltv_expiry_delta.map(|delta| delta < MIN_CLTV_EXPIRY_DELTA).unwrap_or(false) { return Err(APIError::APIMisuseError { err: format!("The chosen CLTV expiry delta is below the minimum of {}", MIN_CLTV_EXPIRY_DELTA), }); @@ -3274,7 +3274,9 @@ where } for channel_id in channel_ids { let channel = peer_state.channel_by_id.get_mut(channel_id).unwrap(); - if !channel.update_config(config) { + let mut config = channel.config(); + config.apply(config_update); + if !channel.update_config(&config) { continue; } if let Ok(msg) = self.get_channel_update_for_broadcast(channel) { @@ -3289,6 +3291,34 @@ where Ok(()) } + /// Atomically updates the [`ChannelConfig`] for the given channels. + /// + /// Once the updates are applied, each eligible channel (advertised with a known short channel + /// ID and a change in [`forwarding_fee_proportional_millionths`], [`forwarding_fee_base_msat`], + /// or [`cltv_expiry_delta`]) has a [`BroadcastChannelUpdate`] event message generated + /// containing the new [`ChannelUpdate`] message which should be broadcast to the network. + /// + /// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect + /// `counterparty_node_id` is provided. + /// + /// Returns [`APIMisuseError`] when a [`cltv_expiry_delta`] update is to be applied with a value + /// below [`MIN_CLTV_EXPIRY_DELTA`]. + /// + /// If an error is returned, none of the updates should be considered applied. + /// + /// [`forwarding_fee_proportional_millionths`]: ChannelConfig::forwarding_fee_proportional_millionths + /// [`forwarding_fee_base_msat`]: ChannelConfig::forwarding_fee_base_msat + /// [`cltv_expiry_delta`]: ChannelConfig::cltv_expiry_delta + /// [`BroadcastChannelUpdate`]: events::MessageSendEvent::BroadcastChannelUpdate + /// [`ChannelUpdate`]: msgs::ChannelUpdate + /// [`ChannelUnavailable`]: APIError::ChannelUnavailable + /// [`APIMisuseError`]: APIError::APIMisuseError + pub fn update_channel_config( + &self, counterparty_node_id: &PublicKey, channel_ids: &[[u8; 32]], config: &ChannelConfig, + ) -> Result<(), APIError> { + return self.update_partial_channel_config(counterparty_node_id, channel_ids, &(*config).into()); + } + /// Attempts to forward an intercepted HTLC over the provided channel id and with the provided /// amount to forward. Should only be called in response to an [`HTLCIntercepted`] event. /// @@ -8578,7 +8608,7 @@ mod tests { use crate::routing::router::{PaymentParameters, RouteParameters, find_route}; use crate::util::errors::APIError; use crate::util::test_utils; - use crate::util::config::ChannelConfig; + use crate::util::config::{ChannelConfig, ChannelConfigUpdate}; use crate::sign::EntropySource; #[test] @@ -9489,6 +9519,62 @@ mod tests { check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed); } + + #[test] + fn test_update_channel_config() { + let chanmon_cfg = create_chanmon_cfgs(2); + let node_cfg = create_node_cfgs(2, &chanmon_cfg); + let mut user_config = test_default_channel_config(); + let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[Some(user_config), Some(user_config)]); + let nodes = create_network(2, &node_cfg, &node_chanmgr); + let _ = create_announced_chan_between_nodes(&nodes, 0, 1); + let channel = &nodes[0].node.list_channels()[0]; + + nodes[0].node.update_channel_config(&channel.counterparty.node_id, &[channel.channel_id], &user_config.channel_config).unwrap(); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 0); + + user_config.channel_config.forwarding_fee_base_msat += 10; + nodes[0].node.update_channel_config(&channel.counterparty.node_id, &[channel.channel_id], &user_config.channel_config).unwrap(); + assert_eq!(nodes[0].node.list_channels()[0].config.unwrap().forwarding_fee_base_msat, user_config.channel_config.forwarding_fee_base_msat); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("expected BroadcastChannelUpdate event"), + } + + nodes[0].node.update_partial_channel_config(&channel.counterparty.node_id, &[channel.channel_id], &ChannelConfigUpdate::default()).unwrap(); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 0); + + let new_cltv_expiry_delta = user_config.channel_config.cltv_expiry_delta + 6; + nodes[0].node.update_partial_channel_config(&channel.counterparty.node_id, &[channel.channel_id], &ChannelConfigUpdate { + cltv_expiry_delta: Some(new_cltv_expiry_delta), + ..Default::default() + }).unwrap(); + assert_eq!(nodes[0].node.list_channels()[0].config.unwrap().cltv_expiry_delta, new_cltv_expiry_delta); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("expected BroadcastChannelUpdate event"), + } + + let new_fee = user_config.channel_config.forwarding_fee_proportional_millionths + 100; + nodes[0].node.update_partial_channel_config(&channel.counterparty.node_id, &[channel.channel_id], &ChannelConfigUpdate { + forwarding_fee_proportional_millionths: Some(new_fee), + ..Default::default() + }).unwrap(); + assert_eq!(nodes[0].node.list_channels()[0].config.unwrap().cltv_expiry_delta, new_cltv_expiry_delta); + assert_eq!(nodes[0].node.list_channels()[0].config.unwrap().forwarding_fee_proportional_millionths, new_fee); + let events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + match &events[0] { + MessageSendEvent::BroadcastChannelUpdate { .. } => {}, + _ => panic!("expected BroadcastChannelUpdate event"), + } + } } #[cfg(ldk_bench)] diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 1e678152c..758306c00 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -399,6 +399,27 @@ pub struct ChannelConfig { pub force_close_avoidance_max_fee_satoshis: u64, } +impl ChannelConfig { + /// Applies the given [`ChannelConfigUpdate`] as a partial update to the [`ChannelConfig`]. + pub fn apply(&mut self, update: &ChannelConfigUpdate) { + if let Some(forwarding_fee_proportional_millionths) = update.forwarding_fee_proportional_millionths { + self.forwarding_fee_proportional_millionths = forwarding_fee_proportional_millionths; + } + if let Some(forwarding_fee_base_msat) = update.forwarding_fee_base_msat { + self.forwarding_fee_base_msat = forwarding_fee_base_msat; + } + if let Some(cltv_expiry_delta) = update.cltv_expiry_delta { + self.cltv_expiry_delta = cltv_expiry_delta; + } + if let Some(max_dust_htlc_exposure_msat) = update.max_dust_htlc_exposure_msat { + self.max_dust_htlc_exposure_msat = max_dust_htlc_exposure_msat; + } + if let Some(force_close_avoidance_max_fee_satoshis) = update.force_close_avoidance_max_fee_satoshis { + self.force_close_avoidance_max_fee_satoshis = force_close_avoidance_max_fee_satoshis; + } + } +} + impl Default for ChannelConfig { /// Provides sane defaults for most configurations (but with zero relay fees!). fn default() -> Self { @@ -423,6 +444,40 @@ impl_writeable_tlv_based!(ChannelConfig, { (10, force_close_avoidance_max_fee_satoshis, required), }); +/// A parallel struct to [`ChannelConfig`] to define partial updates. +#[allow(missing_docs)] +pub struct ChannelConfigUpdate { + pub forwarding_fee_proportional_millionths: Option, + pub forwarding_fee_base_msat: Option, + pub cltv_expiry_delta: Option, + pub max_dust_htlc_exposure_msat: Option, + pub force_close_avoidance_max_fee_satoshis: Option, +} + +impl Default for ChannelConfigUpdate { + fn default() -> ChannelConfigUpdate { + ChannelConfigUpdate { + forwarding_fee_proportional_millionths: None, + forwarding_fee_base_msat: None, + cltv_expiry_delta: None, + max_dust_htlc_exposure_msat: None, + force_close_avoidance_max_fee_satoshis: None, + } + } +} + +impl From for ChannelConfigUpdate { + fn from(config: ChannelConfig) -> ChannelConfigUpdate { + ChannelConfigUpdate { + forwarding_fee_proportional_millionths: Some(config.forwarding_fee_proportional_millionths), + forwarding_fee_base_msat: Some(config.forwarding_fee_base_msat), + cltv_expiry_delta: Some(config.cltv_expiry_delta), + max_dust_htlc_exposure_msat: Some(config.max_dust_htlc_exposure_msat), + force_close_avoidance_max_fee_satoshis: Some(config.force_close_avoidance_max_fee_satoshis), + } + } +} + /// Legacy version of [`ChannelConfig`] that stored the static /// [`ChannelHandshakeConfig::announced_channel`] and /// [`ChannelHandshakeConfig::commit_upfront_shutdown_pubkey`] fields. -- 2.39.5