Merge pull request #350 from tnull/2024-08-channel-config-refactor

Refactor `ChannelConfig` / `MaxDustHTLCExposure`

Author: tnull

bindings/ldk_node.udl

@@ -409,20 +409,19 @@ dictionary BalanceDetails {
 	sequence<PendingSweepBalance> pending_balances_from_channel_closures;
 };
 
-interface ChannelConfig {
-	constructor();
-	u32 forwarding_fee_proportional_millionths();
-	void set_forwarding_fee_proportional_millionths(u32 value);
-	u32 forwarding_fee_base_msat();
-	void set_forwarding_fee_base_msat(u32 fee_msat);
-	u16 cltv_expiry_delta();
-	void set_cltv_expiry_delta(u16 value);
-	u64 force_close_avoidance_max_fee_satoshis();
-	void set_force_close_avoidance_max_fee_satoshis(u64 value_sat);
-	boolean accept_underpaying_htlcs();
-	void set_accept_underpaying_htlcs(boolean value);
-	void set_max_dust_htlc_exposure_from_fixed_limit(u64 limit_msat);
-	void set_max_dust_htlc_exposure_from_fee_rate_multiplier(u64 multiplier);
+dictionary ChannelConfig {
+	u32 forwarding_fee_proportional_millionths;
+	u32 forwarding_fee_base_msat;
+	u16 cltv_expiry_delta;
+	MaxDustHTLCExposure max_dust_htlc_exposure;
+	u64 force_close_avoidance_max_fee_satoshis;
+	boolean accept_underpaying_htlcs;
+};
+
+[Enum]
+interface MaxDustHTLCExposure {
+	FixedLimit ( u64 limit_msat );
+	FeeRateMultiplier ( u64 multiplier );
 };
 
 enum LogLevel {

src/lib.rs

@@ -109,7 +109,7 @@ pub use error::Error as NodeError;
 use error::Error;
 
 pub use event::Event;
-pub use types::ChannelConfig;
+pub use types::{ChannelConfig, MaxDustHTLCExposure};
 
 pub use io::utils::generate_entropy_mnemonic;
 
@@ -1187,7 +1187,7 @@ impl Node {
 	/// Returns a [`UserChannelId`] allowing to locally keep track of the channel.
 	pub fn connect_open_channel(
 		&self, node_id: PublicKey, address: SocketAddress, channel_amount_sats: u64,
-		push_to_counterparty_msat: Option<u64>, channel_config: Option<Arc<ChannelConfig>>,
+		push_to_counterparty_msat: Option<u64>, channel_config: Option<ChannelConfig>,
 		announce_channel: bool,
 	) -> Result<UserChannelId, Error> {
 		let rt_lock = self.runtime.read().unwrap();
@@ -1251,7 +1251,7 @@ impl Node {
 
 		let mut user_config = default_user_config(&self.config);
 		user_config.channel_handshake_config.announced_channel = announce_channel;
-		user_config.channel_config = (*(channel_config.unwrap_or_default())).clone().into();
+		user_config.channel_config = (channel_config.unwrap_or_default()).clone().into();
 		// We set the max inflight to 100% for private channels.
 		// FIXME: LDK will default to this behavior soon, too, at which point we should drop this
 		// manual override.
@@ -1494,7 +1494,7 @@ impl Node {
 	/// Update the config for a previously opened channel.
 	pub fn update_channel_config(
 		&self, user_channel_id: &UserChannelId, counterparty_node_id: PublicKey,
-		channel_config: Arc<ChannelConfig>,
+		channel_config: ChannelConfig,
 	) -> Result<(), Error> {
 		let open_channels =
 			self.channel_manager.list_channels_with_counterparty(&counterparty_node_id);
@@ -1505,7 +1505,7 @@ impl Node {
 				.update_channel_config(
 					&counterparty_node_id,
 					&[channel_details.channel_id],
-					&(*channel_config).clone().into(),
+					&(channel_config).clone().into(),
 				)
 				.map_err(|_| Error::ChannelConfigUpdateFailed)
 		} else {

src/types.rs

@@ -22,7 +22,7 @@ use lightning_transaction_sync::EsploraSyncClient;
 use bitcoin::secp256k1::PublicKey;
 use bitcoin::OutPoint;
 
-use std::sync::{Arc, Mutex, RwLock};
+use std::sync::{Arc, Mutex};
 
 pub(crate) type DynStore = dyn KVStore + Sync + Send;
 
@@ -279,7 +279,7 @@ pub struct ChannelDetails {
 	/// The largest value HTLC (in msat) we currently will accept, for this channel.
 	pub inbound_htlc_maximum_msat: Option<u64>,
 	/// Set of configurable parameters that affect channel operation.
-	pub config: Arc<ChannelConfig>,
+	pub config: ChannelConfig,
 }
 
 impl From<LdkChannelDetails> for ChannelDetails {
@@ -330,7 +330,7 @@ impl From<LdkChannelDetails> for ChannelDetails {
 			inbound_htlc_minimum_msat: value.inbound_htlc_minimum_msat.unwrap_or(0),
 			inbound_htlc_maximum_msat: value.inbound_htlc_maximum_msat,
 			// unwrap safety: `config` is only `None` for LDK objects serialized prior to 0.0.109.
-			config: value.config.map(|c| Arc::new(c.into())).unwrap(),
+			config: value.config.map(|c| c.into()).unwrap(),
 		}
 	}
 }
@@ -350,98 +350,70 @@ pub struct PeerDetails {
 	pub is_connected: bool,
 }
 
-/// Options which apply on a per-channel basis.
-///
-/// See documentation of [`LdkChannelConfig`] for details.
-#[derive(Debug)]
+/// Options which apply on a per-channel basis and may change at runtime or based on negotiation
+/// with our counterparty.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct ChannelConfig {
-	inner: RwLock<LdkChannelConfig>,
-}
-
-impl Clone for ChannelConfig {
-	fn clone(&self) -> Self {
-		self.inner.read().unwrap().clone().into()
-	}
-}
-
-impl ChannelConfig {
-	/// Constructs a new `ChannelConfig`.
-	pub fn new() -> Self {
-		Self::default()
-	}
-
-	/// Returns the set `forwarding_fee_proportional_millionths`.
-	pub fn forwarding_fee_proportional_millionths(&self) -> u32 {
-		self.inner.read().unwrap().forwarding_fee_proportional_millionths
-	}
-
-	/// Sets the `forwarding_fee_proportional_millionths`.
-	pub fn set_forwarding_fee_proportional_millionths(&self, value: u32) {
-		self.inner.write().unwrap().forwarding_fee_proportional_millionths = value;
-	}
-
-	/// Returns the set `forwarding_fee_base_msat`.
-	pub fn forwarding_fee_base_msat(&self) -> u32 {
-		self.inner.read().unwrap().forwarding_fee_base_msat
-	}
-
-	/// Sets the `forwarding_fee_base_msat`.
-	pub fn set_forwarding_fee_base_msat(&self, fee_msat: u32) {
-		self.inner.write().unwrap().forwarding_fee_base_msat = fee_msat;
-	}
-
-	/// Returns the set `cltv_expiry_delta`.
-	pub fn cltv_expiry_delta(&self) -> u16 {
-		self.inner.read().unwrap().cltv_expiry_delta
-	}
-
-	/// Sets the `cltv_expiry_delta`.
-	pub fn set_cltv_expiry_delta(&self, value: u16) {
-		self.inner.write().unwrap().cltv_expiry_delta = value;
-	}
-
-	/// Returns the set `force_close_avoidance_max_fee_satoshis`.
-	pub fn force_close_avoidance_max_fee_satoshis(&self) -> u64 {
-		self.inner.read().unwrap().force_close_avoidance_max_fee_satoshis
-	}
-
-	/// Sets the `force_close_avoidance_max_fee_satoshis`.
-	pub fn set_force_close_avoidance_max_fee_satoshis(&self, value_sat: u64) {
-		self.inner.write().unwrap().force_close_avoidance_max_fee_satoshis = value_sat;
-	}
-
-	/// Returns the set `accept_underpaying_htlcs`.
-	pub fn accept_underpaying_htlcs(&self) -> bool {
-		self.inner.read().unwrap().accept_underpaying_htlcs
-	}
-
-	/// Sets the `accept_underpaying_htlcs`.
-	pub fn set_accept_underpaying_htlcs(&self, value: bool) {
-		self.inner.write().unwrap().accept_underpaying_htlcs = value;
-	}
-
-	/// Sets the `max_dust_htlc_exposure` from a fixed limit.
-	pub fn set_max_dust_htlc_exposure_from_fixed_limit(&self, limit_msat: u64) {
-		self.inner.write().unwrap().max_dust_htlc_exposure =
-			LdkMaxDustHTLCExposure::FixedLimitMsat(limit_msat);
-	}
-
-	/// Sets the `max_dust_htlc_exposure` from a fee rate multiplier.
-	pub fn set_max_dust_htlc_exposure_from_fee_rate_multiplier(&self, multiplier: u64) {
-		self.inner.write().unwrap().max_dust_htlc_exposure =
-			LdkMaxDustHTLCExposure::FeeRateMultiplier(multiplier);
-	}
+	/// Amount (in millionths of a satoshi) charged per satoshi for payments forwarded outbound
+	/// over the channel.
+	/// This may be allowed to change at runtime in a later update, however doing so must result in
+	/// update messages sent to notify all nodes of our updated relay fee.
+	///
+	/// Please refer to [`LdkChannelConfig`] for further details.
+	pub forwarding_fee_proportional_millionths: u32,
+	/// Amount (in milli-satoshi) charged for payments forwarded outbound over the channel, in
+	/// excess of [`ChannelConfig::forwarding_fee_proportional_millionths`].
+	/// This may be allowed to change at runtime in a later update, however doing so must result in
+	/// update messages sent to notify all nodes of our updated relay fee.
+	///
+	/// Please refer to [`LdkChannelConfig`] for further details.
+	pub forwarding_fee_base_msat: u32,
+	/// The difference in the CLTV value between incoming HTLCs and an outbound HTLC forwarded over
+	/// the channel this config applies to.
+	///
+	/// Please refer to [`LdkChannelConfig`] for further details.
+	pub cltv_expiry_delta: u16,
+	/// Limit our total exposure to potential loss to on-chain fees on close, including in-flight
+	/// HTLCs which are burned to fees as they are too small to claim on-chain and fees on
+	/// commitment transaction(s) broadcasted by our counterparty in excess of our own fee estimate.
+	///
+	/// Please refer to [`LdkChannelConfig`] for further details.
+	pub max_dust_htlc_exposure: MaxDustHTLCExposure,
+	/// The additional fee we're willing to pay to avoid waiting for the counterparty's
+	/// `to_self_delay` to reclaim funds.
+	///
+	/// Please refer to [`LdkChannelConfig`] for further details.
+	pub force_close_avoidance_max_fee_satoshis: u64,
+	/// If set, allows this channel's counterparty to skim an additional fee off this node's inbound
+	/// HTLCs. Useful for liquidity providers to offload on-chain channel costs to end users.
+	///
+	/// Please refer to [`LdkChannelConfig`] for further details.
+	pub accept_underpaying_htlcs: bool,
 }
 
 impl From<LdkChannelConfig> for ChannelConfig {
 	fn from(value: LdkChannelConfig) -> Self {
-		Self { inner: RwLock::new(value) }
+		Self {
+			forwarding_fee_proportional_millionths: value.forwarding_fee_proportional_millionths,
+			forwarding_fee_base_msat: value.forwarding_fee_base_msat,
+			cltv_expiry_delta: value.cltv_expiry_delta,
+			max_dust_htlc_exposure: value.max_dust_htlc_exposure.into(),
+			force_close_avoidance_max_fee_satoshis: value.force_close_avoidance_max_fee_satoshis,
+			accept_underpaying_htlcs: value.accept_underpaying_htlcs,
+		}
 	}
 }
 
 impl From<ChannelConfig> for LdkChannelConfig {
 	fn from(value: ChannelConfig) -> Self {
-		*value.inner.read().unwrap()
+		Self {
+			forwarding_fee_proportional_millionths: value.forwarding_fee_proportional_millionths,
+			forwarding_fee_base_msat: value.forwarding_fee_base_msat,
+			cltv_expiry_delta: value.cltv_expiry_delta,
+			max_dust_htlc_exposure: value.max_dust_htlc_exposure.into(),
+			force_close_avoidance_max_fee_satoshis: value.force_close_avoidance_max_fee_satoshis,
+			accept_underpaying_htlcs: value.accept_underpaying_htlcs,
+		}
 	}
 }
 
@@ -450,3 +422,46 @@ impl Default for ChannelConfig {
 		LdkChannelConfig::default().into()
 	}
 }
+
+/// Options for how to set the max dust exposure allowed on a channel.
+///
+/// See [`LdkChannelConfig::max_dust_htlc_exposure`] for details.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum MaxDustHTLCExposure {
+	/// This sets a fixed limit on the total dust exposure in millisatoshis.
+	///
+	/// Please refer to [`LdkMaxDustHTLCExposure`] for further details.
+	FixedLimit {
+		/// The fixed limit, in millisatoshis.
+		limit_msat: u64,
+	},
+	/// This sets a multiplier on the feerate to determine the maximum allowed dust exposure.
+	///
+	/// Please refer to [`LdkMaxDustHTLCExposure`] for further details.
+	FeeRateMultiplier {
+		/// The applied fee rate multiplier.
+		multiplier: u64,
+	},
+}
+
+impl From<LdkMaxDustHTLCExposure> for MaxDustHTLCExposure {
+	fn from(value: LdkMaxDustHTLCExposure) -> Self {
+		match value {
+			LdkMaxDustHTLCExposure::FixedLimitMsat(limit_msat) => Self::FixedLimit { limit_msat },
+			LdkMaxDustHTLCExposure::FeeRateMultiplier(multiplier) => {
+				Self::FeeRateMultiplier { multiplier }
+			},
+		}
+	}
+}
+
+impl From<MaxDustHTLCExposure> for LdkMaxDustHTLCExposure {
+	fn from(value: MaxDustHTLCExposure) -> Self {
+		match value {
+			MaxDustHTLCExposure::FixedLimit { limit_msat } => Self::FixedLimitMsat(limit_msat),
+			MaxDustHTLCExposure::FeeRateMultiplier { multiplier } => {
+				Self::FeeRateMultiplier(multiplier)
+			},
+		}
+	}
+}