Merge pull request #2330 from wvanlint/partial_config_updates

Support atomic partial updates to ChannelConfig

Author: TheBlueMatt

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.
 	///
@@ -8582,7 +8612,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]
@@ -9501,6 +9531,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)]

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<u32>,
+	pub forwarding_fee_base_msat: Option<u32>,
+	pub cltv_expiry_delta: Option<u16>,
+	pub max_dust_htlc_exposure_msat: Option<u64>,
+	pub force_close_avoidance_max_fee_satoshis: Option<u64>,
+}
+
+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<ChannelConfig> 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.