htlcswitch/switch_control: expand godocs and add ErrUnknownPaymentStatus

htlcswitch/switch_control.go

@@ -9,36 +9,53 @@ import (
 )
 
 var (
-	// ErrAlreadyPaid is used when we have already paid
-	ErrAlreadyPaid = errors.New("invoice was already paid")
+	// ErrAlreadyPaid signals we have already paid this payment hash.
+	ErrAlreadyPaid = errors.New("invoice is already paid")
 
-	// ErrPaymentInFlight returns in case if payment is already "in flight"
+	// ErrPaymentInFlight signals that payment for this payment hash is
+	// already "in flight" on the network.
 	ErrPaymentInFlight = errors.New("payment is in transition")
 
-	// ErrPaymentNotInitiated returns in case if payment wasn't initiated
-	// in switch
+	// ErrPaymentNotInitiated is returned  if payment wasn't initiated in
+	// switch.
 	ErrPaymentNotInitiated = errors.New("payment isn't initiated")
 
-	// ErrPaymentAlreadyCompleted returns in case of attempt to complete
-	// completed payment
+	// ErrPaymentAlreadyCompleted is returned in the event we attempt to
+	// recomplete a completed payment.
 	ErrPaymentAlreadyCompleted = errors.New("payment is already completed")
+
+	// ErrUnknownPaymentStatus is returned when we do not recognize the
+	// existing state of a payment.
+	ErrUnknownPaymentStatus = errors.New("unknown payment status")
 )
 
-// ControlTower is a controller interface of sending HTLC messages to switch
+// ControlTower tracks all outgoing payments made by the switch, whose primary
+// purpose is to prevent duplicate payments to the same payment hash. In
+// production, a persistent implementation is preferred so that tracking can
+// survive across restarts. Payments are transition through various payment
+// states, and the ControlTower interface provides access to driving the state
+// transitions.
 type ControlTower interface {
-	// CheckSend intercepts incoming message to provide checks
-	// and fail if specific message is not allowed by implementation
-	CheckSend(htlc *lnwire.UpdateAddHTLC) error
+	// ClearForTakeoff atomically checks that no inflight or completed
+	// payments exist for this payment hash. If none are found, this method
+	// atomically transitions the status for this payment hash as InFlight.
+	ClearForTakeoff(htlc *lnwire.UpdateAddHTLC) error
 
-	// Success marks message transition as successful
+	// Success transitions an InFlight payment into a Completed payment.
+	// After invoking this method, ClearForTakeoff should always return an
+	// error to prevent us from making duplicate payments to the same
+	// payment hash.
 	Success(paymentHash [32]byte) error
 
-	// Fail marks message transition as failed
+	// Fail transitions an InFlight payment into a Grounded Payment. After
+	// invoking this method, ClearForTakeoff should return nil on its next
+	// call for this payment hash, allowing the switch to make a subsequent
+	// payment.
 	Fail(paymentHash [32]byte) error
 }
 
-// paymentControl is implementation of ControlTower to restrict double payment
-// sending.
+// paymentControl is persistent implementation of ControlTower to restrict
+// double payment sending.
 type paymentControl struct {
 	mx sync.Mutex
 
@@ -52,9 +69,9 @@ func NewPaymentControl(db *channeldb.DB) ControlTower {
 	}
 }
 
-// CheckSend checks that a sending htlc wasn't triggered before for specific
-// payment hash, if so, should trigger error depends on current status
-func (p *paymentControl) CheckSend(htlc *lnwire.UpdateAddHTLC) error {
+// ClearForTakeoff checks that we don't already have an InFlight or Completed
+// payment identified by the same payment hash.
+func (p *paymentControl) ClearForTakeoff(htlc *lnwire.UpdateAddHTLC) error {
 	p.mx.Lock()
 	defer p.mx.Unlock()
 
@@ -65,27 +82,31 @@ func (p *paymentControl) CheckSend(htlc *lnwire.UpdateAddHTLC) error {
 	}
 
 	switch paymentStatus {
+
 	case channeldb.StatusGrounded:
 		// It is safe to reattempt a payment if we know that we haven't
-		// left one in flight prior to restarting and switch.
-		return p.db.UpdatePaymentStatus(htlc.PaymentHash,
-			channeldb.StatusInFlight)
+		// left one in flight. Since this one is grounded, Transition
+		// the payment status to InFlight to prevent others.
+		return p.db.UpdatePaymentStatus(htlc.PaymentHash, channeldb.StatusInFlight)
 
 	case channeldb.StatusInFlight:
-		// Not clear if it's safe to reinitiate a payment if there
-		// is already a payment in flight, so we should withhold any
-		// additional attempts to send to that payment hash.
+		// We already have an InFlight payment on the network. We will
+		// disallow any more payment until a response is received.
 		return ErrPaymentInFlight
 
 	case channeldb.StatusCompleted:
-		// It has been already paid and don't want to pay again.
+		// We've already completed a payment to this payment hash,
+		// forbid the switch from sending another.
 		return ErrAlreadyPaid
-	}
 
-	return nil
+	default:
+		return ErrUnknownPaymentStatus
+	}
 }
 
-// Success proceed status changing of payment to next successful status
+// Success transitions an InFlight payment to Completed, otherwise it returns an
+// error. After calling Success, ClearForTakeoff should prevent any further
+// attempts for the same payment hash.
 func (p *paymentControl) Success(paymentHash [32]byte) error {
 	p.mx.Lock()
 	defer p.mx.Unlock()
@@ -97,22 +118,29 @@ func (p *paymentControl) Success(paymentHash [32]byte) error {
 
 	switch paymentStatus {
 	case channeldb.StatusGrounded:
-		// Payment isn't initiated but received.
+		// Our records show the payment as still being grounded, meaning
+		// it never should have left the switch.
 		return ErrPaymentNotInitiated
 
 	case channeldb.StatusInFlight:
-		// Successful transition from InFlight transition to Completed.
+		// A successful response was received for an InFlight payment,
+		// mark it as completed to prevent sending to this payment hash
+		// again.
 		return p.db.UpdatePaymentStatus(paymentHash, channeldb.StatusCompleted)
 
 	case channeldb.StatusCompleted:
-		// Payment is completed before in should be ignored.
+		// The payment was completed previously, alert the caller that
+		// this may be a duplicate call.
 		return ErrPaymentAlreadyCompleted
-	}
 
-	return nil
+	default:
+		return ErrUnknownPaymentStatus
+	}
 }
 
-// Fail proceed status changing of payment to initial status in case of failure
+// Fail transitions an InFlight payment to Grounded, otherwise it returns an
+// error. After calling Fail, ClearForTakeoff should fail any further attempts
+// for the same payment hash.
 func (p *paymentControl) Fail(paymentHash [32]byte) error {
 	p.mx.Lock()
 	defer p.mx.Unlock()
@@ -124,19 +152,22 @@ func (p *paymentControl) Fail(paymentHash [32]byte) error {
 
 	switch paymentStatus {
 	case channeldb.StatusGrounded:
-		// Unpredictable behavior when payment wasn't transited to
-		// StatusInFlight status and was failed.
+		// Our records show the payment as still being grounded, meaning
+		// it never should have left the switch.
 		return ErrPaymentNotInitiated
 
 	case channeldb.StatusInFlight:
-		// If payment wasn't processed by some reason should return to
-		// default status to unlock retrying option for the same payment hash.
+		// A failed response was received for an InFlight payment, mark
+		// it as Grounded again to allow subsequent attempts.
 		return p.db.UpdatePaymentStatus(paymentHash, channeldb.StatusGrounded)
 
 	case channeldb.StatusCompleted:
-		// Payment is completed before and can't be moved to another status.
+		// The payment was completed previously, and we are now
+		// reporting that it has failed. Leave the status as completed,
+		// but alert the user that something is wrong.
 		return ErrPaymentAlreadyCompleted
-	}
 
-	return nil
+	default:
+		return ErrUnknownPaymentStatus
+	}
 }