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.

nGoline

Two small, non-blocking things:

check_invoice compares expires_at against ceil(valid_until), while the client computes the invoice expiry from its own clock before the invoice call. If that call takes more than a second or so (Valgrind on a busy runner), the oracle can still fire on an honest client, now for one call instead of invoice + delinvoice. A 1-2 s margin in the client's expiry, or 1 s of tolerance in the oracle, would close it.
In Observation.forwards, a forward that does not match any listforwards entry gets status=None and is dropped, and check_amount then skips the invoice as "not paid through the mock". If the join ever breaks, the amount checks pass without checking anything. Flagging "paid invoice with mock forwards but no matched status" would keep them honest.

LGTM once Andezion has had another look.

jaonoctus

Agree with @nGoline's two points.

and one nit: the Changelog-Fixed line in 15e4837 says lsps-jitchannel, but the RPC is registered as lsps-lsps2-invoice. Worth rewording it?

LGTM once @Andezion has had another look

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. lightningd adds the expiry to its own clock when it creates the invoice, so the time the `invoice` call itself takes counts as well. Take a few seconds off both expiries so that a slow call doesn't push either invoice past valid_until. Changelog-Fixed: lsps-plugin: `lsps-lsps2-invoice` no longer creates an invoice that 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 · 2026-11-12T15:25:15Z

@nGoline both points held up.

Expiry: lightningd takes the creation time from its own clock, in whole seconds, when the invoice call runs. So the client's rounding plus the time that call takes could put expires_at past valid_until. I fixed this in the client rather than giving the oracle a tolerance, because catching exactly that is the oracle's job. Both invoices now get 5s off their expiry. The client already drops params that expire within a minute, so this doesn't change which params it picks.
Unmatched forwards: added amount.unmatched. It fires for any part the mock forwarded that has no entry in the LSP's listforwards, so if that matching breaks, the amount checks can no longer pass without checking anything. Parts still in flight show up as offered, not missing, so they don't trigger it. There's a unit test for it.

@jaonoctus reworded: the changelog line now names lsps-lsps2-invoice.

Both are squashed into the existing commits: the client fix in 2235ff5, the oracle in 04bdb15.

jaonoctus

tACK a1f7a44

Built on macOS arm64 (rustc 1.99.1, bitcoind v32.1.0) and ran the LSPS tests without Valgrind.

Test log

$ VALGRIND=0 TEST_DIR=/tmp pytest tests/test_cln_lsps.py tests/test_lsps2_oracles.py -v -n 4
tests/test_cln_lsps.py::test_lsps_service_disabled PASSED
tests/test_cln_lsps.py::test_lsps0_listprotocols PASSED
tests/test_cln_lsps.py::test_lsps2_buy PASSED
tests/test_cln_lsps.py::test_lsps2_buyjitchannel_mpp_fixed_invoice PASSED
tests/test_cln_lsps.py::test_lsps2_buyjitchannel_no_mpp_var_invoice PASSED
tests/test_cln_lsps.py::test_lsps2_enabled PASSED
tests/test_cln_lsps.py::test_lsps2_getinfo PASSED
tests/test_cln_lsps.py::test_lsps2_mock_buy_fee_checks PASSED
tests/test_cln_lsps.py::test_lsps2_mock_buy_result PASSED
tests/test_cln_lsps.py::test_lsps2_mock_config_option PASSED
tests/test_cln_lsps.py::test_lsps2_mock_expired_held_parts PASSED
tests/test_cln_lsps.py::test_lsps2_mock_expired_session PASSED
tests/test_cln_lsps.py::test_lsps2_mock_fee PASSED
tests/test_cln_lsps.py::test_lsps2_mock_htlc_deduct PASSED
tests/test_cln_lsps.py::test_lsps2_mock_htlc_error PASSED
tests/test_cln_lsps.py::test_lsps2_mock_jit_channel_mpp_fixed_invoice PASSED
tests/test_cln_lsps.py::test_lsps2_mock_jit_channel_var_invoice PASSED
tests/test_cln_lsps.py::test_lsps2_mock_open_disconnect PASSED
tests/test_cln_lsps.py::test_lsps2_mock_open_failed PASSED
tests/test_cln_lsps.py::test_lsps2_mock_open_never PASSED
tests/test_cln_lsps.py::test_lsps2_mock_protocol PASSED
tests/test_cln_lsps.py::test_lsps2_mock_protocol_errors PASSED
tests/test_cln_lsps.py::test_lsps2_mock_rejects_missing_parameters PASSED
tests/test_cln_lsps.py::test_lsps2_mock_rejects_unknown_config PASSED
tests/test_cln_lsps.py::test_lsps2_mock_request_error PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport0-expected0] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport1-expected1] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport2-expected2] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport3-expected3] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport4-expected4] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport5-expected5] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport6-expected6] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_transport_behaviors[transport7-expected7] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_var_invoice_payment_size[1000] PASSED
tests/test_cln_lsps.py::test_lsps2_mock_var_invoice_payment_size[10000001] PASSED
tests/test_cln_lsps.py::test_lsps2_non_approved_zero_conf PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_amount PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_balance PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_buy_params PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_extra_fee PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_forward_status PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_honest_run PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_invoice PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_robustness PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_route_hint PASSED
tests/test_lsps2_oracles.py::test_lsps2_oracles_zero_conf_channel PASSED

======================== 46 passed in 91.19s (0:01:31) =========================

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 3 times, most recently from a86df62 to 220062b Compare October 28, 2026 08:54

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

daywalker90 reviewed Nov 6, 2026

View reviewed changes

Comment thread plugins/lsps-plugin/src/client.rs Resolved

Comment thread tests/lsps2_oracles.py Resolved

Comment thread tests/lsps2_oracles.py Resolved

nepet force-pushed the lsps-mock-3-oracles branch from 220062b to 6ea2694 Compare November 10, 2026 15:37

daywalker90 previously approved these changes Nov 11, 2026

View reviewed changes

nGoline previously approved these changes Nov 11, 2026

View reviewed changes

jaonoctus previously approved these changes Nov 11, 2026

View reviewed changes

nepet force-pushed the lsps-mock-3-oracles branch from 6ea2694 to d61f0cf Compare November 12, 2026 14:48

nepet added 6 commits November 12, 2026 16:09

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

a1f7a44

nepet dismissed stale reviews from jaonoctus, daywalker90, and nGoline via a1f7a44 November 12, 2026 15:19

nepet force-pushed the lsps-mock-3-oracles branch from d61f0cf to a1f7a44 Compare November 12, 2026 15:19

jaonoctus approved these changes Nov 12, 2026

View reviewed changes

		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!

Uh oh!

Uh oh!

Uh oh!

nGoline left a comment

Choose a reason for hiding this comment

Uh oh!

jaonoctus left a comment • 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!

nepet commented Nov 12, 2026

Uh oh!

jaonoctus left a comment

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!

6 participants

nepet commented Oct 23, 2026 •

edited

Loading

nepet Oct 27, 2026 •

edited

Loading

jaonoctus left a comment •

edited

Loading