LSP-Plugin: Add MPP support

[nepet]

Important

26.04 FREEZE March 11th: Non-bugfix PRs not ready by this date will wait for 26.06.

RC1 is scheduled on March 23rd

The final release is scheduled for April 15th.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

• The changelog has been updated in the relevant commit(s) according to the guidelines.
• Tests have been added or modified to reflect the changes.
• Documentation has been reviewed and updated as needed.
• Related issues have been listed and linked, including any that this PR closes.
• Important All PRs must consider how to reverse any persistent changes for tools/lightning-downgrade

Introduces a state-machine-based approach to managing LSPS2 JIT channel sessions, replacing the previous ad-hoc state tracking with a structured FSM that tracks payment collection from initial channel open through HTLC forwarding to completion.

• Is based on, and requires Withheld channel close: fail back inflight HTLCs and free UTXOs #8943 to be merged first.

[cdecker]

Cache cleared, and restarting the CI while I review the code 👍

[cdecker]

Nice PR, maybe a bit on the long side, and a bit of duplication, but the architecture is nice.

ACK

[cdecker]

Why the different types here?

[cdecker]

Just a quick doubt I had: this refers to which ID? As far as I remember the numbering of HTLCs was using a composite (channel_id, htlc_id) key because the protocol insists on numbering HTLCs, making either an alias necessary or the composite key to make them unique.

This is the DB HTLC ID that counts up monotonically, not the protocol ID which is per-channel, correct?

[nepet]

Very good question indeed, I'll need to check what exactly the htlc_accepted hook provides.

[cdecker]

That's quite the extensive list of potential errors and outcomes, nice listing 👍

[cdecker]

You'll probably want to break this up into a dispatch method (containing the match (field_a, field_b, input)) and several handler methods that take the involved parts and pieces of information as explicit parameters. That'll help get a quick overview, and allow diving deep on specific operations if interested.

[nepet]

do you mean something like the following? This would indeed make it more readable.

pub fn apply(&mut self, input: SessionInput) -> Result<ApplyResult> {
    match(&mut self.stat) {
        SessionState::Collecting => apply_on_collecting(input),
        SessionState::AwaitingChannelReady => apply_on_awaiting_channel_ready(input),
        ....
    }
}

// Maybe needs to get the state passed here and reinjected into the Session later
fn apply_on_collecting(&mut self, input: SessionInput) -> Result<ApplyResult> {
    // Check what needs to be "taken" here to please the ownership model
    match(input) {
        ....
    }
}

[cdecker]

Yep, pretty much, can be a cleanup though. Also a quick diagram on allowable state changes would make this much easier to follow as well. All followup things though, not a blocker.

[cdecker]

That is not to say I don't like the (state, event) matching, as that automatically gives you an indication whether all states and transitions are covered, via the exhaustive enumeration rule in Rust.

[cdecker]

The backlog is mostly intended for bursty behavior, and should be set to the maximum number of events in flight. If there is no more room, we will drop block the sending side, no messages should be lost. If the sender does not need to make progress, and the expectation is that we can process messages in the order they arrive (no interleaving) it should be safe to set the backlog to 1. Don't think about elements queued up, rather consider if you need to make progress on the sendign side at all, while elements are being processed.

[cdecker]

Love the match () {} matrix, less so the deep nesting, as it pulls the matrix apart and makes reasoning about its relations and transitions harder :-)

[nepet]

I'll clean it up and move stuff into handler functions

[cdecker]

Hm, this will break megalithic LSP? They do not have a way to set no reserve.

[nepet]

Whoo! GOOD catch. totally forgot about megalith here. I'll make it an option with a sane (set nothing) default

[cdecker]

Is this not duplicating the entire FSM logic, just because we enter the system through recovery, rather than kicking off a new session? We could just call into the dispatch of events here, and everything else would be the same, or am I missing something?

[nepet]

Let me see if I can clean it up a bit

[cdecker]

Are there "let's define ALL the operations on a variant of the original behavior" useful? I am failing to see how defining the operations on Arc<T> and then just having them forward to T could be useful 🤔

[cdecker]

The CI failures appear to NOT be flaky tests, as I am seeing a lot of the new tests failing on us.

[madelinevibes]

@nepet can you rebase, resolve conflicts and clear CI by the end of this week so we can add this to 26.06?
Release candidate planned for Monday 11 May.

[daywalker90]

rebased on master

[cdecker]

These are guaranteed to be new entries, right? If you try to load an existing one with non-optional fields it'll likely fail.

[cdecker]

Yep, pretty much, can be a cleanup though. Also a quick diagram on allowable state changes would make this much easier to follow as well. All followup things though, not a blocker.

[cdecker]

You probably don't want to use UNKNOWN_NEXT_PEER as that is a permanent error, and the client is supposed to remember that the channel never existed, and future attempts with blocklist them. It's a common error lnd always gets wrong too.

[cdecker]

Sorry my eyes glazed over at this point, I trust the logic is sensible, as reconstructing it from code is painful.

[cdecker]

That is not to say I don't like the (state, event) matching, as that automatically gives you an indication whether all states and transitions are covered, via the exhaustive enumeration rule in Rust.

[nGoline]

Re-reviewed after the 2026-08-19 rebase with more context.

The FSM fixes it: poll_channel_ready's 120s deadline plus check_cltv_timeout bound the wait, and withhold:true means the failure path never broadcasts the funding tx at all.

Two things from my 2026-08-04 review are still open on 3e1dce5 and I've actually found 2 more blocking issues (manager.rs:194, manager.rs:314) (comments in-line).

[nGoline]

Ack!

[daywalker90]

There seems to be a flaky lsps test: https://github.com/ElementsProject/lightning/actions/runs/32951386026/job/98158303280?pr=8948

[daywalker90]

I have added a fix for a test flake i found locally. If i can get an ack on that i want to squash the commits to keep bisectability. For the commit message i want to put:

    plugins(lsps2): add LSPS2 MPP payment collection with session FSM

    Implements an FSM-based session manager for LSPS2 payment collection,
    covering channel discovery, session persistence and restart recovery,
    forwarding/HTLC handling, and channel reserve management.

    - add session FSM for LSPS2 payment collection
    - add session actor with action executor boundary
    - add session manager and unify HTLC handling
    - add integration tests for session lifecycle
    - add restart recovery for session persistence
    - simplify DatastoreProvider to focused trait
    - add EventSink trait for session event notification
    - decouple lsps2 crate from CLN-specific types
    - simplify actor and session manager
    - make collect timeout dev config
    - route recovered sessions through forward_event path
    - adding list_finalized_sessions to the DatastoreProvider
    - key HTLCs by (incoming channel, htlc id)
    - forward replayed HTLCs to recovered sessions
    - prevent double channel funding for one payment hash
    - fail held HTLCs before cltv expiry, not after
    - spawn only one channel-poll task per session
    - harden recovery loop and fee-overflow handling
    - advertise client_trusts_lsp and validate payment size range
    - make channel reserve opt-in and pin datastore compat
    - make finalize_session idempotent
    - only expire offers when starting a new session
    - always release funding inputs when abandoning
    - only abandon session once no forwarded parts remain
    - pytest: cover finalize_session over an existing finalized entry
    - docs: add experimental lsps options to config
    - Add u16 zero len to TEMPORARY_CHANNEL_FAILURE
    - route parts by scid, not payment hash alone
    - fund at most one channel per jit scid
    - monitor recovered sessions for channel death
    - release the channel when recovery finds all forwards failed
    - bound variable-amount payments by the policy size range

    Changelog-Experimental: LSPS2 session state machine for JIT channels

Does that sound good?