-
Notifications
You must be signed in to change notification settings - Fork 133
Refactor liquidity source to support multiple LSP nodes #792
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
base: main
Are you sure you want to change the base?
Changes from all commits
9cfb7de
36390fe
95c88e5
7bb1191
10fcd49
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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>, | ||
| } | ||
|
|
@@ -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( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, the previous API (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We now support multiple LSPs, so
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| &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); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems we only set this here, but not for the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 If we're moving to an explicit |
||
|
|
||
| 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 | ||
| } | ||
|
|
||
|
|
@@ -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 | ||
|
|
@@ -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( | ||
|
|
@@ -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), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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(), | ||
|
|
@@ -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 { | ||
|
|
@@ -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 } => { | ||
|
|
@@ -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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Super confused here: why are we even updating the list in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| .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( | ||
|
|
@@ -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; | ||
| } | ||
|
|
@@ -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; | ||
| } | ||
|
|
@@ -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, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.