lightninglabs / lightninglabs/loop

withdraw: retry confirmation watching on failure and handle reorgs

Open
#1,087 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
595
Forks
135
Avg merge
1d 8h
Merged PRs (30d)
12

Description

## Context

From [PR #1078 review](https://github.com/lightninglabs/loop/pull/1078#discussion_r2870278126), @starius identified two improvements needed for the withdrawal confirmation watching code in `staticaddr/withdraw/manager.go`.

## Current Behavior

In `handleWithdrawal`, when `RegisterConfirmationsNtfn` or `RegisterSpendNtfn` fails, the error is logged and the goroutine returns. There is no retry mechanism, so the withdrawal may never be detected as confirmed.

Additionally, if a withdrawal transaction receives one confirmation but is then reorged out, the current code does not handle this situation — the withdrawal stays in a confirmed state despite no longer being in the chain.

## Issue 2: State persistence failure on handleWithdrawal error

From [PR #1078 review](https://github.com/lightninglabs/loop/pull/1078#discussion_r2894247899), @starius identified a related issue in the same code path:

If `handleWithdrawal` fails (because `GetStaticAddressParameters` or `RegisterSpendNtfn` fails), the caller (`WithdrawDeposits`) returns the error **without** updating internal state or persisting the state change to `Withdrawing`. This is a correctness/state-tracking failure — the withdrawal tx may already be published, but the restart recovery path won't pick it up because recovery sources on `Withdrawing` deposits.

**Recommended fix shape:**

1. After publish success, immediately persist/transition the withdrawal state (`FinalizedWithdrawalTx`, `finalizedWithdrawalTxns`, `Withdrawing`, DB update) **before** calling `handleWithdrawal`.
2. Start `handleWithdrawal` as best-effort; if it fails, log + schedule retry, but do **not** return a user-facing error for an already-published tx.

**Secondary improvement to `handleWithdrawal` itself:**

1. Stop fetching address params inside `handleWithdrawal`; pass the required `pkScript` from the caller.
2. The `pkScript` (of the static address) can be calculated upfront, even before publishing, at the very beginning of FSM operation.

## Proposed Changes

1. **Retry on next block**: If `Register*` calls fail, retry at the next block detection instead of giving up. This retry-on-next-block pattern is already used in other parts of the codebase.

2. **Handle reorgs**: If a withdrawal tx gets confirmed but is later reorged out, detect and handle this automatically (e.g., re-register for spend/confirmation notifications).

3. **Decouple state persistence from handleWithdrawal**: Persist the `Withdrawing` state transition immediately after successful tx publish, so that restart recovery works even if `handleWithdrawal` subsequently fails.

4. **Pass pkScript from caller**: Remove `GetStaticAddressParameters` call from `handleWithdrawal` and pass the `pkScript` in from the caller, where it can be computed once upfront.

## References

- File: `staticaddr/withdraw/manager.go`, around the `handleWithdrawal` function
- PR: #1078

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in staticaddr/withdraw/manager.go, reading handleWithdrawal and its caller WithdrawDeposits, along with the RegisterConfirmationsNtfn and RegisterSpendNtfn paths. Trace the existing next-block retry pattern elsewhere in the codebase and the Withdrawing state persistence flow. Done means failed registrations retry, reorged confirmations are handled, published withdrawals remain recoverable, and pkScript is passed from the caller.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, blockchain
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.