From 09d68ee38b2ecdc5b8bc960cb35204c8e4a9b614 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Sun, 25 Aug 2024 12:01:44 +0300 Subject: [PATCH] Refactor `ChannelConfig` / `MaxDustHTLCExposure` Previously, we chose to expose `ChannelConfig` as a Uniffi `interface`, providing accessor methods. Unfortunately this forced us to `Arc` it everywhere in the API, and also didn't allow to retrieve the currently set dust exposure limits. Here, we refactor our version of `ChannelConfig` to be a normal `struct` (Uniffi `dictionary`), and only expose the `MaxDustHTLCExposure` as an enum-`interface`. --- bindings/ldk_node.udl | 27 +++--- src/lib.rs | 10 +-- src/types.rs | 185 +++++++++++++++++++++++------------------- 3 files changed, 118 insertions(+), 104 deletions(-) diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 6e248e3ff..c30578d4b 100644 --- a/bindings/ldk_node.udl +++ b/bindings/ldk_node.udl @@ -404,20 +404,19 @@ dictionary BalanceDetails { sequence 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 { diff --git a/src/lib.rs b/src/lib.rs index a1d88ef4c..f777b07d1 100644 --- a/src/lib.rs +++ b/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, channel_config: Option>, + push_to_counterparty_msat: Option, channel_config: Option, announce_channel: bool, ) -> Result { 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, + 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 { diff --git a/src/types.rs b/src/types.rs index 0c2faeb78..abc015ce4 100644 --- a/src/types.rs +++ b/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, /// Set of configurable parameters that affect channel operation. - pub config: Arc, + pub config: ChannelConfig, } impl From for ChannelDetails { @@ -330,7 +330,7 @@ impl From 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, -} - -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 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 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 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 for LdkMaxDustHTLCExposure { + fn from(value: MaxDustHTLCExposure) -> Self { + match value { + MaxDustHTLCExposure::FixedLimit { limit_msat } => Self::FixedLimitMsat(limit_msat), + MaxDustHTLCExposure::FeeRateMultiplier { multiplier } => { + Self::FeeRateMultiplier(multiplier) + }, + } + } +}