Skip to content

Fix lost HTLC/channel events by reconnecting lnd subscriptions on drop - #554

Open
markettes wants to merge 8 commits into
mainfrom
fix/lnd-connection-bug
Open

Fix lost HTLC/channel events by reconnecting lnd subscriptions on drop#554
markettes wants to merge 8 commits into
mainfrom
fix/lnd-connection-bug

Conversation

@markettes

@markettes markettes commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

LND's HTLC/channel event streams are live-only with no replay, so when a stream dropped (e.g. on a release) the subscription job exited silently and stayed dead until the next NodeGuard restart — losing every event in the gap.
This adds SubscriptionStreamRunner, a shared resubscribe-with-backoff loop that reconnects on any stream termination (clean end or error) and evicts the stale gRPC channel via a new InvalidateClient before retrying.
Covered by 5 new unit tests; TCP-keepalive/half-open-connection hardening is intentionally deferred to a follow-up PR.

@markettes
markettes requested a review from Jossec101 August 3, 2026 11:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens NodeGuard’s long-lived LND HTLC/channel subscription jobs by introducing a shared runner that automatically resubscribes with exponential backoff when a streaming RPC terminates, preventing silent job death and lost live-only events across LND restarts/releases.

Changes:

  • Added SubscriptionStreamRunner helper to keep streaming subscriptions alive via resubscribe/backoff and optional gRPC client invalidation on errors.
  • Updated HTLC and channel subscription Quartz jobs to use the shared runner and to invalidate cached gRPC channels when streams fail.
  • Added unit tests covering eligibility, cancellation, per-event handling, clean end-of-stream resubscribe, and error retry/invalidation.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
test/NodeGuard.Tests/Helpers/SubscriptionStreamRunnerTests.cs Adds unit tests validating resubscribe behavior, cancellation handling, and invalidation on failures.
src/Helpers/SubscriptionStreamRunner.cs Introduces a reusable loop to run streaming subscriptions with reconnect/backoff behavior.
src/Jobs/NodeHtlcSubscribeJob.cs Switches HTLC subscription to the runner to avoid silent stream death and to reconnect automatically.
src/Jobs/NodeChannelSubscribeJob.cs Switches channel event subscription to the runner and changes per-event error behavior to “skip and continue”.
src/Services/LightningClientService.cs Adds InvalidateClient to evict/dispose cached Lightning gRPC channels on failures.
src/Services/LightningRouterService.cs Adds InvalidateClient to evict/dispose cached Router gRPC channels on failures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Helpers/SubscriptionStreamRunner.cs Outdated
Comment thread src/Services/LightningClientService.cs
Comment thread src/Services/LightningRouterService.cs
Comment thread src/Jobs/NodeHtlcSubscribeJob.cs

@Jossec101 Jossec101 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can this be e2e tested? @markettes

markettes and others added 2 commits August 3, 2026 17:29
- InvalidateClient: remove the cached channel under the _clients lock but dispose
  it outside the lock, so GrpcChannel.Dispose (which can block) no longer stalls
  concurrent GetLightningClient/GetRouterClient calls.
- SubscriptionStreamRunner: dispose the AsyncServerStreamingCall each iteration
  (using var) so repeated resubscribes don't leak the stream.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@markettes

Copy link
Copy Markdown
Contributor Author

can this be e2e tested? @markettes

Added! 🚀

@markettes
markettes requested a review from Jossec101 August 3, 2026 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants