Skip to content

fix(gl-plugin): skip LSPs returning empty opening params for LSPS2#731

Open
cdecker wants to merge 4 commits into
mainfrom
fix/lsps2-empty-opening-params
Open

fix(gl-plugin): skip LSPs returning empty opening params for LSPS2#731
cdecker wants to merge 4 commits into
mainfrom
fix/lsps2-empty-opening-params

Conversation

@cdecker

@cdecker cdecker commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

An LSP could be selected for LSPS2 invoice negotiation even when it returned an empty opening-fee-params set. It would still get picked as lsps[0], leaving us without valid opening params for the invoice.

This PR extracts the selection logic into select_opening_params, which flattens the params across all LSPs and picks the first available (node_id, params) pair. An LSP that returned no params is now skipped in favor of the next LSP that returned usable params. We only error out when no LSP returned any params at all.

Changes

  • fix: select_opening_params helper + updated invoice negotiation path.
  • style: a separate preceding commit applies rustfmt-only whitespace cleanup across gl-plugin (no behavior change), kept apart so this fix is easy to review.

Tests

Added unit tests covering:

  • no LSPs at all → None
  • first LSP empty, second has params → falls back to the second
  • all LSPs empty → None
  • first LSP non-empty → keeps using it
test node::test::test_select_opening_params_all_empty ... ok
test node::test::test_select_opening_params_first_lsp_empty ... ok
test node::test::test_select_opening_params_prefers_first_nonempty ... ok
test node::test::test_select_opening_params_empty ... ok

🤖 Generated with Claude Code

cdecker and others added 4 commits June 29, 2026 16:17
The test harness picked the default CLN node version with a lexicographic
`max()` over the manifest tags (and a `[... "gl" ...][-1]` in the `paths`
fixture). As soon as a new release lands in the shared manifest.json it
becomes the default for the whole suite, even if the client and signer do
not support it yet. That is how `v26.06gl1` started being used before
support was built in.

Make selection deterministic and explicitly bounded:

- Add `version_sort_key()`/`version_base()` parsing the `vX.Y[.Z][glN]`
  tags into numeric, ordered keys. The `glN` suffix is a separate
  component, so ordering is numeric (not lexicographic) and the base
  version can be compared independently of the greenlight suffix.
- Add `ClnVersionManager.supported_versions(lowest, highest)` and
  `latest_supported(lowest, highest)`, filtering to base versions within
  `[lowest, highest]` inclusive and dropping non-numbered tags (`main`).
- Make the existing `latest()` deterministic too.
- In the gl-testing fixtures, pin `LOWEST_SUPPORTED_VERSION` explicitly and
  derive `HIGHEST_SUPPORTED_VERSION` from `glclient.__version__`, i.e. what
  the signer (libhsmd) actually supports. The suffix is ignored when
  comparing, so the signer reporting `v25.12` matches `v25.12gl1` but
  excludes `v26.06gl1`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No functional change; empty commit to re-run the pipeline.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Whitespace-only cleanup across gl-plugin (import ordering, line
wrapping, trailing commas, struct field alignment). No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
An LSP could be selected for LSPS2 invoice negotiation even when it
returned an empty opening-fee-params set, leaving us without valid
opening params. Extract the selection into `select_opening_params`,
which flattens params across all LSPs and picks the first available
pair, so an LSP with no params is skipped in favor of the next LSP that
returned usable params. Returns an error only when no LSP returned any
params at all.

Add unit tests covering the empty, first-LSP-empty, all-empty, and
first-non-empty cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@cdecker cdecker force-pushed the fix/lsps2-empty-opening-params branch from cb20202 to 4d07aa1 Compare June 29, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant