channeldb: return error when payment is not initialized

This commit changes `fetchPaymentStatus` to return an error when the
payment creation info bucket cannot be found. Instead of using the
ambiguous status `StatusUnknown`, we tell the callsite explictly that
there's no such payment. This also means the vague `StatusUnknown` can
now be removed as it has multiple meanings.

channeldb/payment_control.go

@@ -141,36 +141,40 @@ func (p *PaymentControl) InitPayment(paymentHash lntypes.Hash,
 
 		// Get the existing status of this payment, if any.
 		paymentStatus, err := fetchPaymentStatus(bucket)
-		if err != nil {
+
+		switch {
+		// If no error is returned, it means we already have this
+		// payment. We'll check the status to decide whether we allow
+		// retrying the payment or return a specific error.
+		case err == nil:
+			switch paymentStatus {
+
+			// We allow retrying failed payments.
+			case StatusFailed:
+
+			// We already have an InFlight payment on the network.
+			// We will disallow any new payments.
+			case StatusInFlight:
+				updateErr = ErrPaymentInFlight
+				return nil
+
+			// We've already succeeded a payment to this payment
+			// hash, forbid the switch from sending another.
+			case StatusSucceeded:
+				updateErr = ErrAlreadyPaid
+				return nil
+
+			default:
+				updateErr = ErrUnknownPaymentStatus
+				return nil
+			}
+
+		// Otherwise, if the error is not `ErrPaymentNotInitiated`,
+		// we'll return the error.
+		case !errors.Is(err, ErrPaymentNotInitiated):
 			return err
 		}
 
-		switch paymentStatus {
-
-		// We allow retrying failed payments.
-		case StatusFailed:
-
-		// This is a new payment that is being initialized for the
-		// first time.
-		case StatusUnknown:
-
-		// We already have an InFlight payment on the network. We will
-		// disallow any new payments.
-		case StatusInFlight:
-			updateErr = ErrPaymentInFlight
-			return nil
-
-		// We've already succeeded a payment to this payment hash,
-		// forbid the switch from sending another.
-		case StatusSucceeded:
-			updateErr = ErrAlreadyPaid
-			return nil
-
-		default:
-			updateErr = ErrUnknownPaymentStatus
-			return nil
-		}
-
 		// Before we set our new sequence number, we check whether this
 		// payment has a previously set sequence number and remove its
 		// index entry if it exists. This happens in the case where we
@@ -530,14 +534,12 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash,
 		// lets the last attempt to fail with a terminal write its
 		// failure to the PaymentControl without synchronizing with
 		// other attempts.
-		paymentStatus, err := fetchPaymentStatus(bucket)
-		if err != nil {
-			return err
-		}
-
-		if paymentStatus == StatusUnknown {
+		_, err = fetchPaymentStatus(bucket)
+		if errors.Is(err, ErrPaymentNotInitiated) {
 			updateErr = ErrPaymentNotInitiated
 			return nil
+		} else if err != nil {
+			return err
 		}
 
 		// Put the failure reason in the bucket for record keeping.
@@ -702,12 +704,12 @@ func (p *PaymentControl) nextPaymentSequence() ([]byte, error) {
 }
 
 // fetchPaymentStatus fetches the payment status of the payment. If the payment
-// isn't found, it will default to "StatusUnknown".
+// isn't found, it will return error `ErrPaymentNotInitiated`.
 func fetchPaymentStatus(bucket kvdb.RBucket) (PaymentStatus, error) {
 	// Creation info should be set for all payments, regardless of state.
 	// If not, it is unknown.
 	if bucket.Get(paymentCreationInfoKey) == nil {
-		return StatusUnknown, nil
+		return 0, ErrPaymentNotInitiated
 	}
 
 	payment, err := fetchPayment(bucket)
@@ -730,11 +732,6 @@ func ensureInFlight(payment *MPPayment) error {
 	case paymentStatus == StatusInFlight:
 		return nil
 
-	// Our records show the payment as unknown, meaning it never
-	// should have left the switch.
-	case paymentStatus == StatusUnknown:
-		return ErrPaymentNotInitiated
-
 	// The payment succeeded previously.
 	case paymentStatus == StatusSucceeded:
 		return ErrPaymentAlreadySucceeded

channeldb/payment_control_test.go

@@ -273,8 +273,6 @@ func TestPaymentControlSuccessesWithoutInFlight(t *testing.T) {
 	if err != ErrPaymentNotInitiated {
 		t.Fatalf("expected ErrPaymentNotInitiated, got %v", err)
 	}
-
-	assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusUnknown)
 }
 
 // TestPaymentControlFailsWithoutInFlight checks that a strict payment
@@ -295,8 +293,6 @@ func TestPaymentControlFailsWithoutInFlight(t *testing.T) {
 	if err != ErrPaymentNotInitiated {
 		t.Fatalf("expected ErrPaymentNotInitiated, got %v", err)
 	}
-
-	assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusUnknown)
 }
 
 // TestPaymentControlDeleteNonInFlight checks that calling DeletePayments only
@@ -1085,7 +1081,7 @@ func assertPaymentStatus(t *testing.T, p *PaymentControl,
 	t.Helper()
 
 	payment, err := p.FetchPayment(hash)
-	if expStatus == StatusUnknown && err == ErrPaymentNotInitiated {
+	if err == ErrPaymentNotInitiated {
 		return
 	}
 	if err != nil {

channeldb/payment_status.go

@@ -4,9 +4,8 @@ package channeldb
 type PaymentStatus byte
 
 const (
-	// StatusUnknown is the status where a payment has never been initiated
-	// and hence is unknown.
-	StatusUnknown PaymentStatus = 0
+	// NOTE: PaymentStatus = 0 was previously used for status unknown and
+	// is now deprecated.
 
 	// StatusInFlight is the status where a payment has been initiated, but
 	// a response has not been received.
@@ -24,9 +23,6 @@ const (
 // String returns readable representation of payment status.
 func (ps PaymentStatus) String() string {
 	switch ps {
-	case StatusUnknown:
-		return "Unknown"
-
 	case StatusInFlight:
 		return "In Flight"
 

lnrpc/routerrpc/router_backend.go

@@ -1370,9 +1370,6 @@ func convertPaymentStatus(dbStatus channeldb.PaymentStatus) (
 	lnrpc.Payment_PaymentStatus, error) {
 
 	switch dbStatus {
-	case channeldb.StatusUnknown:
-		return lnrpc.Payment_UNKNOWN, nil
-
 	case channeldb.StatusInFlight:
 		return lnrpc.Payment_IN_FLIGHT, nil