Merge pull request #351 from tnull/2024-08-336-followup

#336 followups

Author: tnull

bindings/ldk_node.udl

@@ -8,15 +8,14 @@ dictionary Config {
 	string? log_dir_path;
 	Network network;
 	sequence<SocketAddress>? listening_addresses;
-	u32 default_cltv_expiry_delta;
 	u64 onchain_wallet_sync_interval_secs;
 	u64 wallet_sync_interval_secs;
 	u64 fee_rate_cache_update_interval_secs;
 	sequence<PublicKey> trusted_peers_0conf;
 	u64 probing_liquidity_limit_multiplier;
 	LogLevel log_level;
 	AnchorChannelsConfig? anchor_channels_config;
-	SendingParameters? sending_parameters_config;
+	SendingParameters? sending_parameters;
 };
 
 dictionary AnchorChannelsConfig {
@@ -151,10 +150,10 @@ interface OnchainPayment {
 };
 
 interface UnifiedQrPayment {
-    [Throws=NodeError]
-    string receive(u64 amount_sats, [ByRef]string message, u32 expiry_sec);
-    [Throws=NodeError]
-    QrPaymentResult send([ByRef]string uri_str);
+	[Throws=NodeError]
+	string receive(u64 amount_sats, [ByRef]string message, u32 expiry_sec);
+	[Throws=NodeError]
+	QrPaymentResult send([ByRef]string uri_str);
 };
 
 [Error]
@@ -290,9 +289,9 @@ interface PaymentKind {
 
 [Enum]
 interface QrPaymentResult {
-    Onchain(Txid txid);
-    Bolt11(PaymentId payment_id);
-    Bolt12(PaymentId payment_id);
+	Onchain(Txid txid);
+	Bolt11(PaymentId payment_id);
+	Bolt12(PaymentId payment_id);
 };
 
 enum PaymentDirection {
@@ -321,10 +320,16 @@ dictionary PaymentDetails {
 };
 
 dictionary SendingParameters {
-    u64? max_total_routing_fee_msat;
-    u32? max_total_cltv_expiry_delta;
-    u8? max_path_count;
-    u8? max_channel_saturation_power_of_half;
+	MaxTotalRoutingFeeLimit? max_total_routing_fee_msat;
+	u32? max_total_cltv_expiry_delta;
+	u8? max_path_count;
+	u8? max_channel_saturation_power_of_half;
+};
+
+[Enum]
+interface MaxTotalRoutingFeeLimit {
+	None ();
+	Some ( u64 amount_msat );
 };
 
 [NonExhaustive]

bindings/python/src/ldk_node/test_ldk_node.py

@@ -186,7 +186,7 @@ def test_channel_full_cycle(self):
         node_2.event_handled()
 
         invoice = node_2.bolt11_payment().receive(2500000, "asdf", 9217)
-        node_1.bolt11_payment().send(invoice)
+        node_1.bolt11_payment().send(invoice, None)
 
         payment_successful_event_1 = node_1.wait_next_event()
         assert isinstance(payment_successful_event_1, Event.PAYMENT_SUCCESSFUL)

src/config.rs

@@ -1,5 +1,3 @@
-use std::time::Duration;
-
 use crate::payment::SendingParameters;
 
 use lightning::ln::msgs::SocketAddress;
@@ -9,10 +7,11 @@ use lightning::util::logger::Level as LogLevel;
 use bitcoin::secp256k1::PublicKey;
 use bitcoin::Network;
 
+use std::time::Duration;
+
 // Config defaults
 const DEFAULT_STORAGE_DIR_PATH: &str = "/tmp/ldk_node/";
 const DEFAULT_NETWORK: Network = Network::Bitcoin;
-const DEFAULT_CLTV_EXPIRY_DELTA: u32 = 144;
 const DEFAULT_BDK_WALLET_SYNC_INTERVAL_SECS: u64 = 80;
 const DEFAULT_LDK_WALLET_SYNC_INTERVAL_SECS: u64 = 30;
 const DEFAULT_FEE_RATE_CACHE_UPDATE_INTERVAL_SECS: u64 = 60 * 10;
@@ -88,9 +87,10 @@ pub(crate) const WALLET_KEYS_SEED_LEN: usize = 64;
 /// | `probing_liquidity_limit_multiplier`   | 3                  |
 /// | `log_level`                            | Debug              |
 /// | `anchor_channels_config`               | Some(..)           |
-/// | `sending_parameters_config`            | None               |
+/// | `sending_parameters`                   | None               |
 ///
-/// See [`AnchorChannelsConfig`] for more information on its respective default values.
+/// See [`AnchorChannelsConfig`] and [`SendingParameters`] for more information regarding their
+/// respective default values.
 ///
 /// [`Node`]: crate::Node
 pub struct Config {
@@ -104,8 +104,6 @@ pub struct Config {
 	pub network: Network,
 	/// The addresses on which the node will listen for incoming connections.
 	pub listening_addresses: Option<Vec<SocketAddress>>,
-	/// The default CLTV expiry delta to be used for payments.
-	pub default_cltv_expiry_delta: u32,
 	/// The time in-between background sync attempts of the onchain wallet, in seconds.
 	///
 	/// **Note:** A minimum of 10 seconds is always enforced.
@@ -150,12 +148,14 @@ pub struct Config {
 	/// closure. We *will* however still try to get the Anchor spending transactions confirmed
 	/// on-chain with the funds available.
 	pub anchor_channels_config: Option<AnchorChannelsConfig>,
-
 	/// Configuration options for payment routing and pathfinding.
 	///
 	/// Setting the `SendingParameters` provides flexibility to customize how payments are routed,
 	/// including setting limits on routing fees, CLTV expiry, and channel utilization.
-	pub sending_parameters_config: Option<SendingParameters>,
+	///
+	/// **Note:** If unset, default parameters will be used, and you will be able to override the
+	/// parameters on a per-payment basis in the corresponding method calls.
+	pub sending_parameters: Option<SendingParameters>,
 }
 
 impl Default for Config {
@@ -165,15 +165,14 @@ impl Default for Config {
 			log_dir_path: None,
 			network: DEFAULT_NETWORK,
 			listening_addresses: None,
-			default_cltv_expiry_delta: DEFAULT_CLTV_EXPIRY_DELTA,
 			onchain_wallet_sync_interval_secs: DEFAULT_BDK_WALLET_SYNC_INTERVAL_SECS,
 			wallet_sync_interval_secs: DEFAULT_LDK_WALLET_SYNC_INTERVAL_SECS,
 			fee_rate_cache_update_interval_secs: DEFAULT_FEE_RATE_CACHE_UPDATE_INTERVAL_SECS,
 			trusted_peers_0conf: Vec::new(),
 			probing_liquidity_limit_multiplier: DEFAULT_PROBING_LIQUIDITY_LIMIT_MULTIPLIER,
 			log_level: DEFAULT_LOG_LEVEL,
 			anchor_channels_config: Some(AnchorChannelsConfig::default()),
-			sending_parameters_config: None,
+			sending_parameters: None,
 		}
 	}
 }

src/payment/bolt11.rs

@@ -71,10 +71,8 @@ impl Bolt11Payment {
 
 	/// Send a payment given an invoice.
 	///
-	/// If [`SendingParameters`] are provided they will override the node's default routing parameters
-	/// on a per-field basis. Each field in `SendingParameters` that is set replaces the corresponding
-	/// default value. Fields that are not set fall back to the node's configured defaults. If no
-	/// `SendingParameters` are provided, the method fully relies on these defaults.
+	/// If `sending_parameters` are provided they will override the default as well as the
+	/// node-wide parameters configured via [`Config::sending_parameters`] on a per-field basis.
 	pub fn send(
 		&self, invoice: &Bolt11Invoice, sending_parameters: Option<SendingParameters>,
 	) -> Result<PaymentId, Error> {
@@ -98,39 +96,20 @@ impl Bolt11Payment {
 			}
 		}
 
-		if let Some(user_set_params) = sending_parameters {
-			if let Some(mut default_params) =
-				self.config.sending_parameters_config.as_ref().cloned()
-			{
-				default_params.max_total_routing_fee_msat = user_set_params
-					.max_total_routing_fee_msat
-					.or(default_params.max_total_routing_fee_msat);
-				default_params.max_total_cltv_expiry_delta = user_set_params
-					.max_total_cltv_expiry_delta
-					.or(default_params.max_total_cltv_expiry_delta);
-				default_params.max_path_count =
-					user_set_params.max_path_count.or(default_params.max_path_count);
-				default_params.max_channel_saturation_power_of_half = user_set_params
-					.max_channel_saturation_power_of_half
-					.or(default_params.max_channel_saturation_power_of_half);
-
-				route_params.max_total_routing_fee_msat = default_params.max_total_routing_fee_msat;
-				route_params.payment_params.max_total_cltv_expiry_delta =
-					default_params.max_total_cltv_expiry_delta.unwrap_or_default();
-				route_params.payment_params.max_path_count =
-					default_params.max_path_count.unwrap_or_default();
-				route_params.payment_params.max_channel_saturation_power_of_half =
-					default_params.max_channel_saturation_power_of_half.unwrap_or_default();
-			}
-		} else if let Some(default_params) = &self.config.sending_parameters_config {
-			route_params.max_total_routing_fee_msat = default_params.max_total_routing_fee_msat;
-			route_params.payment_params.max_total_cltv_expiry_delta =
-				default_params.max_total_cltv_expiry_delta.unwrap_or_default();
-			route_params.payment_params.max_path_count =
-				default_params.max_path_count.unwrap_or_default();
-			route_params.payment_params.max_channel_saturation_power_of_half =
-				default_params.max_channel_saturation_power_of_half.unwrap_or_default();
-		}
+		let override_params =
+			sending_parameters.as_ref().or(self.config.sending_parameters.as_ref());
+		if let Some(override_params) = override_params {
+			override_params
+				.max_total_routing_fee_msat
+				.map(|f| route_params.max_total_routing_fee_msat = f.into());
+			override_params
+				.max_total_cltv_expiry_delta
+				.map(|d| route_params.payment_params.max_total_cltv_expiry_delta = d);
+			override_params.max_path_count.map(|p| route_params.payment_params.max_path_count = p);
+			override_params
+				.max_channel_saturation_power_of_half
+				.map(|s| route_params.payment_params.max_channel_saturation_power_of_half = s);
+		};
 
 		let payment_secret = Some(*invoice.payment_secret());
 		let retry_strategy = Retry::Timeout(LDK_PAYMENT_RETRY_TIMEOUT);
@@ -197,10 +176,8 @@ impl Bolt11Payment {
 	/// This can be used to pay a so-called "zero-amount" invoice, i.e., an invoice that leaves the
 	/// amount paid to be determined by the user.
 	///
-	/// If [`SendingParameters`] are provided they will override the node's default routing parameters
-	/// on a per-field basis. Each field in `SendingParameters` that is set replaces the corresponding
-	/// default value. Fields that are not set fall back to the node's configured defaults. If no
-	/// `SendingParameters` are provided, the method fully relies on these defaults.
+	/// If `sending_parameters` are provided they will override the default as well as the
+	/// node-wide parameters configured via [`Config::sending_parameters`] on a per-field basis.
 	pub fn send_using_amount(
 		&self, invoice: &Bolt11Invoice, amount_msat: u64,
 		sending_parameters: Option<SendingParameters>,
@@ -247,39 +224,20 @@ impl Bolt11Payment {
 		let mut route_params =
 			RouteParameters::from_payment_params_and_value(payment_params, amount_msat);
 
-		if let Some(user_set_params) = sending_parameters {
-			if let Some(mut default_params) =
-				self.config.sending_parameters_config.as_ref().cloned()
-			{
-				default_params.max_total_routing_fee_msat = user_set_params
-					.max_total_routing_fee_msat
-					.or(default_params.max_total_routing_fee_msat);
-				default_params.max_total_cltv_expiry_delta = user_set_params
-					.max_total_cltv_expiry_delta
-					.or(default_params.max_total_cltv_expiry_delta);
-				default_params.max_path_count =
-					user_set_params.max_path_count.or(default_params.max_path_count);
-				default_params.max_channel_saturation_power_of_half = user_set_params
-					.max_channel_saturation_power_of_half
-					.or(default_params.max_channel_saturation_power_of_half);
-
-				route_params.max_total_routing_fee_msat = default_params.max_total_routing_fee_msat;
-				route_params.payment_params.max_total_cltv_expiry_delta =
-					default_params.max_total_cltv_expiry_delta.unwrap_or_default();
-				route_params.payment_params.max_path_count =
-					default_params.max_path_count.unwrap_or_default();
-				route_params.payment_params.max_channel_saturation_power_of_half =
-					default_params.max_channel_saturation_power_of_half.unwrap_or_default();
-			}
-		} else if let Some(default_params) = &self.config.sending_parameters_config {
-			route_params.max_total_routing_fee_msat = default_params.max_total_routing_fee_msat;
-			route_params.payment_params.max_total_cltv_expiry_delta =
-				default_params.max_total_cltv_expiry_delta.unwrap_or_default();
-			route_params.payment_params.max_path_count =
-				default_params.max_path_count.unwrap_or_default();
-			route_params.payment_params.max_channel_saturation_power_of_half =
-				default_params.max_channel_saturation_power_of_half.unwrap_or_default();
-		}
+		let override_params =
+			sending_parameters.as_ref().or(self.config.sending_parameters.as_ref());
+		if let Some(override_params) = override_params {
+			override_params
+				.max_total_routing_fee_msat
+				.map(|f| route_params.max_total_routing_fee_msat = f.into());
+			override_params
+				.max_total_cltv_expiry_delta
+				.map(|d| route_params.payment_params.max_total_cltv_expiry_delta = d);
+			override_params.max_path_count.map(|p| route_params.payment_params.max_path_count = p);
+			override_params
+				.max_channel_saturation_power_of_half
+				.map(|s| route_params.payment_params.max_channel_saturation_power_of_half = s);
+		};
 
 		let retry_strategy = Retry::Timeout(LDK_PAYMENT_RETRY_TIMEOUT);
 		let recipient_fields = RecipientOnionFields::secret_only(*payment_secret);

src/payment/mod.rs

@@ -14,7 +14,7 @@ pub use spontaneous::SpontaneousPayment;
 pub use store::{LSPFeeLimits, PaymentDetails, PaymentDirection, PaymentKind, PaymentStatus};
 pub use unified_qr::{QrPaymentResult, UnifiedQrPayment};
 
-/// Represents information used to route a payment.
+/// Represents information used to send a payment.
 #[derive(Clone, Debug, PartialEq)]
 pub struct SendingParameters {
 	/// The maximum total fees, in millisatoshi, that may accrue during route finding.
@@ -23,22 +23,28 @@ pub struct SendingParameters {
 	/// paths.
 	///
 	/// Note that values below a few sats may result in some paths being spuriously ignored.
-	pub max_total_routing_fee_msat: Option<u64>,
-
+	#[cfg(not(feature = "uniffi"))]
+	pub max_total_routing_fee_msat: Option<Option<u64>>,
+	/// The maximum total fees, in millisatoshi, that may accrue during route finding.
+	///
+	/// This limit also applies to the total fees that may arise while retrying failed payment
+	/// paths.
+	///
+	/// Note that values below a few sats may result in some paths being spuriously ignored.
+	#[cfg(feature = "uniffi")]
+	pub max_total_routing_fee_msat: Option<MaxTotalRoutingFeeLimit>,
 	/// The maximum total CLTV delta we accept for the route.
 	///
 	/// Defaults to [`DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA`].
 	///
 	/// [`DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA`]: lightning::routing::router::DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA
 	pub max_total_cltv_expiry_delta: Option<u32>,
-
 	/// The maximum number of paths that may be used by (MPP) payments.
 	///
 	/// Defaults to [`DEFAULT_MAX_PATH_COUNT`].
 	///
 	/// [`DEFAULT_MAX_PATH_COUNT`]: lightning::routing::router::DEFAULT_MAX_PATH_COUNT
 	pub max_path_count: Option<u8>,
-
 	/// Selects the maximum share of a channel's total capacity which will be sent over a channel,
 	/// as a power of 1/2.
 	///
@@ -62,3 +68,32 @@ pub struct SendingParameters {
 	/// Default value: 2
 	pub max_channel_saturation_power_of_half: Option<u8>,
 }
+
+/// Represents the possible states of [`SendingParameters::max_total_routing_fee_msat`].
+//
+// Required only in bindings as UniFFI can't expose `Option<Option<..>>`.
+#[cfg(feature = "uniffi")]
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub enum MaxTotalRoutingFeeLimit {
+	None,
+	Some { amount_msat: u64 },
+}
+
+#[cfg(feature = "uniffi")]
+impl From<MaxTotalRoutingFeeLimit> for Option<u64> {
+	fn from(value: MaxTotalRoutingFeeLimit) -> Self {
+		match value {
+			MaxTotalRoutingFeeLimit::Some { amount_msat } => Some(amount_msat),
+			MaxTotalRoutingFeeLimit::None => None,
+		}
+	}
+}
+
+#[cfg(feature = "uniffi")]
+impl From<Option<u64>> for MaxTotalRoutingFeeLimit {
+	fn from(value: Option<u64>) -> Self {
+		value.map_or(MaxTotalRoutingFeeLimit::None, |amount_msat| MaxTotalRoutingFeeLimit::Some {
+			amount_msat,
+		})
+	}
+}