diff --git a/staticaddr/withdraw/manager.go b/staticaddr/withdraw/manager.go index 3a7927a45..c867b56a5 100644 --- a/staticaddr/withdraw/manager.go +++ b/staticaddr/withdraw/manager.go @@ -439,19 +439,37 @@ func (m *Manager) WithdrawDeposits(ctx context.Context, return "", "", err } - published, err := m.publishFinalizedWithdrawalTx(ctx, finalizedTx) + withdrawalPkScript, err := txscript.PayToAddrScript(withdrawalAddress) if err != nil { - return "", "", err + return "", "", fmt.Errorf("could not get withdrawal "+ + "pkscript: %w", err) } - if !published { - return "", "", nil + // Attach the finalized withdrawal transaction before transitioning the + // deposits. The state transition persists it, allowing recovery to + // republish the transaction if publishing blocks or the client restarts. + previousWithdrawalTxns := make([]*wire.MsgTx, len(deposits)) + for i, d := range deposits { + d.Lock() + previousWithdrawalTxns[i] = d.FinalizedWithdrawalTx + d.FinalizedWithdrawalTx = finalizedTx + d.Unlock() } - withdrawalPkScript, err := txscript.PayToAddrScript(withdrawalAddress) + // Transition before publishing so wallet reconciliation can't remove a + // spent deposit from the active set while publication is in progress. + err = m.cfg.DepositManager.TransitionDeposits( + ctx, deposits, deposit.OnWithdrawInitiated, deposit.Withdrawing, + ) if err != nil { - return "", "", fmt.Errorf("could not get withdrawal "+ - "pkscript: %w", err) + for i, d := range deposits { + d.Lock() + d.FinalizedWithdrawalTx = previousWithdrawalTxns[i] + d.Unlock() + } + + return "", "", fmt.Errorf("failed to transition deposits %w", + err) } // If this is the first time this cluster of deposits is withdrawn, we @@ -479,49 +497,43 @@ func (m *Manager) WithdrawDeposits(ctx context.Context, // If a previous withdrawal existed across the selected deposits, and // it isn't the same as the new withdrawal, we remove it from the // finalized withdrawals to stop republishing it on block arrivals. - deposits[0].Lock() - prevTx := deposits[0].FinalizedWithdrawalTx - deposits[0].Unlock() + previousWithdrawalTx := previousWithdrawalTxns[0] + if previousWithdrawalTx != nil && + previousWithdrawalTx.TxHash() != finalizedTx.TxHash() { - if prevTx != nil && prevTx.TxHash() != finalizedTx.TxHash() { m.mu.Lock() - delete(m.finalizedWithdrawalTxns, prevTx.TxHash()) + delete(m.finalizedWithdrawalTxns, previousWithdrawalTx.TxHash()) m.mu.Unlock() } - // Attach the finalized withdrawal tx to the deposits. After a client - // restart we can use this address as an indicator to republish the - // withdrawal tx and continue the withdrawal. - // Deposits with the same withdrawal tx are part of the same withdrawal. - for _, d := range deposits { - d.Lock() - d.FinalizedWithdrawalTx = finalizedTx - d.Unlock() - } - // Add the new withdrawal tx to the finalized withdrawals to republish // it on block arrivals. m.mu.Lock() m.finalizedWithdrawalTxns[finalizedTx.TxHash()] = finalizedTx m.mu.Unlock() - // Transition the deposits to the withdrawing state. If the user fee - // bumped a withdrawal this results in a NOOP transition. - err = m.cfg.DepositManager.TransitionDeposits( - ctx, deposits, deposit.OnWithdrawInitiated, deposit.Withdrawing, - ) + // A fee bump is a self-transition, which the deposit FSM doesn't + // persist. Explicitly store the replacement transaction in that case. + if allWithdrawing { + for _, d := range deposits { + err = m.cfg.DepositManager.UpdateDeposit(ctx, d) + if err != nil { + return "", "", fmt.Errorf("failed to update "+ + "deposit %w", err) + } + } + } + + // Publish only after the finalized transaction is durable and local + // tracking is set up. Keep that preparation on publication errors because + // the wallet RPC outcome can be ambiguous. + published, err := m.publishFinalizedWithdrawalTx(ctx, finalizedTx) if err != nil { - return "", "", fmt.Errorf("failed to transition deposits %w", - err) + return "", "", err } - // Update the deposits in the database. - for _, d := range deposits { - err = m.cfg.DepositManager.UpdateDeposit(ctx, d) - if err != nil { - return "", "", fmt.Errorf("failed to update "+ - "deposit %w", err) - } + if !published { + return "", "", nil } return finalizedTx.TxID(), withdrawalAddress.String(), nil