staticaddr: lock deposit reads while withdrawing

Author: hieblmi

staticaddr/withdraw/manager.go

@@ -294,7 +294,7 @@ func (m *Manager) recoverWithdrawals(ctx context.Context) error {
 	return nil
 }
 
-// WithdrawDeposits starts a deposits withdrawal flow. If the amount is set to 0
+// WithdrawDeposits starts a deposit withdrawal flow. If the amount is set to 0,
 // the full amount of the selected deposits will be withdrawn.
 func (m *Manager) WithdrawDeposits(ctx context.Context,
 	outpoints []wire.OutPoint, destAddr string, satPerVbyte int64,
@@ -309,6 +309,16 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 		deposits       []*deposit.Deposit
 		allDeposited   bool
 		allWithdrawing bool
+		lockDeposits   = func(deposits []*deposit.Deposit) {
+			for _, d := range deposits {
+				d.Lock()
+			}
+		}
+		unlockDeposits = func(deposits []*deposit.Deposit) {
+			for _, d := range deposits {
+				d.Unlock()
+			}
+		}
 	)
 
 	// Ensure that the deposits are in a state in which they can be
@@ -319,8 +329,8 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 
 	// If not all passed outpoints are in state Deposited, we'll check if
 	// they are all in state Withdrawing. If they are, then the user is
-	// requesting a fee bump, if not we'll return an error as we only allow
-	// fee bumping deposits in state Withdrawing.
+	// requesting a fee bump, if not, we'll return an error as we only allow
+	// fee-bumping deposits in state Withdrawing.
 	if !allDeposited {
 		deposits, allWithdrawing = m.cfg.DepositManager.AllOutpointsActiveDeposits(
 			outpoints, deposit.Withdrawing,
@@ -329,7 +339,16 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 		if !allWithdrawing {
 			return "", "", ErrWithdrawingMixedDeposits
 		}
+	}
+
+	// We lock the deposits for the duration of the withdrawal to avoid
+	// race conditions.
+	lockDeposits(deposits)
+	defer unlockDeposits(deposits)
 
+	// Perform some checks on the deposits of a withdrawal if it is a fee
+	// bump.
+	if !allDeposited {
 		// Ensure that all previously withdrawn deposits reference their
 		// finalized withdrawal tx.
 		for _, d := range deposits {
@@ -338,7 +357,7 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 			}
 		}
 
-		// If republishing of an existing withdrawal is requested we
+		// If republishing of an existing withdrawal is requested, we
 		// ensure that all deposits remain clustered in the context of
 		// the same withdrawal tx. We do this by checking that they have
 		// the same previous withdrawal tx hash. This ensures that the
@@ -351,7 +370,7 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 			}
 		}
 
-		// We also avoid that the user selects a subset of previously
+		// We also avoid it that the user selects a subset of previously
 		// clustered deposits for a fee bump. This would result in a
 		// different transaction shape.
 		outpointMap := make(map[wire.OutPoint]struct{})
@@ -441,7 +460,7 @@ 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
+	// it is different from the new withdrawal, we remove it from the
 	// finalized withdrawals to stop republishing it on block arrivals.
 	deposits[0].Lock()
 	prevTx := deposits[0].FinalizedWithdrawalTx
@@ -454,7 +473,7 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 	}
 
 	// Attach the finalized withdrawal tx to the deposits. After a client
-	// restart we can use this address as an indicator to republish the
+	//  restarts, 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 {
@@ -470,7 +489,7 @@ func (m *Manager) WithdrawDeposits(ctx context.Context,
 	m.mu.Unlock()
 
 	// Transition the deposits to the withdrawing state. If the user fee
-	// bumped a withdrawal this results in a NOOP transition.
+	// bumped a withdrawal, this results in a NOOP transition.
 	err = m.cfg.DepositManager.TransitionDeposits(
 		ctx, deposits, deposit.OnWithdrawInitiated, deposit.Withdrawing,
 	)