Skip to content

LSPS2: Fail (or abandon) intercepted HTLCs if LSP channel open fails #3712

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
128 changes: 127 additions & 1 deletion lightning-liquidity/src/lsps2/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ use crate::prelude::{new_hash_map, HashMap};
use crate::sync::{Arc, Mutex, MutexGuard, RwLock};

use lightning::events::HTLCHandlingFailureType;
use lightning::ln::channelmanager::{AChannelManager, InterceptId};
use lightning::ln::channelmanager::{AChannelManager, FailureCode, InterceptId};
use lightning::ln::msgs::{ErrorAction, LightningError};
use lightning::ln::types::ChannelId;
use lightning::util::errors::APIError;
use lightning::util::hash_tables::HashSet;
use lightning::util::logger::Level;

Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't remove this whitespace, it's there to group the imports.

use lightning_types::payment::PaymentHash;
Expand Down Expand Up @@ -985,6 +986,131 @@ where
Ok(())
}

/// Abandons a pending JIT‐open flow for `user_channel_id`, removing all local state.
///
/// This removes the intercept SCID, any outbound channel state, and associated
/// channel‐ID mappings for the specified `user_channel_id`, but only while the JIT
/// channel is still in `PendingInitialPayment` or `PendingChannelOpen`.
///
/// Returns an error if:
/// - there is no channel matching `user_channel_id`, or
/// - the channel has already advanced past `PendingChannelOpen` (e.g. to
/// `PendingPaymentForward` or beyond).
///
/// Note: this does *not* close or roll back any on‐chain channel which may already
/// have been opened. The caller must only invoke this before a channel is actually
/// confirmed (or else provide its own on‐chain cleanup) and is responsible for
/// managing any pending channel open attempts separately.
pub fn channel_open_abandoned(
Copy link
Contributor

Choose a reason for hiding this comment

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

I think to make this a bit safer, it should probably fail if we're already at PendingPaymentForward or later, i.e., we already opened the channel?

Also, what would happen if this is called after the LSPS2ServiceEvent::OpenChannel has been emitted?

Copy link
Author

Choose a reason for hiding this comment

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

Now when abandon() is called, if it's not at PendingInitialPayment or PendingChannelOpen, then it fails.

Also, what would happen if this is called after the LSPS2ServiceEvent::OpenChannel has been emitted?

You mean if the channel is already opened, but our state machine hasn’t caught up yet and still thinks we're at PendingChannelOpen? (Is that possible?). In that case, if we call abandon(), we’d end up forgetting about a channel that was actually opened, potentially leaving funds locked. If that scenario is possible and we can't reliably detect it, then this version of abandon() isn't safe.

Copy link
Contributor

Choose a reason for hiding this comment

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

You mean if the channel is already opened, but our state machine hasn’t caught up yet and still thinks we're at PendingChannelOpen? (Is that possible?).

Yes. I think it would be possible if the user had already initiated a channel open but hasn't gotten the ChannelReady yet? It might be worth documenting that the user is responsible for cleaning up any pending channel opens.

&self, counterparty_node_id: &PublicKey, user_channel_id: u128,
) -> Result<(), APIError> {
let outer_state_lock = self.per_peer_state.read().unwrap();
let inner_state_lock =
outer_state_lock.get(counterparty_node_id).ok_or_else(|| APIError::APIMisuseError {
err: format!("No counterparty state for: {}", counterparty_node_id),
})?;
let mut peer_state = inner_state_lock.lock().unwrap();

let intercept_scid = peer_state
.intercept_scid_by_user_channel_id
.remove(&user_channel_id)
.ok_or_else(|| APIError::APIMisuseError {
err: format!("Could not find a channel with user_channel_id {}", user_channel_id),
})?;

if let Some(jit_channel) =
peer_state.outbound_channels_by_intercept_scid.get(&intercept_scid)
{
if !matches!(
jit_channel.state,
OutboundJITChannelState::PendingInitialPayment { .. }
| OutboundJITChannelState::PendingChannelOpen { .. }
) {
return Err(APIError::APIMisuseError {
err: "Cannot abandon channel open after channel creation or payment forwarding"
.to_string(),
});
}
}

peer_state.outbound_channels_by_intercept_scid.remove(&intercept_scid);

peer_state.intercept_scid_by_channel_id.retain(|_, &mut scid| scid != intercept_scid);

Ok(())
}

/// Used to fail intercepted HTLCs backwards when a channel open attempt ultimately fails.
///
/// This function should be called after receiving an [`LSPS2ServiceEvent::OpenChannel`] event
/// but only if the channel could not be successfully established. It resets the JIT channel
/// state so that the payer may try the payment again.
///
/// [`LSPS2ServiceEvent::OpenChannel`]: crate::lsps2::event::LSPS2ServiceEvent::OpenChannel
pub fn channel_open_failed(
&self, counterparty_node_id: &PublicKey, user_channel_id: u128,
) -> Result<(), APIError> {
let outer_state_lock = self.per_peer_state.read().unwrap();

let inner_state_lock =
outer_state_lock.get(counterparty_node_id).ok_or_else(|| APIError::APIMisuseError {
err: format!("No counterparty state for: {}", counterparty_node_id),
})?;

let mut peer_state = inner_state_lock.lock().unwrap();

let intercept_scid = peer_state
.intercept_scid_by_user_channel_id
.get(&user_channel_id)
.copied()
.ok_or_else(|| APIError::APIMisuseError {
err: format!("Could not find a channel with user_channel_id {}", user_channel_id),
})?;

let jit_channel = peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.ok_or_else(|| APIError::APIMisuseError {
err: format!(
"Failed to map intercept_scid {} for user_channel_id {} to a channel.",
intercept_scid, user_channel_id,
),
})?;

if !matches!(jit_channel.state, OutboundJITChannelState::PendingChannelOpen { .. }) {
return Err(APIError::APIMisuseError {
err: "Channel is not in the PendingChannelOpen state.".to_string(),
});
}

let mut payment_queue = match &jit_channel.state {
OutboundJITChannelState::PendingChannelOpen { payment_queue, .. } => {
payment_queue.clone()
},
_ => {
return Err(APIError::APIMisuseError {
err: "Channel is not in the PendingChannelOpen state.".to_string(),
});
},
};
let payment_hashes: Vec<_> = payment_queue
.clear()
.into_iter()
.map(|htlc| htlc.payment_hash)
.collect::<HashSet<_>>()
.into_iter()
.collect();
for payment_hash in payment_hashes {
self.channel_manager
.get_cm()
.fail_htlc_backwards_with_reason(&payment_hash, FailureCode::TemporaryNodeFailure);
}

jit_channel.state = OutboundJITChannelState::PendingInitialPayment { payment_queue };

Ok(())
}

/// Forward [`Event::ChannelReady`] event parameters into this function.
///
/// Will forward the intercepted HTLC if it matches a channel
Expand Down
55 changes: 54 additions & 1 deletion lightning-liquidity/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use lightning::sign::EntropySource;

use bitcoin::blockdata::constants::{genesis_block, ChainHash};
use bitcoin::blockdata::transaction::Transaction;
use bitcoin::secp256k1::SecretKey;
use bitcoin::Network;
use lightning::chain::channelmonitor::ANTI_REORG_DELAY;
use lightning::chain::{chainmonitor, BestBlock, Confirm};
Expand All @@ -34,6 +35,8 @@ use lightning::util::persist::{
SCORER_PERSISTENCE_SECONDARY_NAMESPACE,
};
use lightning::util::test_utils;
use lightning_liquidity::lsps2::client::{LSPS2ClientConfig, LSPS2ClientHandler};
use lightning_liquidity::lsps2::service::{LSPS2ServiceConfig, LSPS2ServiceHandler};
use lightning_liquidity::{LiquidityClientConfig, LiquidityManager, LiquidityServiceConfig};
use lightning_persister::fs_store::FilesystemStore;

Expand Down Expand Up @@ -487,7 +490,7 @@ pub(crate) fn create_liquidity_node(
}
}

pub(crate) fn create_service_and_client_nodes(
fn create_service_and_client_nodes(
persist_dir: &str, service_config: LiquidityServiceConfig, client_config: LiquidityClientConfig,
) -> (Node, Node) {
let persist_temp_path = env::temp_dir().join(persist_dir);
Expand Down Expand Up @@ -671,3 +674,53 @@ fn advance_chain(node: &mut Node, num_blocks: u32) {
}
}
}

pub(crate) fn setup_test_lsps2() -> (
&'static LSPS2ClientHandler<Arc<KeysManager>>,
&'static LSPS2ServiceHandler<Arc<ChannelManager>>,
bitcoin::secp256k1::PublicKey,
bitcoin::secp256k1::PublicKey,
&'static Node,
&'static Node,
[u8; 32],
) {
let promise_secret = [42; 32];
let signing_key = SecretKey::from_slice(&promise_secret).unwrap();
let lsps2_service_config = LSPS2ServiceConfig { promise_secret };
let service_config = LiquidityServiceConfig {
#[cfg(lsps1_service)]
lsps1_service_config: None,
lsps2_service_config: Some(lsps2_service_config),
advertise_service: true,
};

let lsps2_client_config = LSPS2ClientConfig::default();
let client_config = LiquidityClientConfig {
lsps1_client_config: None,
lsps2_client_config: Some(lsps2_client_config),
};

let (service_node, client_node) =
create_service_and_client_nodes("webhook_registration_flow", service_config, client_config);

// Leak the nodes to extend their lifetime to 'static since this is test code
let service_node = Box::leak(Box::new(service_node));
let client_node = Box::leak(Box::new(client_node));

let client_handler = client_node.liquidity_manager.lsps2_client_handler().unwrap();
let service_handler = service_node.liquidity_manager.lsps2_service_handler().unwrap();

let secp = bitcoin::secp256k1::Secp256k1::new();
let service_node_id = bitcoin::secp256k1::PublicKey::from_secret_key(&secp, &signing_key);
let client_node_id = client_node.channel_manager.get_our_node_id();

(
client_handler,
service_handler,
service_node_id,
client_node_id,
service_node,
client_node,
promise_secret,
)
}
Loading
Loading