Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor ChannelConfig / MaxDustHTLCExposure #350

Merged
merged 1 commit into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 13 additions & 14 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -404,20 +404,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 {
Expand Down
10 changes: 5 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down
185 changes: 100 additions & 85 deletions src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
}
}
}
Expand All @@ -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,
}
}
}

Expand All @@ -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,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include the units _sats_per_kw?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, a multiplier is unitless?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now added a fixup dropping the (in sats/KW) from the comment as IMO it's misleading (the feerate has a unit, but it doesn't matter which one, the multiplier is just a multiplier.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, thanks I misread that

},
}

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)
},
}
}
}
Loading