Skip to content

Client trusts lsp #572

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Client trusts lsp #572

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9c1b38
Compatibility changes. DO NOT REVIEW
martinsaposnic Jun 9, 2025
53a8174
Support client_trusts_lsp
martinsaposnic Jun 16, 2025
2cc5267
fixup: client_trusts_lsp is now a dynamic flag. also channel_needs_ma…
martinsaposnic Jul 22, 2025
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
63 changes: 47 additions & 16 deletions src/event.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// http://opensource.org/licenses/MIT>, at your option. You may not use this file except in
// accordance with one or both of these licenses.

use crate::payment::PaymentDetails;
use crate::types::{CustomTlvRecord, DynStore, PaymentStore, Sweeper, Wallet};

use crate::{
Expand All @@ -19,9 +20,7 @@ use crate::fee_estimator::ConfirmationTarget;
use crate::liquidity::LiquiditySource;
use crate::logger::Logger;

use crate::payment::store::{
PaymentDetails, PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus,
};
use crate::payment::store::{PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus};

use crate::io::{
EVENT_QUEUE_PERSISTENCE_KEY, EVENT_QUEUE_PERSISTENCE_PRIMARY_NAMESPACE,
Expand All @@ -36,6 +35,9 @@ use lightning::impl_writeable_tlv_based_enum;
use lightning::ln::channelmanager::PaymentId;
use lightning::ln::types::ChannelId;
use lightning::routing::gossip::NodeId;
use lightning::util::config::{
ChannelConfigOverrides, ChannelConfigUpdate, ChannelHandshakeConfigUpdate,
};
use lightning::util::errors::APIError;
use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer};

Expand Down Expand Up @@ -493,7 +495,7 @@ where
counterparty_node_id,
channel_value_satoshis,
output_script,
..
user_channel_id,
} => {
// Construct the raw transaction with the output that is paid the amount of the
// channel.
Expand All @@ -512,12 +514,39 @@ where
locktime,
) {
Ok(final_tx) => {
// Give the funding transaction back to LDK for opening the channel.
match self.channel_manager.funding_transaction_generated(
temporary_channel_id,
counterparty_node_id,
final_tx,
) {
let needs_manual_broadcast = self
.liquidity_source
.as_ref()
.map(|ls| {
ls.as_ref().lsps2_channel_needs_manual_broadcast(
counterparty_node_id,
user_channel_id,
)
})
.unwrap_or(false);

let result = if needs_manual_broadcast {
self.liquidity_source.as_ref().map(|ls| {
ls.lsps2_store_funding_transaction(
user_channel_id,
counterparty_node_id,
final_tx.clone(),
);
});
self.channel_manager.funding_transaction_generated_manual_broadcast(
temporary_channel_id,
counterparty_node_id,
final_tx,
)
} else {
self.channel_manager.funding_transaction_generated(
temporary_channel_id,
counterparty_node_id,
final_tx,
)
};

match result {
Ok(()) => {},
Err(APIError::APIMisuseError { err }) => {
log_error!(self.logger, "Panicking due to APIMisuseError: {}", err);
Expand Down Expand Up @@ -556,16 +585,17 @@ where
},
}
},
LdkEvent::FundingTxBroadcastSafe { .. } => {
debug_assert!(false, "We currently only support safe funding, so this event should never be emitted.");
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);
});
},
LdkEvent::PaymentClaimable {
payment_hash,
purpose,
amount_msat,
receiver_node_id: _,
via_channel_id: _,
via_user_channel_id: _,
via_channel_ids: _,
claim_deadline,
onion_fields,
counterparty_skimmed_fee_msat,
Expand Down Expand Up @@ -683,8 +713,9 @@ where
// the payment has been registered via `_for_hash` variants and needs to be manually claimed via
// user interaction.
match info.kind {
PaymentKind::Bolt11 { preimage, .. } => {
if purpose.preimage().is_none() {
PaymentKind::Bolt11 { preimage, .. }
| PaymentKind::Bolt11Jit { preimage, .. } => {
if preimage.is_none() || purpose.preimage().is_none() {
debug_assert!(
preimage.is_none(),
"We would have registered the preimage if we knew"
Expand Down
80 changes: 78 additions & 2 deletions src/liquidity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ use std::time::Duration;
const LIQUIDITY_REQUEST_TIMEOUT_SECS: u64 = 5;

const LSPS2_GETINFO_REQUEST_EXPIRY: Duration = Duration::from_secs(60 * 60 * 24);
const LSPS2_CLIENT_TRUSTS_LSP_MODE: bool = true;
const LSPS2_CHANNEL_CLTV_EXPIRY_DELTA: u32 = 72;

struct LSPS1Client {
Expand Down Expand Up @@ -134,6 +133,8 @@ pub struct LSPS2ServiceConfig {
pub min_payment_size_msat: u64,
/// The maximum payment size that we will accept when opening a channel.
pub max_payment_size_msat: u64,
/// Use the client trusts lsp model
pub client_trusts_lsp: bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to understand the client_trusts_lsp configuration here. According to bLIP-52, I expect the LSP to dynamically switch from lsp_trusts_client to client_trusts_lsp mode upon detecting an attack, without requiring a restart.
However, this configuration appears to be static and set at node startup. If that's the case, what happens when:

  1. A node initially running in lsp_trusts_client mode detects an attack
  2. The node restarts with client_trusts_lsp: true to switch modes
  3. There's an existing outbound JIT channel where the client expects the LSP to broadcast the funding transaction before sending the preimage to claim.

What are the potential consequences for that in-flight JIT channel during the mode transition?

Copy link
Author

Choose a reason for hiding this comment

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

just pushed a fixup that makes the flag dynamic

There's an existing outbound JIT channel where the client expects the LSP to broadcast the funding transaction before sending the preimage to claim.

I just posted a question about this https://discord.com/channels/915026692102316113/994015949176963183/1397280758196080660

my current interpretation about this is that once lsps2.buy succeeds, that flag is part of the contract for this flow. the LSP can always abort and make you start over, but it cannot change the trust model mid negotiation

we will see what they respond on discord

Copy link
Author

Choose a reason for hiding this comment

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

my current interpretation about this is that once lsps2.buy succeeds, that flag is part of the contract for this flow. the LSP can always abort and make you start over, but it cannot change the trust model mid negotiation

I created a test that shows how this works (test lsps2_in_flight_under_attack_switch)

Copy link
Collaborator

Choose a reason for hiding this comment

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

my current interpretation about this is that once lsps2.buy succeeds, that flag is part of the contract for this flow. the LSP can always abort and make you start over, but it cannot change the trust model mid negotiation

Yes, as also noted on Discord, I agree with that interpretation and even think that for now we can leave the flag to be statically determined at startup.

}

pub(crate) struct LiquiditySourceBuilder<L: Deref>
Expand Down Expand Up @@ -292,6 +293,81 @@ where
self.lsps2_client.as_ref().map(|s| (s.lsp_node_id, s.lsp_address.clone()))
}

pub(crate) fn lsps2_channel_needs_manual_broadcast(
&self, counterparty_node_id: PublicKey, user_channel_id: u128,
) -> bool {
// if we are not in a client_trusts_lsp model, we don't check and just return false
if !self.is_client_trusts_lsp() {
log_debug!(self.logger, "Skipping funding transaction broadcast as client trusts LSP.");
return false;
}

// if we are in a client_trusts_lsp model, then we check if the LSP has an LSPS2 operation in progress
self.lsps2_service.as_ref().map_or(false, |_| {
let lsps2_service_handler = self.liquidity_manager.lsps2_service_handler();
if let Some(handler) = lsps2_service_handler {
handler
.channel_needs_manual_broadcast(user_channel_id, &counterparty_node_id)
.unwrap_or(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure about unwrap_or here. I think we should log the error and be concerned about its cause if channel_needs_manual_broadcast returns an APIError::APIMisuseError? Say we couldn't find a channel mapped to the provided user_channel_id, should we still go on to call channel_manager.funding_transaction_generated for a non-existent channel when handling FundingGenerationReady? I am confused about this.

Copy link
Author

Choose a reason for hiding this comment

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

this is also fixed in commit 2cc5267

thank you for the review!

} else {
log_error!(self.logger, "LSPS2 service handler is not available.");
false
}
})
}

pub(crate) fn lsps2_store_funding_transaction(
&self, user_channel_id: u128, counterparty_node_id: PublicKey, funding_tx: Transaction,
) {
if !self.is_client_trusts_lsp() {
log_debug!(self.logger, "Skipping funding transaction broadcast as client trusts LSP.");
return;
}
self.lsps2_service.as_ref().map(|_| {
let lsps2_service_handler = self.liquidity_manager.lsps2_service_handler();
if let Some(handler) = lsps2_service_handler {
handler
.store_funding_transaction(user_channel_id, &counterparty_node_id, funding_tx)
.unwrap_or_else(|e| {
debug_assert!(false, "Failed to store funding transaction: {:?}", e);
log_error!(self.logger, "Failed to store funding transaction: {:?}", e);
});
} else {
log_error!(self.logger, "LSPS2 service handler is not available.");
}
});
}

pub(crate) fn lsps2_funding_tx_broadcast_safe(
&self, user_channel_id: u128, counterparty_node_id: PublicKey,
) {
if !self.is_client_trusts_lsp() {
log_debug!(self.logger, "Skipping funding transaction broadcast as client trusts LSP.");
return;
}
self.lsps2_service.as_ref().map(|_| {
let lsps2_service_handler = self.liquidity_manager.lsps2_service_handler();
if let Some(handler) = lsps2_service_handler {
handler
.funding_tx_broadcast_safe(user_channel_id, &counterparty_node_id)
.unwrap_or_else(|e| {
debug_assert!(false, "Failed to store funding transaction: {:?}", e);
log_error!(self.logger, "Failed to store funding transaction: {:?}", e);
});
} else {
log_error!(self.logger, "LSPS2 service handler is not available.");
}
});
}

fn is_client_trusts_lsp(&self) -> bool {
if let Some(lsps2_service) = self.lsps2_service.as_ref() {
lsps2_service.service_config.client_trusts_lsp
} else {
false
}
}

pub(crate) async fn handle_next_event(&self) {
match self.liquidity_manager.next_event_async().await {
LiquidityEvent::LSPS1Client(LSPS1ClientEvent::SupportedOptionsReady {
Expand Down Expand Up @@ -580,7 +656,7 @@ where
request_id,
intercept_scid,
LSPS2_CHANNEL_CLTV_EXPIRY_DELTA,
LSPS2_CLIENT_TRUSTS_LSP_MODE,
service_config.client_trusts_lsp,
user_channel_id,
) {
Ok(()) => {},
Expand Down
14 changes: 10 additions & 4 deletions src/payment/bolt11.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ impl Bolt11Payment {
expiry_secs,
max_total_lsp_fee_limit_msat,
None,
true,
)?;
Ok(maybe_wrap(invoice))
}
Expand All @@ -592,6 +593,7 @@ impl Bolt11Payment {
expiry_secs,
max_total_lsp_fee_limit_msat,
None,
false,
)?;
let preimage = preimage.ok_or(Error::InvoiceCreationFailed)?;
Ok((maybe_wrap(invoice), preimage))
Expand Down Expand Up @@ -619,15 +621,16 @@ impl Bolt11Payment {
expiry_secs,
None,
max_proportional_lsp_fee_limit_ppm_msat,
true,
)?;
Ok(maybe_wrap(invoice))
}

fn receive_via_jit_channel_inner(
&self, amount_msat: Option<u64>, description: &LdkBolt11InvoiceDescription,
expiry_secs: u32, max_total_lsp_fee_limit_msat: Option<u64>,
max_proportional_lsp_fee_limit_ppm_msat: Option<u64>,
) -> Result<LdkBolt11Invoice, Error> {
max_proportional_lsp_fee_limit_ppm_msat: Option<u64>, auto_claim: bool,
) -> Result<(LdkBolt11Invoice, Option<PaymentPreimage>), Error> {
let liquidity_source =
self.liquidity_source.as_ref().ok_or(Error::LiquiditySourceUnavailable)?;

Expand Down Expand Up @@ -690,9 +693,12 @@ impl Bolt11Payment {
let id = PaymentId(payment_hash.0);
let preimage =
self.channel_manager.get_payment_preimage(payment_hash, payment_secret.clone()).ok();

let stored_preimage = if auto_claim { preimage } else { None };

let kind = PaymentKind::Bolt11Jit {
hash: payment_hash,
preimage,
preimage: stored_preimage,
secret: Some(payment_secret.clone()),
counterparty_skimmed_fee_msat: None,
lsp_fee_limits,
Expand All @@ -710,7 +716,7 @@ impl Bolt11Payment {
// Persist LSP peer to make sure we reconnect on restart.
self.peer_store.add_peer(peer_info)?;

Ok(invoice)
Ok((invoice, preimage))
}

/// Sends payment probes over all paths of a route that would be used to pay the given invoice.
Expand Down
Loading
Loading