Skip to content

fix: skip custom message WebSocket subscription for REST-based backends at startup#3948

Open
myxmaster wants to merge 1 commit intoZeusLN:masterfrom
myxmaster:fix/lsp-custom-msg-subscription
Open

fix: skip custom message WebSocket subscription for REST-based backends at startup#3948
myxmaster wants to merge 1 commit intoZeusLN:masterfrom
myxmaster:fix/lsp-custom-msg-subscription

Conversation

@myxmaster
Copy link
Copy Markdown
Collaborator

@myxmaster myxmaster commented Apr 5, 2026

Description

When connecting to an LND REST wallet while an embedded-lnd wallet config also exists, a spurious subscribeCustomMessages WebSocket error appeared in the debug output.

For LND REST and LNC, LSPS1 uses the REST API and LSPS7 uses custom messages only as a fallback when channels with the LSP already exist. The WebSocket subscription to /v1/custommessage/subscribe is therefore not needed unconditionally at startup.

Changes:

  • Wallet.tsx: only call subscribeCustomMessages at startup for backends that use custom messages for LSPS1 (!supportsLSPS1rest())
  • LSPStore channels reaction: subscribe lazily just before getExtendableChannels() for backends using the custom message LSPS7 fallback (LND REST, LNC), and only when a channel with the LSP actually exists
  • LSPStore subscribeCustomMessages: set customMessagesSubscriber = true in the non-embedded-lnd branch so the deduplication guard works correctly; ; reset it to undefined on WebSocket error so a subsequent retry is possible; fix error logging (error instead of error.message)

This pull request is categorized as a:

  • New feature
  • Bug fix
  • Code refactor
  • Configuration change
  • Locales update
  • Quality assurance
  • Other

Checklist

  • I’ve run yarn run tsc and made sure my code compiles correctly
  • I’ve run yarn run lint and made sure my code didn’t contain any problematic patterns
  • I’ve run yarn run prettier and made sure my code is formatted correctly
  • I’ve run yarn run test and made sure all of the tests pass

Testing

If you modified or added a utility file, did you add new unit tests?

  • No, I’m a fool
  • Yes
  • N/A

I have tested this PR on the following platforms (please specify OS version and phone model/VM):

  • Android
  • iOS

I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):

On-device

  • LDK Node
  • Embedded LND

Remote

  • LND (REST)
  • LND (Lightning Node Connect)
  • Core Lightning (CLNRest)
  • Nostr Wallet Connect
  • LndHub

Locales

  • I’ve added new locale text that requires translations
  • I’m aware that new translations should be made on the ZEUS Transfix page and not directly to this repo

Third Party Dependencies and Packages

  • Contributors will need to run yarn after this PR is merged in
  • 3rd party dependencies have been modified:
    • verify that package.json and yarn.lock have been properly updated
    • verify that dependencies are installed for both iOS and Android platforms

Other:

  • Changes were made that require an update to the README
  • Changes were made that require an update to onboarding

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements lazy subscription for custom messages, specifically targeting backends that use REST for LSPS1 but require custom messages for LSPS7 fallback. It also introduces a sentinel to prevent duplicate subscriptions. Feedback was provided regarding the need to reset this sentinel in the event of a WebSocket error to ensure that transient failures do not permanently block future subscription attempts.

@myxmaster myxmaster force-pushed the fix/lsp-custom-msg-subscription branch from fb6e946 to 25c6abb Compare April 5, 2026 14:06
@kaloudis
Copy link
Copy Markdown
Contributor

kaloudis commented Apr 6, 2026

/v1/custommessage/subscribe is used for LSPS7

@myxmaster
Copy link
Copy Markdown
Collaborator Author

Updated the PR description for clarity.

You're right that /v1/custommessage/subscribe is needed for LSPS7 -> the subscription is now set up lazily in the channels reaction, just before getExtendableChannels(), when a channel with the LSP actually exists.

Copy link
Copy Markdown
Contributor

@ajaysehwal ajaysehwal left a comment

Choose a reason for hiding this comment

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

tACK

@kaloudis
Copy link
Copy Markdown
Contributor

kaloudis commented Apr 6, 2026

Not sure this is worth the added complexity, especially when we're considering refactoring the wallet startup calls out of Wallet.tsx and into the individual backends.

@myxmaster
Copy link
Copy Markdown
Collaborator Author

Fair point. If that is on the near-term roadmap, let's close this and let the refactor handle it cleanly instead.

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.

3 participants