Refactor Payment PR 3

channeldb/db.go

@@ -352,7 +352,6 @@ type DB struct {
 	dbPath                    string
 	clock                     clock.Clock
 	dryRun                    bool
-	keepFailedPaymentAttempts bool
 	storeFinalHtlcResolutions bool
 
 	// noRevLogAmtData if true, means that commitment transaction amount
@@ -413,7 +412,6 @@ func CreateWithBackend(backend kvdb.Backend, modifiers ...OptionModifier) (*DB,
 		},
 		clock:                     opts.clock,
 		dryRun:                    opts.dryRun,
-		keepFailedPaymentAttempts: opts.keepFailedPaymentAttempts,
 		storeFinalHtlcResolutions: opts.storeFinalHtlcResolutions,
 		noRevLogAmtData:           opts.NoRevLogAmtData,
 	}

channeldb/duplicate_payments.go

@@ -11,6 +11,7 @@ import (
 	"github.com/lightningnetwork/lnd/kvdb"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
+	paymentsdb "github.com/lightningnetwork/lnd/payments/db"
 	"github.com/lightningnetwork/lnd/routing/route"
 )
 
@@ -74,7 +75,7 @@ func fetchDuplicatePaymentStatus(bucket kvdb.RBucket) (PaymentStatus, error) {
 		return StatusInFlight, nil
 	}
 
-	return 0, ErrPaymentNotInitiated
+	return 0, paymentsdb.ErrPaymentNotInitiated
 }
 
 func deserializeDuplicateHTLCAttemptInfo(r io.Reader) (

channeldb/mp_payment.go

@@ -15,6 +15,7 @@ import (
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnutils"
 	"github.com/lightningnetwork/lnd/lnwire"
+	paymentsdb "github.com/lightningnetwork/lnd/payments/db"
 	"github.com/lightningnetwork/lnd/routing/route"
 )
 
@@ -349,12 +350,12 @@ func (m *MPPayment) Registrable() error {
 	// are settled HTLCs or the payment is failed. If we already have
 	// settled HTLCs, we won't allow adding more HTLCs.
 	if m.State.HasSettledHTLC {
-		return ErrPaymentPendingSettled
+		return paymentsdb.ErrPaymentPendingSettled
 	}
 
 	// If the payment is already failed, we won't allow adding more HTLCs.
 	if m.State.PaymentFailed {
-		return ErrPaymentPendingFailed
+		return paymentsdb.ErrPaymentPendingFailed
 	}
 
 	// Otherwise we can add more HTLCs.
@@ -371,8 +372,8 @@ func (m *MPPayment) setState() error {
 	// Sanity check we haven't sent a value larger than the payment amount.
 	totalAmt := m.Info.Value
 	if sentAmt > totalAmt {
-		return fmt.Errorf("%w: sent=%v, total=%v", ErrSentExceedsTotal,
-			sentAmt, totalAmt)
+		return fmt.Errorf("%w: sent=%v, total=%v",
+			paymentsdb.ErrSentExceedsTotal, sentAmt, totalAmt)
 	}
 
 	// Get any terminal info for this payment.
@@ -451,7 +452,7 @@ func (m *MPPayment) NeedWaitAttempts() (bool, error) {
 		case StatusSucceeded:
 			return false, fmt.Errorf("%w: parts of the payment "+
 				"already succeeded but still have remaining "+
-				"amount %v", ErrPaymentInternal,
+				"amount %v", paymentsdb.ErrPaymentInternal,
 				m.State.RemainingAmt)
 
 		// The payment is failed and we have no inflight HTLCs, no need
@@ -462,7 +463,7 @@ func (m *MPPayment) NeedWaitAttempts() (bool, error) {
 		// Unknown payment status.
 		default:
 			return false, fmt.Errorf("%w: %s",
-				ErrUnknownPaymentStatus, m.Status)
+				paymentsdb.ErrUnknownPaymentStatus, m.Status)
 		}
 	}
 
@@ -472,7 +473,8 @@ func (m *MPPayment) NeedWaitAttempts() (bool, error) {
 	// When the payment is newly created, yet the payment has no remaining
 	// amount, return an error.
 	case StatusInitiated:
-		return false, fmt.Errorf("%w: %v", ErrPaymentInternal, m.Status)
+		return false, fmt.Errorf("%w: %v",
+			paymentsdb.ErrPaymentInternal, m.Status)
 
 	// If the payment is inflight, we must wait.
 	//
@@ -493,12 +495,13 @@ func (m *MPPayment) NeedWaitAttempts() (bool, error) {
 	// marked as failed with a reason, which means the remainingAmt must
 	// not be zero because our sentAmt is zero.
 	case StatusFailed:
-		return false, fmt.Errorf("%w: %v", ErrPaymentInternal, m.Status)
+		return false, fmt.Errorf("%w: %v",
+			paymentsdb.ErrPaymentInternal, m.Status)
 
 	// Unknown payment status.
 	default:
-		return false, fmt.Errorf("%w: %s", ErrUnknownPaymentStatus,
-			m.Status)
+		return false, fmt.Errorf("%w: %s",
+			paymentsdb.ErrUnknownPaymentStatus, m.Status)
 	}
 }
 
@@ -528,7 +531,8 @@ func (m *MPPayment) AllowMoreAttempts() (bool, error) {
 		// remainingAmt, return an error.
 		if m.Status == StatusInitiated {
 			return false, fmt.Errorf("%w: initiated payment has "+
-				"zero remainingAmt", ErrPaymentInternal)
+				"zero remainingAmt",
+				paymentsdb.ErrPaymentInternal)
 		}
 
 		// Otherwise, exit early since all other statuses with zero
@@ -544,8 +548,8 @@ func (m *MPPayment) AllowMoreAttempts() (bool, error) {
 	// as the preimage is received. In this case, return an error state.
 	if m.Status == StatusSucceeded {
 		return false, fmt.Errorf("%w: payment already succeeded but "+
-			"still have remaining amount %v", ErrPaymentInternal,
-			m.State.RemainingAmt)
+			"still have remaining amount %v",
+			paymentsdb.ErrPaymentInternal, m.State.RemainingAmt)
 	}
 
 	// Now check if we can register a new HTLC.

channeldb/mp_payment_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/btcsuite/btcd/btcec/v2"
 	"github.com/lightningnetwork/lnd/lntypes"
 	"github.com/lightningnetwork/lnd/lnwire"
+	paymentsdb "github.com/lightningnetwork/lnd/payments/db"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/stretchr/testify/require"
 )
@@ -65,35 +66,35 @@ func TestRegistrable(t *testing.T) {
 			// Test inflight status with settled HTLC but no failed
 			// payment.
 			status:         StatusInFlight,
-			registryErr:    ErrPaymentPendingSettled,
+			registryErr:    paymentsdb.ErrPaymentPendingSettled,
 			hasSettledHTLC: true,
 		},
 		{
 			// Test inflight status with no settled HTLC but failed
 			// payment.
 			status:        StatusInFlight,
-			registryErr:   ErrPaymentPendingFailed,
+			registryErr:   paymentsdb.ErrPaymentPendingFailed,
 			paymentFailed: true,
 		},
 		{
 			// Test error state with settled HTLC and failed
 			// payment.
 			status:         0,
-			registryErr:    ErrUnknownPaymentStatus,
+			registryErr:    paymentsdb.ErrUnknownPaymentStatus,
 			hasSettledHTLC: true,
 			paymentFailed:  true,
 		},
 		{
 			status:      StatusSucceeded,
-			registryErr: ErrPaymentAlreadySucceeded,
+			registryErr: paymentsdb.ErrPaymentAlreadySucceeded,
 		},
 		{
 			status:      StatusFailed,
-			registryErr: ErrPaymentAlreadyFailed,
+			registryErr: paymentsdb.ErrPaymentAlreadyFailed,
 		},
 		{
 			status:      0,
-			registryErr: ErrUnknownPaymentStatus,
+			registryErr: paymentsdb.ErrUnknownPaymentStatus,
 		},
 	}
 
@@ -149,7 +150,7 @@ func TestPaymentSetState(t *testing.T) {
 				},
 			},
 			totalAmt:    1,
-			errExpected: ErrSentExceedsTotal,
+			errExpected: paymentsdb.ErrSentExceedsTotal,
 		},
 		{
 			// Test that when the htlc is failed, the fee is not
@@ -294,7 +295,7 @@ func TestNeedWaitAttempts(t *testing.T) {
 			status:       StatusSucceeded,
 			remainingAmt: 1000,
 			needWait:     false,
-			expectedErr:  ErrPaymentInternal,
+			expectedErr:  paymentsdb.ErrPaymentInternal,
 		},
 		{
 			// Payment is in terminal state, no need to wait.
@@ -309,7 +310,7 @@ func TestNeedWaitAttempts(t *testing.T) {
 			status:       StatusInitiated,
 			remainingAmt: 0,
 			needWait:     false,
-			expectedErr:  ErrPaymentInternal,
+			expectedErr:  paymentsdb.ErrPaymentInternal,
 		},
 		{
 			// With zero remainingAmt we must wait for the results.
@@ -330,21 +331,21 @@ func TestNeedWaitAttempts(t *testing.T) {
 			status:       StatusFailed,
 			remainingAmt: 0,
 			needWait:     false,
-			expectedErr:  ErrPaymentInternal,
+			expectedErr:  paymentsdb.ErrPaymentInternal,
 		},
 		{
 			// Payment is in an unknown status, return an error.
 			status:       0,
 			remainingAmt: 0,
 			needWait:     false,
-			expectedErr:  ErrUnknownPaymentStatus,
+			expectedErr:  paymentsdb.ErrUnknownPaymentStatus,
 		},
 		{
 			// Payment is in an unknown status, return an error.
 			status:       0,
 			remainingAmt: 1000,
 			needWait:     false,
-			expectedErr:  ErrUnknownPaymentStatus,
+			expectedErr:  paymentsdb.ErrUnknownPaymentStatus,
 		},
 	}
 
@@ -397,7 +398,7 @@ func TestAllowMoreAttempts(t *testing.T) {
 			status:       StatusInitiated,
 			remainingAmt: 0,
 			allowMore:    false,
-			expectedErr:  ErrPaymentInternal,
+			expectedErr:  paymentsdb.ErrPaymentInternal,
 		},
 		{
 			// With zero remainingAmt we don't allow more HTLC
@@ -505,7 +506,7 @@ func TestAllowMoreAttempts(t *testing.T) {
 			remainingAmt:   1000,
 			hasSettledHTLC: true,
 			allowMore:      false,
-			expectedErr:    ErrPaymentInternal,
+			expectedErr:    paymentsdb.ErrPaymentInternal,
 		},
 		{
 			// With the payment failed with no inflight HTLCs, we

channeldb/options.go

@@ -68,10 +68,6 @@ type Options struct {
 	// database if set to true.
 	dryRun bool
 
-	// keepFailedPaymentAttempts determines whether failed htlc attempts
-	// are kept on disk or removed to save space.
-	keepFailedPaymentAttempts bool
-
 	// storeFinalHtlcResolutions determines whether to persistently store
 	// the final resolution of incoming htlcs.
 	storeFinalHtlcResolutions bool
@@ -120,14 +116,6 @@ func OptionDryRunMigration(dryRun bool) OptionModifier {
 	}
 }
 
-// OptionKeepFailedPaymentAttempts controls whether failed payment attempts are
-// kept on disk after a payment settles.
-func OptionKeepFailedPaymentAttempts(keepFailedPaymentAttempts bool) OptionModifier {
-	return func(o *Options) {
-		o.keepFailedPaymentAttempts = keepFailedPaymentAttempts
-	}
-}
-
 // OptionStoreFinalHtlcResolutions controls whether to persistently store the
 // final resolution of incoming htlcs.
 func OptionStoreFinalHtlcResolutions(

channeldb/payment_status.go

@@ -1,6 +1,10 @@
 package channeldb
 
-import "fmt"
+import (
+	"fmt"
+
+	paymentsdb "github.com/lightningnetwork/lnd/payments/db"
+)
 
 // PaymentStatus represent current status of payment.
 type PaymentStatus byte
@@ -58,24 +62,25 @@ func (ps PaymentStatus) initializable() error {
 	// again in case other goroutines have already been creating HTLCs for
 	// it.
 	case StatusInitiated:
-		return ErrPaymentExists
+		return paymentsdb.ErrPaymentExists
 
 	// We already have an InFlight payment on the network. We will disallow
 	// any new payments.
 	case StatusInFlight:
-		return ErrPaymentInFlight
+		return paymentsdb.ErrPaymentInFlight
 
 	// The payment has been attempted and is succeeded so we won't allow
 	// creating it again.
 	case StatusSucceeded:
-		return ErrAlreadyPaid
+		return paymentsdb.ErrAlreadyPaid
 
 	// We allow retrying failed payments.
 	case StatusFailed:
 		return nil
 
 	default:
-		return fmt.Errorf("%w: %v", ErrUnknownPaymentStatus, ps)
+		return fmt.Errorf("%w: %v", paymentsdb.ErrUnknownPaymentStatus,
+			ps)
 	}
 }
 
@@ -91,7 +96,7 @@ func (ps PaymentStatus) removable() error {
 	// There are still inflight HTLCs and the payment needs to wait for the
 	// final outcomes.
 	case StatusInFlight:
-		return ErrPaymentInFlight
+		return paymentsdb.ErrPaymentInFlight
 
 	// The payment has been attempted and is succeeded and is allowed to be
 	// removed.
@@ -103,7 +108,8 @@ func (ps PaymentStatus) removable() error {
 		return nil
 
 	default:
-		return fmt.Errorf("%w: %v", ErrUnknownPaymentStatus, ps)
+		return fmt.Errorf("%w: %v", paymentsdb.ErrUnknownPaymentStatus,
+			ps)
 	}
 }
 
@@ -121,13 +127,14 @@ func (ps PaymentStatus) updatable() error {
 
 	// If the payment has a terminal condition, we won't allow any updates.
 	case StatusSucceeded:
-		return ErrPaymentAlreadySucceeded
+		return paymentsdb.ErrPaymentAlreadySucceeded
 
 	case StatusFailed:
-		return ErrPaymentAlreadyFailed
+		return paymentsdb.ErrPaymentAlreadyFailed
 
 	default:
-		return fmt.Errorf("%w: %v", ErrUnknownPaymentStatus, ps)
+		return fmt.Errorf("%w: %v", paymentsdb.ErrUnknownPaymentStatus,
+			ps)
 	}
 }