-
Notifications
You must be signed in to change notification settings - Fork 124
Description
Context
From PR #1078 review, @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, @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:
- After publish success, immediately persist/transition the withdrawal state (
FinalizedWithdrawalTx,finalizedWithdrawalTxns,Withdrawing, DB update) before callinghandleWithdrawal. - Start
handleWithdrawalas 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:
- Stop fetching address params inside
handleWithdrawal; pass the requiredpkScriptfrom the caller. - The
pkScript(of the static address) can be calculated upfront, even before publishing, at the very beginning of FSM operation.
Proposed Changes
-
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. -
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).
-
Decouple state persistence from handleWithdrawal: Persist the
Withdrawingstate transition immediately after successful tx publish, so that restart recovery works even ifhandleWithdrawalsubsequently fails. -
Pass pkScript from caller: Remove
GetStaticAddressParameterscall fromhandleWithdrawaland pass thepkScriptin from the caller, where it can be computed once upfront.
References
- File:
staticaddr/withdraw/manager.go, around thehandleWithdrawalfunction - PR: staticaddr: various fixes #1078