lightninglabs / lightninglabs/loop
withdraw: retry confirmation watching on failure and handle reorgs
- 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