tests: invariant checks for the LSPS client tests by nepet · Pull Request #9657 · ElementsProject/lightning

nepet · 2026-10-23T12:12:31Z

Important

26.09 FREEZE August 5th: Non-bugfix PRs not ready by this date will wait for 26.12.

RC1 is scheduled on August 17th

The final release is scheduled for September 7th.

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

This adds tests/lsps2_oracles.py, a set of checks that each LSPS client test runs at its end, whatever the mock did. check_oracles(client, lsp) reads the client's invoices, channels and log, plus the mock's records, and reports every violation it finds:

amount: the client accepts no payment for less than the invoice amount minus the opening fee! The fee is taken once per JIT channel, each HTLC is short of its onion amount by exactly its extra_fee
identity: route hints go only through the LSP, on a SCID it issued to this client. The only channel used before its funding confirms is the one opened for the purchase
params: the client buys only with params exactly as issued, before they expire, with consistent size limits and a valid promise. The invoice matches the buy result
robustness: the client plugin keeps running, nothing crashed in the log, no HTLC stays stuck, and the client sends no flood of requests.

The checks work on a plain-data snapshot, so tests/test_lsps2_oracles.py unit-tests them on hand-written runs. The client itself is not changed yet.

cdecker · 2026-10-23T18:13:25Z

+    return [cid for cid in found if cid.startswith(oracle)]
+
+
+def test_lsps2_oracles_honest_run():


What's the rationale for these tests. They seem to be checking the message formats, and it seems to me like a JSON schema che per would be better suited for this. Otherwise it may end up with tests testing the test payloads without running the tests 😅

Good question, I should have made that clearer! These aren't really format checks. They're unit tests for the oracles in lsps2_oracles.py, and most of those compare data from different places: the invoice amount against the fee computed from the bought params, each HTLC's extra_fee against its onion and forward amounts, the route-hint SCID against the SCIDs issued to this client, the buy time against valid_until, and so on. As far as I can tell, a schema can't express those. The promise check is the only one that's really schema-like.

As for why they get their own tests: in the real client tests (#9658) the client behaves, so every oracle passes. An oracle with a bug that means it can never fire would pass just the same, and we'd never notice. So each test here takes an honest run, breaks one thing, and checks that exactly the matching check fires. The honest run itself makes sure there are no false positives.

I've added this to the module docstring in 0c842c3.

daywalker90 · 2026-10-26T10:11:11Z

needs rebase

Andezion · 2026-10-27T12:16:12Z

+        return not any(c["htlcs"] for c in channels)
+
+    try:
+        wait_for(settled)


if a test expects HTLCs to stay held, this waits the full TIMEOUT before reporting. should waiving robustness.hanging skip the wait?

When a test waives robustness.hanging it's saying HTLCs may stay held, so check_oracles now skips the wait then. Fixed in fef4211

Andezion · 2026-10-27T12:16:53Z

+MAX_PROMISE_BYTES = 512
+
+# LSPS requests a client may send in one test; a retry loop exceeds it.
+MAX_LSPS_REQUESTS = 20


20 requests is a limit for the whole test. a test with +-7 jit invoices would hit it without any retry loop. should this be per session?

I'd keep it per test. No test in this stack sends more than one JIT invoice, so the limit is nowhere near. Per session doesn't fit well: list_protocols and get_info come before any session exists, and a client that keeps buying would open a new session each time and so raise its own limit. If a test ever needs many invoices, we can raise the bound then.

Andezion · 2026-10-27T12:17:50Z

+                for hop in route:
+                    session = self.session(jit_channel_scid=hop["short_channel_id"])
+                    if session is not None:
+                        out.append((inv, session))


an invoice with two hops on the same session is added twice, so violations are reported twice. maybe a break after the first match would fix it?

Right. A break would only cover hops within one route, and the same session can also show up in two route hints, so lsps2_invoices now pairs an invoice with each session only once. Two hops on the JIT SCID are flagged by identity.route_hint as well. Fixed in f20d209.

Andezion · 2026-10-27T12:20:10Z

    only_one(l1.rpc.listpeerchannels()["channels"])
    paid = only_one(l1.rpc.listinvoices("lsp-jit-channel-0")["invoices"])
    assert paid["status"] == "paid"
+    check_oracles(l1, l2)


i think this check_oracles fails on ci (Valgrind 5/12): params.invoice: invoice 'lsp-jit-channel-0' expires after the params' valid_until

but i think the oracle is right. the client computes expiry once at client.rs at line 421, then creates the invoice twice (public and reduced copy). if more than +-1s passes in between, the second invoice outlives valid_until

maybe we could either fix the client, or waive params.invoice here with a todo?? what do you think?

The oracle is right: the client computes expiry once and uses it for both the public invoice and its reduced copy, so the copy can outlive valid_until by however long the first invoice and the delinvoice took. It's a small fix, so I've added it here as the first commit: the copy's expiry is now computed from valid_until right before it's created.

Andezion · 2026-10-27T12:22:03Z

+            e
+            for e in self.events
+            if e["type"] == "htlc"
+            and e["decision"] == "forward"


this counts every htlc the mock forwarded, including ones the client later failed. if the client rejects a bad part and the payer retries, both forwards count. then amount.extra_fee and amount.received fire, but the client did the right thing?

maaaaybe the mock can record in_channel/in_htlc_id, so we keep only forwards that are settled in listforwards?

Yes. The mock now records each forward's incoming in_channel/in_htlc_id, and Observation.from_nodes takes the forward's status from the LSP's listforwards and only settled parts count. There's a unit test for a failed part next to a settled one. Fixed.

Andezion · 2026-10-27T12:23:36Z

+
+    def settled():
+        channels = client.rpc.listpeerchannels()["channels"]
+        return not any(c["htlcs"] for c in channels)


htlcs is not always in listpeerchannels?? (for example, an OPENINGD entry has none), so this can raise KeyError. maybe we should use c.get("htlcs", [])? and the same stuff with the line 485, and to_us_msat on line 319

Yes, an OPENINGD entry has no htlcs, so both places use .get now. to_us_msat on line 319 is only read for channels the peer opened, and an OPENINGD entry is always our own open, so it can't get there. Fixed.

For a fixed amount the client creates the invoice twice: the public one and, after deleting it, a copy for the amount minus the opening fee, which the reduced HTLCs pay. Both used an expiry computed before the first call, so the copy expired later than the public invoice by however long creating and deleting that took, which can be past the params' valid_until. Compute the copy's expiry from valid_until when creating it. Changelog-Fixed: lsps-plugin: for a fixed amount, the invoice `lsps-jitchannel` stores for the reduced payment no longer stays valid past the opening fee params' `valid_until`.

Every LSPS2 client test should end by checking the same invariants on what the client did, whatever the mock service misbehaved in. Add tests/lsps2_oracles.py with the first two: - Amount: a paid invoice received at least its amount minus the opening fee it owes (none for payments after the session), and the client holds no more in channels its peers opened than its paid invoices received. - Identity: invoices route only through known channels or through the LSP on a jit_channel_scid it issued to this client, and the only channel used before its funding confirms is the session's. The checks work on an Observation snapshot of plain data, gathered from the client and the mock's state, so they are unit tested on hand- written runs. For that the mock now timestamps its events, reports the params it issued, remembers which payment opened each session and records the incoming HTLC of each part it forwards, so that only the parts the client settled count. The honest JIT tests now run the oracles.

Expired, contradictory or modified params must never lead to a payment. Two checks: - Every lsps2.buy the client sent uses params exactly as the LSP issued them, before valid_until, with usable payment size limits, a payment size the fee does not eat up, and a promise of at most 512 bytes of printable ASCII without '"' or '\'. - Every LSPS2 invoice follows the buy result the client was sent: the lsp_cltv_expiry_delta in a route hint without fees, an expiry no later than valid_until and the payment size as amount.

The last oracle for the LSPS2 client tests: the LSPS client plugin is still running and never crashed, no HTLC is left pending in the client's channels, and the client sent no more than 20 LSPS requests. check_oracles first waits for the client's HTLCs to settle, so that a check does not race a payment still in flight, unless the test waives robustness.hanging.

A client that accepts a part whose extra_fee does not match what was deducted from it breaks bLIP-52 even when the total deducted happens to be the right fee, and the amount oracle did not see that. Check that each part of a paid invoice arrived short by exactly its extra_fee and that the extra_fees add up to at most what the payment owes.

nepet requested review from Andezion, cdecker and daywalker90 October 23, 2026 12:12

nepet added this to the v26.12 milestone Oct 23, 2026

nepet mentioned this pull request Oct 23, 2026

tests: run the LSPS client tests against the mock service #9658

Open

5 tasks done

cdecker previously approved these changes Oct 23, 2026

View reviewed changes

nepet dismissed cdecker’s stale review via 0c842c3 October 26, 2026 09:18

nepet requested a review from cdecker October 26, 2026 09:27

daywalker90 added the needs-rebase label Oct 26, 2026

nepet force-pushed the lsps-mock-3-oracles branch 2 times, most recently from eb8df2a to 842cce7 Compare October 27, 2026 12:00

nepet removed the needs-rebase label Oct 27, 2026

Andezion requested changes Oct 27, 2026

View reviewed changes

nepet force-pushed the lsps-mock-3-oracles branch 2 times, most recently from 1190231 to a86df62 Compare October 28, 2026 08:48

nepet added 6 commits October 28, 2026 09:49

tests: explain why the LSPS2 oracles have their own unit tests

220062b

nepet force-pushed the lsps-mock-3-oracles branch from a86df62 to 220062b Compare October 28, 2026 08:54

nepet requested a review from Andezion October 28, 2026 08:58

		return [cid for cid in found if cid.startswith(oracle)]


		def test_lsps2_oracles_honest_run():

Conversation

nepet commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daywalker90 commented Oct 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nepet Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nepet commented Oct 23, 2026 •

edited

Loading

nepet Oct 27, 2026 •

edited

Loading