Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
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
33 changes: 25 additions & 8 deletions lightning-liquidity/src/lsps2/payment_queue.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -26,21 +26,29 @@ impl PaymentQueue {
PaymentQueue { payments: Vec::new() }
}

fn payment_status(entry: &PaymentQueueEntry) -> (u64, usize) {
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
}

pub(crate) fn add_htlc(&mut self, new_htlc: InterceptedHTLC) -> (u64, usize) {
if let Some(entry) = self
.payments
.iter()
.find(|entry| entry.htlcs.iter().any(|htlc| htlc.intercept_id == new_htlc.intercept_id))
{
debug_assert_eq!(entry.payment_hash, new_htlc.payment_hash);
return Self::payment_status(entry);
}

let payment =
self.payments.iter_mut().find(|entry| entry.payment_hash == new_htlc.payment_hash);
if let Some(entry) = payment {
// HTLCs within a payment should have the same payment hash.
debug_assert!(entry.htlcs.iter().all(|htlc| htlc.payment_hash == entry.payment_hash));
// The given HTLC should not already be present.
debug_assert!(entry
.htlcs
.iter()
.all(|htlc| htlc.intercept_id != new_htlc.intercept_id));
entry.htlcs.push(new_htlc);
let total_expected_outbound_amount_msat =
entry.htlcs.iter().map(|htlc| htlc.expected_outbound_amount_msat).sum();
(total_expected_outbound_amount_msat, entry.htlcs.len())
Self::payment_status(entry)
} else {
let expected_outbound_amount_msat = new_htlc.expected_outbound_amount_msat;
let entry =
Expand DownExpand Up@@ -127,6 +135,15 @@ mod tests {
(500_000_000, 2),
);

assert_eq!(
payment_queue.add_htlc(InterceptedHTLC {
intercept_id: InterceptId([2; 32]),
expected_outbound_amount_msat: 300_000_000,
payment_hash: PaymentHash([100; 32]),
}),
(500_000_000, 2),
);

let expected_entry = PaymentQueueEntry {
payment_hash: PaymentHash([100; 32]),
htlcs: vec![
Expand Down
192 changes: 192 additions & 0 deletions lightning-liquidity/src/lsps2/service.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -644,6 +644,26 @@ impl PeerState {
});
}

fn remove_terminal_channel_state(&mut self, channel_id: ChannelId) -> Option<u64> {
let intercept_scid = self.intercept_scid_by_channel_id.get(&channel_id).copied()?;
let should_remove = self
.outbound_channels_by_intercept_scid
.get(&intercept_scid)
.and_then(|entry| entry.get_channel_id())
.is_some_and(|existing_channel_id| existing_channel_id == channel_id);

if !should_remove {
return None;
}

self.outbound_channels_by_intercept_scid.remove(&intercept_scid);
self.intercept_scid_by_channel_id.remove(&channel_id);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| *iscid != intercept_scid);
self.needs_persist = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here is some overlap with #4703 perhaps.


Some(intercept_scid)
}

fn pending_requests_and_channels(&self) -> usize {
let pending_requests = self.pending_requests.len();
let pending_outbound_channels = self
Expand DownExpand Up@@ -1252,6 +1272,45 @@ where
Ok(())
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Will prune terminal JIT channel state once the corresponding channel has closed.
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub async fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let counterparty_node_id =
self.peer_by_channel_id.read().unwrap().get(&channel_id).copied();
let Some(counterparty_node_id) = counterparty_node_id else {
return Ok(());
};
Comment on lines +1283 to +1285

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Elsewhere we return APIError::APIMisuseError. Should we do the same here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good question. We could for consistency, but honestly I'm regretting we moved away from having the event-handling-related API idempotent. It seems not having it idempotent might run into issues / unexpected errors in case events get replayed on restart? But maybe that's okay?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmm... yeah I guess we return Ok(()) for htlc_intercepted, so might also depend on the event.


let removed_intercept_scid = {
let outer_state_lock = self.per_peer_state.read().unwrap();
match outer_state_lock.get(&counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
peer_state.remove_terminal_channel_state(channel_id)
},
None => None,
}
};

if let Some(intercept_scid) = removed_intercept_scid {
self.peer_by_intercept_scid.write().unwrap().remove(&intercept_scid);
self.peer_by_channel_id.write().unwrap().remove(&channel_id);
self.persist_peer_state(counterparty_node_id).await.map_err(|e| {
APIError::APIMisuseError {
err: format!(
"Failed to persist peer state after channel {} closed: {}",
channel_id, e
),
}
})?;
}

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
Expand DownExpand Up@@ -2270,6 +2329,25 @@ where
}
}

/// Forward [`Event::ChannelClosed`] event parameter into this function.
///
/// Wraps [`LSPS2ServiceHandler::channel_closed`].
///
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub fn channel_closed(&self, channel_id: ChannelId) -> Result<(), APIError> {
let mut fut = pin!(self.inner.channel_closed(channel_id));

let mut waker = dummy_waker();
let mut ctx = task::Context::from_waker(&mut waker);
match fut.as_mut().poll(&mut ctx) {
task::Poll::Ready(result) => result,
task::Poll::Pending => {
// In a sync context, we can't wait for the future to complete.
unreachable!("Should not be pending in a sync context");
},
}
}

/// Wraps [`LSPS2ServiceHandler::channel_needs_manual_broadcast`].
pub fn channel_needs_manual_broadcast(
&self, user_channel_id: u128, counterparty_node_id: &PublicKey,
Expand DownExpand Up@@ -2361,6 +2439,8 @@ mod tests {

use bitcoin::{absolute::LockTime, transaction::Version};
use core::str::FromStr;
use lightning::io::Cursor;
use lightning::util::ser::{Readable, Writeable};

const MAX_VALUE_MSAT: u64 = 21_000_000_0000_0000_000;

Expand DownExpand Up@@ -2764,6 +2844,118 @@ mod tests {
}
}

#[test]
fn replayed_intercepted_htlc_after_persist_is_idempotent() {
let payment_size_msat = Some(500_000_000);
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let intercept_scid = 42;
let user_channel_id = 43;
let htlc = InterceptedHTLC {
intercept_id: InterceptId([1; 32]),
expected_outbound_amount_msat: 500_000_000,
payment_hash: PaymentHash([2; 32]),
};

let mut jit_channel =
OutboundJITChannel::new(payment_size_msat, opening_fee_params, user_channel_id, false);
assert!(matches!(
jit_channel.htlc_intercepted(htlc).unwrap(),
Some(HTLCInterceptedAction::OpenChannel(_))
));

let mut peer_state = PeerState::new();
peer_state.intercept_scid_by_user_channel_id.insert(user_channel_id, intercept_scid);
peer_state.insert_outbound_channel(intercept_scid, jit_channel);

let encoded_peer_state = peer_state.encode();
let mut decoded_peer_state = PeerState::read(&mut Cursor::new(encoded_peer_state)).unwrap();
let decoded_jit_channel = decoded_peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
.unwrap();

assert!(decoded_jit_channel.htlc_intercepted(htlc).unwrap().is_none());

let ForwardPaymentAction(_, fee_payment) =
decoded_jit_channel.channel_ready(ChannelId([3; 32])).unwrap();
assert_eq!(fee_payment.htlcs, vec![htlc]);
}

#[test]
fn removes_terminal_state_for_closed_channel() {
let opening_fee_params = LSPS2OpeningFeeParams {
min_fee_msat: 10_000_000,
proportional: 10_000,
valid_until: LSPSDateTime::from_str("2035-05-20T08:30:45Z").unwrap(),
min_lifetime: 4032,
max_client_to_self_delay: 2016,
min_payment_size_msat: 10_000_000,
max_payment_size_msat: 1_000_000_000,
promise: "ignore".to_string(),
};
let stale_intercept_scid = 42;
let stale_user_channel_id = 43;
let stale_channel_id = ChannelId([44; 32]);
let live_intercept_scid = 45;
let live_user_channel_id = 46;
let live_channel_id = ChannelId([47; 32]);

let mut stale_jit_channel =
OutboundJITChannel::new(None, opening_fee_params.clone(), stale_user_channel_id, false);
stale_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: stale_channel_id };
let mut live_jit_channel =
OutboundJITChannel::new(None, opening_fee_params, live_user_channel_id, false);
live_jit_channel.state =
OutboundJITChannelState::PaymentForwarded { channel_id: live_channel_id };

let mut peer_state = PeerState::new();
peer_state.insert_outbound_channel(stale_intercept_scid, stale_jit_channel);
peer_state.insert_outbound_channel(live_intercept_scid, live_jit_channel);
peer_state
.intercept_scid_by_user_channel_id
.insert(stale_user_channel_id, stale_intercept_scid);
peer_state
.intercept_scid_by_user_channel_id
.insert(live_user_channel_id, live_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(stale_channel_id, stale_intercept_scid);
peer_state.intercept_scid_by_channel_id.insert(live_channel_id, live_intercept_scid);
peer_state.needs_persist = false;

assert_eq!(
peer_state.remove_terminal_channel_state(stale_channel_id),
Some(stale_intercept_scid)
);
assert!(!peer_state
.outbound_channels_by_intercept_scid
.contains_key(&stale_intercept_scid));
assert!(peer_state.outbound_channels_by_intercept_scid.contains_key(&live_intercept_scid));
assert!(!peer_state.intercept_scid_by_user_channel_id.contains_key(&stale_user_channel_id));
assert_eq!(
peer_state.intercept_scid_by_user_channel_id.get(&live_user_channel_id),
Some(&live_intercept_scid)
);
assert!(!peer_state.intercept_scid_by_channel_id.contains_key(&stale_channel_id));
assert_eq!(
peer_state.intercept_scid_by_channel_id.get(&live_channel_id),
Some(&live_intercept_scid)
);
assert!(peer_state.needs_persist);

peer_state.needs_persist = false;
assert_eq!(peer_state.remove_terminal_channel_state(stale_channel_id), None);
assert!(!peer_state.needs_persist);
}

#[test]
fn broadcast_not_allowed_after_non_paying_fee_payment_claimed() {
let min_fee_msat: u64 = 12345;
Expand Down
2 changes: 2 additions & 0 deletions lightning-liquidity/src/manager.rs
Original file line numberDiff line numberDiff line change
Expand Up@@ -256,13 +256,15 @@ where
/// - [`Event::ChannelReady`] to [`LSPS2ServiceHandler::channel_ready`]
/// - [`Event::HTLCHandlingFailed`] to [`LSPS2ServiceHandler::htlc_handling_failed`]
/// - [`Event::PaymentForwarded`] to [`LSPS2ServiceHandler::payment_forwarded`]
/// - [`Event::ChannelClosed`] to [`LSPS2ServiceHandler::channel_closed`]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add a pending changelog for this?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

For what exactly? For the fact that we expect users to now forward channel closed events? Or something else?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, since the docs say "If the LSPS2 service is configured, users must forward the following parameters from LDK events:"

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Added.

///
/// [`PeerManager`]: lightning::ln::peer_handler::PeerManager
/// [`MessageHandler`]: lightning::ln::peer_handler::MessageHandler
/// [`Event::HTLCIntercepted`]: lightning::events::Event::HTLCIntercepted
/// [`Event::ChannelReady`]: lightning::events::Event::ChannelReady
/// [`Event::HTLCHandlingFailed`]: lightning::events::Event::HTLCHandlingFailed
/// [`Event::PaymentForwarded`]: lightning::events::Event::PaymentForwarded
/// [`Event::ChannelClosed`]: lightning::events::Event::ChannelClosed
pub struct LiquidityManager<
ES: EntropySource + Clone,
NS: NodeSigner + Clone,
Expand Down
2 changes: 2 additions & 0 deletions pending_changelog/4656.txt
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
## API Updates
* The `LSPS2ServiceHandler` now expects LDK's `ChannelClosed` events to be forwarded to the new `channel_closed` method. (#4656)