Skip to content
Open
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
7 changes: 3 additions & 4 deletions bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ interface Builder {
void set_gossip_source_p2p();
void set_gossip_source_rgs(string rgs_server_url);
void set_pathfinding_scores_source(string url);
void set_liquidity_source_lsps1(PublicKey node_id, SocketAddress address, string? token);
void set_liquidity_source_lsps2(PublicKey node_id, SocketAddress address, string? token);
void add_lsp(PublicKey node_id, SocketAddress address, string? token);
void set_storage_dir_path(string storage_dir_path);
void set_filesystem_logger(string? log_file_path, LogLevel? max_log_level);
void set_log_facade_logger();
Expand Down Expand Up @@ -97,7 +96,7 @@ interface Node {
SpontaneousPayment spontaneous_payment();
OnchainPayment onchain_payment();
UnifiedPayment unified_payment();
LSPS1Liquidity lsps1_liquidity();
Liquidity liquidity();
[Throws=NodeError]
void lnurl_auth(string lnurl);
[Throws=NodeError]
Expand Down Expand Up @@ -165,7 +164,7 @@ interface FeeRate {

typedef interface UnifiedPayment;

typedef interface LSPS1Liquidity;
typedef interface Liquidity;

[Error]
enum NodeError {
Expand Down
95 changes: 21 additions & 74 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ use crate::io::{
PENDING_PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE,
PENDING_PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE,
};
use crate::liquidity::{
LSPS1ClientConfig, LSPS2ClientConfig, LSPS2ServiceConfig, LiquiditySourceBuilder,
};
use crate::liquidity::{LSPS2ServiceConfig, LiquiditySourceBuilder, LspConfig};
use crate::lnurl_auth::LnurlAuth;
use crate::logger::{log_error, LdkLogger, LogLevel, LogWriter, Logger};
use crate::message_handler::NodeCustomMessageHandler;
Expand Down Expand Up @@ -120,10 +118,8 @@ struct PathfindingScoresSyncConfig {

#[derive(Debug, Clone, Default)]
struct LiquiditySourceConfig {
// Act as an LSPS1 client connecting to the given service.
lsps1_client: Option<LSPS1ClientConfig>,
// Act as an LSPS2 client connecting to the given service.
lsps2_client: Option<LSPS2ClientConfig>,
// Acts for both LSPS1 and LSPS2 clients connecting to the given service.
lsp_nodes: Vec<LspConfig>,
// Act as an LSPS2 service.
lsps2_service: Option<LSPS2ServiceConfig>,
}
Expand Down Expand Up @@ -435,45 +431,24 @@ impl NodeBuilder {
self
}

/// Configures the [`Node`] instance to source inbound liquidity from the given
/// [bLIP-51 / LSPS1] service.
///
/// Will mark the LSP as trusted for 0-confirmation channels, see [`Config::trusted_peers_0conf`].
///
/// The given `token` will be used by the LSP to authenticate the user.
///
/// [bLIP-51 / LSPS1]: https://github.com/lightning/blips/blob/master/blip-0051.md
pub fn set_liquidity_source_lsps1(
&mut self, node_id: PublicKey, address: SocketAddress, token: Option<String>,
) -> &mut Self {
// Mark the LSP as trusted for 0conf
self.config.trusted_peers_0conf.push(node_id.clone());

let liquidity_source_config =
self.liquidity_source_config.get_or_insert(LiquiditySourceConfig::default());
let lsps1_client_config = LSPS1ClientConfig { node_id, address, token };
liquidity_source_config.lsps1_client = Some(lsps1_client_config);
self
}

/// Configures the [`Node`] instance to source just-in-time inbound liquidity from the given
/// [bLIP-52 / LSPS2] service.
/// Configures the [`Node`] instance to source inbound liquidity from the given LSP, without specifying
/// the exact protocol used (e.g., LSPS1 or LSPS2).
///
/// Will mark the LSP as trusted for 0-confirmation channels, see [`Config::trusted_peers_0conf`].
///
/// The given `token` will be used by the LSP to authenticate the user.
///
/// [bLIP-52 / LSPS2]: https://github.com/lightning/blips/blob/master/blip-0052.md
pub fn set_liquidity_source_lsps2(
/// This method is useful when the user wants to connect to an LSP but does not want to be concerned with
/// the specific protocol used for liquidity provision. The node will automatically detect and use the
/// appropriate protocol supported by the LSP.
pub fn add_lsp(
Comment thread
tnull marked this conversation as resolved.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, the previous API (set_liquidity_source_lsps2) tried to somewhat disambiguate the case where we are the client vs. where we are the provider. We also intentionally had most builder methods start with set_ to reach a more homogeneous API. Breaking the latter is fine, but if we rename this it would be good to find a name that also works in context of set_liquidity_provider_lsps2, as add_lsp and set_liquidity_provider_lsps2 read now like they do very similar things, but they actually don't.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We now support multiple LSPs, so set_ felt misleading, implying a single value, and discovery determines the protocol. How does renaming it to add_liquidity_source sound? That would restore the source/provider contrast with set_liquidity_provider_lsps2.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That sounds good, and maybe since we're already breaking the pattern it would then be clearer to rename the other one to enable_liquidity_service or enable_liquidity_provider or similar and have it take an lsps2_service_config: Option<LSPS2ServiceConfig> argument, given that we'll soon also want to add an LSPS1 provider?

&mut self, node_id: PublicKey, address: SocketAddress, token: Option<String>,
) -> &mut Self {
// Mark the LSP as trusted for 0conf
self.config.trusted_peers_0conf.push(node_id.clone());
self.config.trusted_peers_0conf.push(node_id);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems we only set this here, but not for the Liquidity::add_lsp variant? Maybe they both should now take trust_peer_0conf: bool?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

LSPs added via the builder were always marked as trusted for 0-conf. Instead of mutating the config at runtime, I took that as the intended default and added the || is_lsp_node check in event.rs so runtime-added LSPs get the same treatment, which is why Liquidity::add_lsp doesn't touch trusted_peers_0conf.

If we're moving to an explicit trust_peer_0conf: bool parameter, my plan would be to store it on LspConfig and have the event handler check that alongside trusted_peers_0conf, rather than trying to mutate Config at runtime. The || is_lsp_node auto-trust would go away. What do you think?


let liquidity_source_config =
self.liquidity_source_config.get_or_insert(LiquiditySourceConfig::default());
let lsps2_client_config = LSPS2ClientConfig { node_id, address, token };
liquidity_source_config.lsps2_client = Some(lsps2_client_config);
liquidity_source_config.lsp_nodes.push(LspConfig { node_id, address, token });
self
}

Expand Down Expand Up @@ -956,32 +931,16 @@ impl ArcedNodeBuilder {
self.inner.write().expect("lock").set_pathfinding_scores_source(url);
}

/// Configures the [`Node`] instance to source inbound liquidity from the given
/// [bLIP-51 / LSPS1] service.
/// Configures the [`Node`] instance to source inbound liquidity from the given LSP.
///
/// Will mark the LSP as trusted for 0-confirmation channels, see [`Config::trusted_peers_0conf`].
///
/// The given `token` will be used by the LSP to authenticate the user.
///
/// [bLIP-51 / LSPS1]: https://github.com/lightning/blips/blob/master/blip-0051.md
pub fn set_liquidity_source_lsps1(
&self, node_id: PublicKey, address: SocketAddress, token: Option<String>,
) {
self.inner.write().expect("lock").set_liquidity_source_lsps1(node_id, address, token);
}

/// Configures the [`Node`] instance to source just-in-time inbound liquidity from the given
/// [bLIP-52 / LSPS2] service.
///
/// Will mark the LSP as trusted for 0-confirmation channels, see [`Config::trusted_peers_0conf`].
///
/// The given `token` will be used by the LSP to authenticate the user.
///
/// [bLIP-52 / LSPS2]: https://github.com/lightning/blips/blob/master/blip-0052.md
pub fn set_liquidity_source_lsps2(
&self, node_id: PublicKey, address: SocketAddress, token: Option<String>,
) {
self.inner.write().expect("lock").set_liquidity_source_lsps2(node_id, address, token);
/// This method is useful when the user wants to connect to an LSP but does not want to be concerned with
/// the specific protocol used for liquidity provision. The node will automatically detect and use the
/// appropriate protocol supported by the LSP.
pub fn add_lsp(&self, node_id: PublicKey, address: SocketAddress, token: Option<String>) {
self.inner.write().expect("lock").add_lsp(node_id, address, token);
}

/// Configures the [`Node`] instance to provide an [LSPS2] service, issuing just-in-time
Expand Down Expand Up @@ -1806,21 +1765,7 @@ fn build_with_store_internal(
Arc::clone(&logger),
);

lsc.lsps1_client.as_ref().map(|config| {
liquidity_source_builder.lsps1_client(
config.node_id,
config.address.clone(),
config.token.clone(),
)
});

lsc.lsps2_client.as_ref().map(|config| {
liquidity_source_builder.lsps2_client(
config.node_id,
config.address.clone(),
config.token.clone(),
)
});
liquidity_source_builder.set_lsp_nodes(lsc.lsp_nodes.clone());

let promise_secret = {
let lsps_xpriv = derive_xprv(
Expand Down Expand Up @@ -1889,7 +1834,9 @@ fn build_with_store_internal(
}
}));

liquidity_source.as_ref().map(|l| l.set_peer_manager(Arc::downgrade(&peer_manager)));
liquidity_source
.as_ref()
.map(|l| l.lsps2_service().set_peer_manager(Arc::downgrade(&peer_manager)));

let connection_manager = Arc::new(ConnectionManager::new(
Arc::clone(&peer_manager),
Expand Down
5 changes: 5 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ pub enum Error {
LnurlAuthTimeout,
/// The provided lnurl is invalid.
InvalidLnurl,
/// An LSP with the given node id has already been added.
DuplicateLspNode,
}

impl fmt::Display for Error {
Expand Down Expand Up @@ -222,6 +224,9 @@ impl fmt::Display for Error {
Self::LnurlAuthFailed => write!(f, "LNURL-auth authentication failed."),
Self::LnurlAuthTimeout => write!(f, "LNURL-auth authentication timed out."),
Self::InvalidLnurl => write!(f, "The provided lnurl is invalid."),
Self::DuplicateLspNode => {
write!(f, "An LSP with the given node id has already been added.")
},
}
}
}
Expand Down
60 changes: 33 additions & 27 deletions src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,15 +581,15 @@ where
Ok(final_tx) => {
let needs_manual_broadcast =
self.liquidity_source.as_ref().map_or(false, |ls| {
ls.as_ref().lsps2_channel_needs_manual_broadcast(
ls.as_ref().lsps2_service().lsps2_channel_needs_manual_broadcast(
counterparty_node_id,
user_channel_id,
)
});

let result = if needs_manual_broadcast {
self.liquidity_source.as_ref().map(|ls| {
ls.lsps2_store_funding_transaction(
ls.lsps2_service().lsps2_store_funding_transaction(
user_channel_id,
counterparty_node_id,
final_tx.clone(),
Expand Down Expand Up @@ -653,7 +653,8 @@ where
},
LdkEvent::FundingTxBroadcastSafe { user_channel_id, counterparty_node_id, .. } => {
self.liquidity_source.as_ref().map(|ls| {
ls.lsps2_funding_tx_broadcast_safe(user_channel_id, counterparty_node_id);
ls.lsps2_service()
.lsps2_funding_tx_broadcast_safe(user_channel_id, counterparty_node_id);
});
},
LdkEvent::PaymentClaimable {
Expand Down Expand Up @@ -1162,7 +1163,10 @@ where
LdkEvent::ProbeFailed { .. } => {},
LdkEvent::HTLCHandlingFailed { failure_type, .. } => {
if let Some(liquidity_source) = self.liquidity_source.as_ref() {
liquidity_source.handle_htlc_handling_failed(failure_type).await;
liquidity_source
.lsps2_service()
.handle_htlc_handling_failed(failure_type)
.await;
}
},
LdkEvent::SpendableOutputs { outputs, channel_id, counterparty_node_id } => {
Expand Down Expand Up @@ -1263,31 +1267,30 @@ where
.try_into()
.expect("slice is exactly 16 bytes"),
);
let allow_0conf = self.config.trusted_peers_0conf.contains(&counterparty_node_id);
let mut channel_override_config = None;
if let Some((lsp_node_id, _)) = self
let is_lsp_node = self
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Super confused here: why are we even updating the list in add_lsp if we now decide we trust every LSP for 0conf? Shouldn't we still maintain the list and only trust individual LSPs if the users configures them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is the same inconsistency you flagged on the other comment #792 (comment). I trusted every LSP via || is_lsp_node because at compile time, LSPs added through the builder were already always trusted for 0-conf

.liquidity_source
.as_ref()
.and_then(|ls| ls.as_ref().get_lsps2_lsp_details())
{
if lsp_node_id == counterparty_node_id {
// When we're an LSPS2 client, allow claiming underpaying HTLCs as the LSP will skim off some fee. We'll
// check that they don't take too much before claiming.
//
// We also set maximum allowed inbound HTLC value in flight
// to 100%. We should eventually be able to set this on a per-channel basis, but for
// now we just bump the default for all channels.
channel_override_config = Some(ChannelConfigOverrides {
handshake_overrides: Some(ChannelHandshakeConfigUpdate {
max_inbound_htlc_value_in_flight_percent_of_channel: Some(100),
..Default::default()
}),
update_overrides: Some(ChannelConfigUpdate {
accept_underpaying_htlcs: Some(true),
..Default::default()
}),
});
}
.map_or(false, |ls| ls.as_ref().is_lsps_node(&counterparty_node_id));
let allow_0conf =
self.config.trusted_peers_0conf.contains(&counterparty_node_id) || is_lsp_node;
let mut channel_override_config = None;
if is_lsp_node {
// When we're an LSPS2 client, allow claiming underpaying HTLCs as the LSP will skim off some fee. We'll
// check that they don't take too much before claiming.
//
// We also set maximum allowed inbound HTLC value in flight
// to 100%. We should eventually be able to set this on a per-channel basis, but for
// now we just bump the default for all channels.
channel_override_config = Some(ChannelConfigOverrides {
handshake_overrides: Some(ChannelHandshakeConfigUpdate {
max_inbound_htlc_value_in_flight_percent_of_channel: Some(100),
..Default::default()
}),
update_overrides: Some(ChannelConfigUpdate {
accept_underpaying_htlcs: Some(true),
..Default::default()
}),
});
}
let res = if allow_0conf {
self.channel_manager.accept_inbound_channel_from_trusted_peer(
Expand Down Expand Up @@ -1416,6 +1419,7 @@ where
if let Some(liquidity_source) = self.liquidity_source.as_ref() {
let skimmed_fee_msat = skimmed_fee_msat.unwrap_or(0);
liquidity_source
.lsps2_service()
.handle_payment_forwarded(Some(next_htlc.channel_id), skimmed_fee_msat)
.await;
}
Expand Down Expand Up @@ -1529,6 +1533,7 @@ where

if let Some(liquidity_source) = self.liquidity_source.as_ref() {
liquidity_source
.lsps2_service()
.handle_channel_ready(user_channel_id, &channel_id, &counterparty_node_id)
.await;
}
Expand Down Expand Up @@ -1600,6 +1605,7 @@ where
} => {
if let Some(liquidity_source) = self.liquidity_source.as_ref() {
liquidity_source
.lsps2_service()
.handle_htlc_intercepted(
requested_next_hop_scid,
intercept_id,
Expand Down
Loading
Loading