Merge pull request #3970 from halseth/amp-router-mvp-2020

MPP: Enable MultiPathPayments for payment lifecycle

channeldb/mp_payment.go

@@ -131,6 +131,52 @@ type MPPayment struct {
 	Status PaymentStatus
 }
 
+// TerminalInfo returns any HTLC settle info recorded. If no settle info is
+// recorded, any payment level failure will be returned. If neither a settle
+// nor a failure is recorded, both return values will be nil.
+func (m *MPPayment) TerminalInfo() (*HTLCSettleInfo, *FailureReason) {
+	for _, h := range m.HTLCs {
+		if h.Settle != nil {
+			return h.Settle, nil
+		}
+	}
+
+	return nil, m.FailureReason
+}
+
+// SentAmt returns the sum of sent amount and fees for HTLCs that are either
+// settled or still in flight.
+func (m *MPPayment) SentAmt() (lnwire.MilliSatoshi, lnwire.MilliSatoshi) {
+	var sent, fees lnwire.MilliSatoshi
+	for _, h := range m.HTLCs {
+		if h.Failure != nil {
+			continue
+		}
+
+		// The attempt was not failed, meaning the amount was
+		// potentially sent to the receiver.
+		sent += h.Route.ReceiverAmt()
+		fees += h.Route.TotalFees()
+	}
+
+	return sent, fees
+}
+
+// InFlightHTLCs returns the HTLCs that are still in-flight, meaning they have
+// not been settled or failed.
+func (m *MPPayment) InFlightHTLCs() []HTLCAttempt {
+	var inflights []HTLCAttempt
+	for _, h := range m.HTLCs {
+		if h.Settle != nil || h.Failure != nil {
+			continue
+		}
+
+		inflights = append(inflights, h)
+	}
+
+	return inflights
+}
+
 // serializeHTLCSettleInfo serializes the details of a settled htlc.
 func serializeHTLCSettleInfo(w io.Writer, s *HTLCSettleInfo) error {
 	if _, err := w.Write(s.Preimage[:]); err != nil {

channeldb/payment_control.go

@@ -18,22 +18,59 @@ var (
 	// already "in flight" on the network.
 	ErrPaymentInFlight = errors.New("payment is in transition")
 
-	// ErrPaymentNotInitiated is returned  if payment wasn't initiated in
-	// switch.
+	// ErrPaymentNotInitiated is returned if the payment wasn't initiated.
 	ErrPaymentNotInitiated = errors.New("payment isn't initiated")
 
 	// ErrPaymentAlreadySucceeded is returned in the event we attempt to
 	// change the status of a payment already succeeded.
 	ErrPaymentAlreadySucceeded = errors.New("payment is already succeeded")
 
-	// ErrPaymentAlreadyFailed is returned in the event we attempt to
-	// re-fail a failed payment.
+	// ErrPaymentAlreadyFailed is returned in the event we attempt to alter
+	// a failed payment.
 	ErrPaymentAlreadyFailed = errors.New("payment has already failed")
 
 	// ErrUnknownPaymentStatus is returned when we do not recognize the
 	// existing state of a payment.
 	ErrUnknownPaymentStatus = errors.New("unknown payment status")
 
+	// ErrPaymentTerminal is returned if we attempt to alter a payment that
+	// already has reached a terminal condition.
+	ErrPaymentTerminal = errors.New("payment has reached terminal condition")
+
+	// ErrAttemptAlreadySettled is returned if we try to alter an already
+	// settled HTLC attempt.
+	ErrAttemptAlreadySettled = errors.New("attempt already settled")
+
+	// ErrAttemptAlreadyFailed is returned if we try to alter an already
+	// failed HTLC attempt.
+	ErrAttemptAlreadyFailed = errors.New("attempt already failed")
+
+	// ErrValueMismatch is returned if we try to register a non-MPP attempt
+	// with an amount that doesn't match the payment amount.
+	ErrValueMismatch = errors.New("attempted value doesn't match payment" +
+		"amount")
+
+	// ErrValueExceedsAmt is returned if we try to register an attempt that
+	// would take the total sent amount above the payment amount.
+	ErrValueExceedsAmt = errors.New("attempted value exceeds payment" +
+		"amount")
+
+	// ErrNonMPPayment is returned if we try to register an MPP attempt for
+	// a payment that already has a non-MPP attempt regitered.
+	ErrNonMPPayment = errors.New("payment has non-MPP attempts")
+
+	// ErrMPPayment is returned if we try to register a non-MPP attempt for
+	// a payment that already has an MPP attempt regitered.
+	ErrMPPayment = errors.New("payment has MPP attempts")
+
+	// ErrMPPPaymentAddrMismatch is returned if we try to register an MPP
+	// shard where the payment address doesn't match existing shards.
+	ErrMPPPaymentAddrMismatch = errors.New("payment address mismatch")
+
+	// ErrMPPTotalAmountMismatch is returned if we try to register an MPP
+	// shard where the total amount doesn't match existing shards.
+	ErrMPPTotalAmountMismatch = errors.New("mp payment total amount mismatch")
+
 	// errNoAttemptInfo is returned when no attempt info is stored yet.
 	errNoAttemptInfo = errors.New("unable to find attempt info for " +
 		"inflight payment")
@@ -52,7 +89,7 @@ func NewPaymentControl(db *DB) *PaymentControl {
 }
 
 // InitPayment checks or records the given PaymentCreationInfo with the DB,
-// making sure it does not already exist as an in-flight payment. Then this
+// making sure it does not already exist as an in-flight payment. When this
 // method returns successfully, the payment is guranteeed to be in the InFlight
 // state.
 func (p *PaymentControl) InitPayment(paymentHash lntypes.Hash,
@@ -168,12 +205,69 @@ func (p *PaymentControl) RegisterAttempt(paymentHash lntypes.Hash,
 			return err
 		}
 
-		// We can only register attempts for payments that are
-		// in-flight.
-		if err := ensureInFlight(bucket); err != nil {
+		payment, err := fetchPayment(bucket)
+		if err != nil {
 			return err
 		}
 
+		// Ensure the payment is in-flight.
+		if err := ensureInFlight(payment); err != nil {
+			return err
+		}
+
+		// We cannot register a new attempt if the payment already has
+		// reached a terminal condition:
+		settle, fail := payment.TerminalInfo()
+		if settle != nil || fail != nil {
+			return ErrPaymentTerminal
+		}
+
+		// Make sure any existing shards match the new one with regards
+		// to MPP options.
+		mpp := attempt.Route.FinalHop().MPP
+		for _, h := range payment.InFlightHTLCs() {
+			hMpp := h.Route.FinalHop().MPP
+
+			switch {
+
+			// We tried to register a non-MPP attempt for a MPP
+			// payment.
+			case mpp == nil && hMpp != nil:
+				return ErrMPPayment
+
+			// We tried to register a MPP shard for a non-MPP
+			// payment.
+			case mpp != nil && hMpp == nil:
+				return ErrNonMPPayment
+
+			// Non-MPP payment, nothing more to validate.
+			case mpp == nil:
+				continue
+			}
+
+			// Check that MPP options match.
+			if mpp.PaymentAddr() != hMpp.PaymentAddr() {
+				return ErrMPPPaymentAddrMismatch
+			}
+
+			if mpp.TotalMsat() != hMpp.TotalMsat() {
+				return ErrMPPTotalAmountMismatch
+			}
+		}
+
+		// If this is a non-MPP attempt, it must match the total amount
+		// exactly.
+		amt := attempt.Route.ReceiverAmt()
+		if mpp == nil && amt != payment.Info.Value {
+			return ErrValueMismatch
+		}
+
+		// Ensure we aren't sending more than the total payment amount.
+		sentAmt, _ := payment.SentAmt()
+		if sentAmt+amt > payment.Info.Value {
+			return ErrValueExceedsAmt
+		}
+
 		htlcsBucket, err := bucket.CreateBucketIfNotExists(
 			paymentHtlcsBucket,
 		)
@@ -241,8 +335,15 @@ func (p *PaymentControl) updateHtlcKey(paymentHash lntypes.Hash,
 			return err
 		}
 
-		// We can only update keys of in-flight payments.
-		if err := ensureInFlight(bucket); err != nil {
+		p, err := fetchPayment(bucket)
+		if err != nil {
+			return err
+		}
+
+		// We can only update keys of in-flight payments. We allow
+		// updating keys even if the payment has reached a terminal
+		// condition, since the HTLC outcomes must still be updated.
+		if err := ensureInFlight(p); err != nil {
 			return err
 		}
 
@@ -257,6 +358,15 @@ func (p *PaymentControl) updateHtlcKey(paymentHash lntypes.Hash,
 				attemptID)
 		}
 
+		// Make sure the shard is not already failed or settled.
+		if htlcBucket.Get(htlcFailInfoKey) != nil {
+			return ErrAttemptAlreadyFailed
+		}
+
+		if htlcBucket.Get(htlcSettleInfoKey) != nil {
+			return ErrAttemptAlreadySettled
+		}
+
 		// Add or update the key for this htlc.
 		err = htlcBucket.Put(key, value)
 		if err != nil {
@@ -299,9 +409,17 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash,
 			return err
 		}
 
-		// We can only mark in-flight payments as failed.
-		if err := ensureInFlight(bucket); err != nil {
-			updateErr = err
+		// We mark the payent as failed as long as it is known. This
+		// 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 {
+			updateErr = ErrPaymentNotInitiated
 			return nil
 		}
 
@@ -318,14 +436,6 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash,
 			return err
 		}
 
-		// Final sanity check to see if there are no in-flight htlcs.
-		for _, htlc := range payment.HTLCs {
-			if htlc.Settle == nil && htlc.Failure == nil {
-				return errors.New("payment failed with " +
-					"in-flight htlc(s)")
-			}
-		}
-
 		return nil
 	})
 	if err != nil {
@@ -428,45 +538,29 @@ func nextPaymentSequence(tx kvdb.RwTx) ([]byte, error) {
 // fetchPaymentStatus fetches the payment status of the payment. If the payment
 // isn't found, it will default to "StatusUnknown".
 func fetchPaymentStatus(bucket kvdb.ReadBucket) (PaymentStatus, error) {
-	htlcsBucket := bucket.NestedReadBucket(paymentHtlcsBucket)
-	if htlcsBucket != nil {
-		htlcs, err := fetchHtlcAttempts(htlcsBucket)
-		if err != nil {
-			return 0, err
-		}
-
-		// Go through all HTLCs, and return StatusSucceeded if any of
-		// them did succeed.
-		for _, h := range htlcs {
-			if h.Settle != nil {
-				return StatusSucceeded, nil
-			}
-		}
+	// Creation info should be set for all payments, regardless of state.
+	// If not, it is unknown.
+	if bucket.Get(paymentCreationInfoKey) == nil {
+		return StatusUnknown, nil
 	}
 
-	if bucket.Get(paymentFailInfoKey) != nil {
-		return StatusFailed, nil
+	payment, err := fetchPayment(bucket)
+	if err != nil {
+		return 0, err
 	}
 
-	if bucket.Get(paymentCreationInfoKey) != nil {
-		return StatusInFlight, nil
-	}
-
-	return StatusUnknown, nil
+	return payment.Status, nil
 }
 
 // ensureInFlight checks whether the payment found in the given bucket has
 // status InFlight, and returns an error otherwise. This should be used to
 // ensure we only mark in-flight payments as succeeded or failed.
-func ensureInFlight(bucket kvdb.ReadBucket) error {
-	paymentStatus, err := fetchPaymentStatus(bucket)
-	if err != nil {
-		return err
-	}
+func ensureInFlight(payment *MPPayment) error {
+	paymentStatus := payment.Status
 
 	switch {
 
-	// The payment was indeed InFlight, return.
+	// The payment was indeed InFlight.
 	case paymentStatus == StatusInFlight:
 		return nil
 
@@ -528,14 +622,7 @@ func (p *PaymentControl) FetchInFlightPayments() ([]*InFlightPayment, error) {
 			inFlight := &InFlightPayment{}
 
 			// Get the CreationInfo.
-			b := bucket.Get(paymentCreationInfoKey)
-			if b == nil {
-				return fmt.Errorf("unable to find creation " +
-					"info for inflight payment")
-			}
-
-			r := bytes.NewReader(b)
-			inFlight.Info, err = deserializePaymentCreationInfo(r)
+			inFlight.Info, err = fetchCreationInfo(bucket)
 			if err != nil {
 				return err
 			}

channeldb/payment_control_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/btcsuite/fastsha256"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/record"
 )
 
 func initDB() (*DB, error) {
@@ -48,14 +49,14 @@ func genInfo() (*PaymentCreationInfo, *HTLCAttemptInfo,
 	rhash := fastsha256.Sum256(preimage[:])
 	return &PaymentCreationInfo{
 			PaymentHash:    rhash,
-			Value:          1,
+			Value:          testRoute.ReceiverAmt(),
 			CreationTime:   time.Unix(time.Now().Unix(), 0),
 			PaymentRequest: []byte("hola"),
 		},
 		&HTLCAttemptInfo{
-			AttemptID:  1,
+			AttemptID:  0,
 			SessionKey: priv,
-			Route:      testRoute,
+			Route:      *testRoute.Copy(),
 		}, preimage, nil
 }
 
@@ -85,8 +86,7 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
-		nil,
+		t, pControl, info.PaymentHash, info, nil, nil,
 	)
 
 	// Fail the payment, which should moved it to Failed.
@@ -99,8 +99,7 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 	// Verify the status is indeed Failed.
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusFailed)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
-		&failReason,
+		t, pControl, info.PaymentHash, info, &failReason, nil,
 	)
 
 	// Sends the htlc again, which should succeed since the prior payment
@@ -112,44 +111,57 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
-		nil,
+		t, pControl, info.PaymentHash, info, nil, nil,
 	)
 
 	// Record a new attempt. In this test scenario, the attempt fails.
 	// However, this is not communicated to control tower in the current
 	// implementation. It only registers the initiation of the attempt.
-	attempt.AttemptID = 2
 	err = pControl.RegisterAttempt(info.PaymentHash, attempt)
 	if err != nil {
 		t.Fatalf("unable to register attempt: %v", err)
 	}
 
+	htlcReason := HTLCFailUnreadable
 	err = pControl.FailAttempt(
-		info.PaymentHash, 2, &HTLCFailInfo{
-			Reason: HTLCFailUnreadable,
+		info.PaymentHash, attempt.AttemptID,
+		&HTLCFailInfo{
+			Reason: htlcReason,
 		},
 	)
 	if err != nil {
 		t.Fatal(err)
 	}
+	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+
+	htlc := &htlcStatus{
+		HTLCAttemptInfo: attempt,
+		failure:         &htlcReason,
+	}
+
+	assertPaymentInfo(t, pControl, info.PaymentHash, info, nil, htlc)
 
 	// Record another attempt.
-	attempt.AttemptID = 3
+	attempt.AttemptID = 1
 	err = pControl.RegisterAttempt(info.PaymentHash, attempt)
 	if err != nil {
 		t.Fatalf("unable to send htlc message: %v", err)
 	}
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+
+	htlc = &htlcStatus{
+		HTLCAttemptInfo: attempt,
+	}
+
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, 0, attempt, lntypes.Preimage{},
-		nil,
+		t, pControl, info.PaymentHash, info, nil, htlc,
 	)
 
-	// Settle the attempt and verify that status was changed to StatusSucceeded.
+	// Settle the attempt and verify that status was changed to
+	// StatusSucceeded.
 	var payment *MPPayment
 	payment, err = pControl.SettleAttempt(
-		info.PaymentHash, 3,
+		info.PaymentHash, attempt.AttemptID,
 		&HTLCSettleInfo{
 			Preimage: preimg,
 		},
@@ -171,7 +183,11 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 	}
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusSucceeded)
-	assertPaymentInfo(t, pControl, info.PaymentHash, info, 1, attempt, preimg, nil)
+
+	htlc.settle = &preimg
+	assertPaymentInfo(
+		t, pControl, info.PaymentHash, info, nil, htlc,
+	)
 
 	// Attempt a final payment, which should now fail since the prior
 	// payment succeed.
@@ -207,8 +223,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
-		nil,
+		t, pControl, info.PaymentHash, info, nil, nil,
 	)
 
 	// Try to initiate double sending of htlc message with the same
@@ -226,9 +241,12 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 		t.Fatalf("unable to send htlc message: %v", err)
 	}
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+
+	htlc := &htlcStatus{
+		HTLCAttemptInfo: attempt,
+	}
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, 0, attempt, lntypes.Preimage{},
-		nil,
+		t, pControl, info.PaymentHash, info, nil, htlc,
 	)
 
 	// Sends base htlc message which initiate StatusInFlight.
@@ -240,7 +258,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 
 	// After settling, the error should be ErrAlreadyPaid.
 	_, err = pControl.SettleAttempt(
-		info.PaymentHash, 1,
+		info.PaymentHash, attempt.AttemptID,
 		&HTLCSettleInfo{
 			Preimage: preimg,
 		},
@@ -249,7 +267,9 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 		t.Fatalf("error shouldn't have been received, got: %v", err)
 	}
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusSucceeded)
-	assertPaymentInfo(t, pControl, info.PaymentHash, info, 0, attempt, preimg, nil)
+
+	htlc.settle = &preimg
+	assertPaymentInfo(t, pControl, info.PaymentHash, info, nil, htlc)
 
 	err = pControl.InitPayment(info.PaymentHash, info)
 	if err != ErrAlreadyPaid {
@@ -360,12 +380,17 @@ func TestPaymentControlDeleteNonInFligt(t *testing.T) {
 			t.Fatalf("unable to send htlc message: %v", err)
 		}
 
+		htlc := &htlcStatus{
+			HTLCAttemptInfo: attempt,
+		}
+
 		if p.failed {
 			// Fail the payment attempt.
+			htlcFailure := HTLCFailUnreadable
 			err := pControl.FailAttempt(
 				info.PaymentHash, attempt.AttemptID,
 				&HTLCFailInfo{
-					Reason: HTLCFailUnreadable,
+					Reason: htlcFailure,
 				},
 			)
 			if err != nil {
@@ -381,14 +406,16 @@ func TestPaymentControlDeleteNonInFligt(t *testing.T) {
 
 			// Verify the status is indeed Failed.
 			assertPaymentStatus(t, pControl, info.PaymentHash, StatusFailed)
+
+			htlc.failure = &htlcFailure
 			assertPaymentInfo(
-				t, pControl, info.PaymentHash, info, 0, attempt,
-				lntypes.Preimage{}, &failReason,
+				t, pControl, info.PaymentHash, info,
+				&failReason, htlc,
 			)
 		} else if p.success {
 			// Verifies that status was changed to StatusSucceeded.
 			_, err := pControl.SettleAttempt(
-				info.PaymentHash, 1,
+				info.PaymentHash, attempt.AttemptID,
 				&HTLCSettleInfo{
 					Preimage: preimg,
 				},
@@ -398,14 +425,15 @@ func TestPaymentControlDeleteNonInFligt(t *testing.T) {
 			}
 
 			assertPaymentStatus(t, pControl, info.PaymentHash, StatusSucceeded)
+
+			htlc.settle = &preimg
 			assertPaymentInfo(
-				t, pControl, info.PaymentHash, info, 0, attempt, preimg, nil,
+				t, pControl, info.PaymentHash, info, nil, htlc,
 			)
 		} else {
 			assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 			assertPaymentInfo(
-				t, pControl, info.PaymentHash, info, 0, attempt,
-				lntypes.Preimage{}, nil,
+				t, pControl, info.PaymentHash, info, nil, htlc,
 			)
 		}
 	}
@@ -431,6 +459,366 @@ func TestPaymentControlDeleteNonInFligt(t *testing.T) {
 	}
 }
 
+// TestPaymentControlMultiShard checks the ability of payment control to
+// have multiple in-flight HTLCs for a single payment.
+func TestPaymentControlMultiShard(t *testing.T) {
+	t.Parallel()
+
+	// We will register three HTLC attempts, and always fail the second
+	// one. We'll generate all combinations of settling/failing the first
+	// and third HTLC, and assert that the payment status end up as we
+	// expect.
+	type testCase struct {
+		settleFirst bool
+		settleLast  bool
+	}
+
+	var tests []testCase
+	for _, f := range []bool{true, false} {
+		for _, l := range []bool{true, false} {
+			tests = append(tests, testCase{f, l})
+		}
+	}
+
+	runSubTest := func(t *testing.T, test testCase) {
+		db, err := initDB()
+		if err != nil {
+			t.Fatalf("unable to init db: %v", err)
+		}
+
+		pControl := NewPaymentControl(db)
+
+		info, attempt, preimg, err := genInfo()
+		if err != nil {
+			t.Fatalf("unable to generate htlc message: %v", err)
+		}
+
+		// Init the payment, moving it to the StatusInFlight state.
+		err = pControl.InitPayment(info.PaymentHash, info)
+		if err != nil {
+			t.Fatalf("unable to send htlc message: %v", err)
+		}
+
+		assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+		assertPaymentInfo(
+			t, pControl, info.PaymentHash, info, nil, nil,
+		)
+
+		// Create three unique attempts we'll use for the test, and
+		// register them with the payment control. We set each
+		// attempts's value to one third of the payment amount, and
+		// populate the MPP options.
+		shardAmt := info.Value / 3
+		attempt.Route.FinalHop().AmtToForward = shardAmt
+		attempt.Route.FinalHop().MPP = record.NewMPP(
+			info.Value, [32]byte{1},
+		)
+
+		var attempts []*HTLCAttemptInfo
+		for i := uint64(0); i < 3; i++ {
+			a := *attempt
+			a.AttemptID = i
+			attempts = append(attempts, &a)
+
+			err = pControl.RegisterAttempt(info.PaymentHash, &a)
+			if err != nil {
+				t.Fatalf("unable to send htlc message: %v", err)
+			}
+			assertPaymentStatus(
+				t, pControl, info.PaymentHash, StatusInFlight,
+			)
+
+			htlc := &htlcStatus{
+				HTLCAttemptInfo: &a,
+			}
+			assertPaymentInfo(
+				t, pControl, info.PaymentHash, info, nil, htlc,
+			)
+		}
+
+		// For a fourth attempt, check that attempting to
+		// register it will fail since the total sent amount
+		// will be too large.
+		b := *attempt
+		b.AttemptID = 3
+		err = pControl.RegisterAttempt(info.PaymentHash, &b)
+		if err != ErrValueExceedsAmt {
+			t.Fatalf("expected ErrValueExceedsAmt, got: %v",
+				err)
+		}
+
+		// Fail the second attempt.
+		a := attempts[1]
+		htlcFail := HTLCFailUnreadable
+		err = pControl.FailAttempt(
+			info.PaymentHash, a.AttemptID,
+			&HTLCFailInfo{
+				Reason: htlcFail,
+			},
+		)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		htlc := &htlcStatus{
+			HTLCAttemptInfo: a,
+			failure:         &htlcFail,
+		}
+		assertPaymentInfo(
+			t, pControl, info.PaymentHash, info, nil, htlc,
+		)
+
+		// Payment should still be in-flight.
+		assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+
+		// Depending on the test case, settle or fail the first attempt.
+		a = attempts[0]
+		htlc = &htlcStatus{
+			HTLCAttemptInfo: a,
+		}
+
+		var firstFailReason *FailureReason
+		if test.settleFirst {
+			_, err := pControl.SettleAttempt(
+				info.PaymentHash, a.AttemptID,
+				&HTLCSettleInfo{
+					Preimage: preimg,
+				},
+			)
+			if err != nil {
+				t.Fatalf("error shouldn't have been "+
+					"received, got: %v", err)
+			}
+
+			// Assert that the HTLC has had the preimage recorded.
+			htlc.settle = &preimg
+			assertPaymentInfo(
+				t, pControl, info.PaymentHash, info, nil, htlc,
+			)
+		} else {
+			err := pControl.FailAttempt(
+				info.PaymentHash, a.AttemptID,
+				&HTLCFailInfo{
+					Reason: htlcFail,
+				},
+			)
+			if err != nil {
+				t.Fatalf("error shouldn't have been "+
+					"received, got: %v", err)
+			}
+
+			// Assert the failure was recorded.
+			htlc.failure = &htlcFail
+			assertPaymentInfo(
+				t, pControl, info.PaymentHash, info, nil, htlc,
+			)
+
+			// We also record a payment level fail, to move it into
+			// a terminal state.
+			failReason := FailureReasonNoRoute
+			_, err = pControl.Fail(info.PaymentHash, failReason)
+			if err != nil {
+				t.Fatalf("unable to fail payment hash: %v", err)
+			}
+
+			// Record the reason we failed the payment, such that
+			// we can assert this later in the test.
+			firstFailReason = &failReason
+		}
+
+		// The payment should still be considered in-flight, since there
+		// is still an active HTLC.
+		assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+
+		// Try to register yet another attempt. This should fail now
+		// that the payment has reached a terminal condition.
+		b = *attempt
+		b.AttemptID = 3
+		err = pControl.RegisterAttempt(info.PaymentHash, &b)
+		if err != ErrPaymentTerminal {
+			t.Fatalf("expected ErrPaymentTerminal, got: %v", err)
+		}
+
+		assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
+
+		// Settle or fail the remaining attempt based on the testcase.
+		a = attempts[2]
+		htlc = &htlcStatus{
+			HTLCAttemptInfo: a,
+		}
+		if test.settleLast {
+			// Settle the last outstanding attempt.
+			_, err = pControl.SettleAttempt(
+				info.PaymentHash, a.AttemptID,
+				&HTLCSettleInfo{
+					Preimage: preimg,
+				},
+			)
+			if err != nil {
+				t.Fatalf("error shouldn't have been "+
+					"received, got: %v", err)
+			}
+
+			htlc.settle = &preimg
+			assertPaymentInfo(
+				t, pControl, info.PaymentHash, info,
+				firstFailReason, htlc,
+			)
+		} else {
+			// Fail the attempt.
+			err := pControl.FailAttempt(
+				info.PaymentHash, a.AttemptID,
+				&HTLCFailInfo{
+					Reason: htlcFail,
+				},
+			)
+			if err != nil {
+				t.Fatalf("error shouldn't have been "+
+					"received, got: %v", err)
+			}
+
+			// Assert the failure was recorded.
+			htlc.failure = &htlcFail
+			assertPaymentInfo(
+				t, pControl, info.PaymentHash, info,
+				firstFailReason, htlc,
+			)
+
+			// Check that we can override any perevious terminal
+			// failure. This is to allow multiple concurrent shard
+			// write a terminal failure to the database without
+			// syncing.
+			failReason := FailureReasonPaymentDetails
+			_, err = pControl.Fail(info.PaymentHash, failReason)
+			if err != nil {
+				t.Fatalf("unable to fail payment hash: %v", err)
+			}
+		}
+
+		// If any of the two attempts settled, the payment should end
+		// up in the Succeeded state. If both failed the payment should
+		// also be Failed at this poinnt.
+		finalStatus := StatusFailed
+		expRegErr := ErrPaymentAlreadyFailed
+		if test.settleFirst || test.settleLast {
+			finalStatus = StatusSucceeded
+			expRegErr = ErrPaymentAlreadySucceeded
+		}
+
+		assertPaymentStatus(t, pControl, info.PaymentHash, finalStatus)
+
+		// Finally assert we cannot register more attempts.
+		err = pControl.RegisterAttempt(info.PaymentHash, &b)
+		if err != expRegErr {
+			t.Fatalf("expected error %v, got: %v", expRegErr, err)
+		}
+	}
+
+	for _, test := range tests {
+		test := test
+		subTest := fmt.Sprintf("first=%v, second=%v",
+			test.settleFirst, test.settleLast)
+
+		t.Run(subTest, func(t *testing.T) {
+			runSubTest(t, test)
+		})
+	}
+}
+
+func TestPaymentControlMPPRecordValidation(t *testing.T) {
+	t.Parallel()
+
+	db, err := initDB()
+	if err != nil {
+		t.Fatalf("unable to init db: %v", err)
+	}
+
+	pControl := NewPaymentControl(db)
+
+	info, attempt, _, err := genInfo()
+	if err != nil {
+		t.Fatalf("unable to generate htlc message: %v", err)
+	}
+
+	// Init the payment.
+	err = pControl.InitPayment(info.PaymentHash, info)
+	if err != nil {
+		t.Fatalf("unable to send htlc message: %v", err)
+	}
+
+	// Create three unique attempts we'll use for the test, and
+	// register them with the payment control. We set each
+	// attempts's value to one third of the payment amount, and
+	// populate the MPP options.
+	shardAmt := info.Value / 3
+	attempt.Route.FinalHop().AmtToForward = shardAmt
+	attempt.Route.FinalHop().MPP = record.NewMPP(
+		info.Value, [32]byte{1},
+	)
+
+	err = pControl.RegisterAttempt(info.PaymentHash, attempt)
+	if err != nil {
+		t.Fatalf("unable to send htlc message: %v", err)
+	}
+
+	// Now try to register a non-MPP attempt, which should fail.
+	b := *attempt
+	b.AttemptID = 1
+	b.Route.FinalHop().MPP = nil
+	err = pControl.RegisterAttempt(info.PaymentHash, &b)
+	if err != ErrMPPayment {
+		t.Fatalf("expected ErrMPPayment, got: %v", err)
+	}
+
+	// Try to register attempt one with a different payment address.
+	b.Route.FinalHop().MPP = record.NewMPP(
+		info.Value, [32]byte{2},
+	)
+	err = pControl.RegisterAttempt(info.PaymentHash, &b)
+	if err != ErrMPPPaymentAddrMismatch {
+		t.Fatalf("expected ErrMPPPaymentAddrMismatch, got: %v", err)
+	}
+
+	// Try registering one with a different total amount.
+	b.Route.FinalHop().MPP = record.NewMPP(
+		info.Value/2, [32]byte{1},
+	)
+	err = pControl.RegisterAttempt(info.PaymentHash, &b)
+	if err != ErrMPPTotalAmountMismatch {
+		t.Fatalf("expected ErrMPPTotalAmountMismatch, got: %v", err)
+	}
+
+	// Create and init a new payment. This time we'll check that we cannot
+	// register an MPP attempt if we already registered a non-MPP one.
+	info, attempt, _, err = genInfo()
+	if err != nil {
+		t.Fatalf("unable to generate htlc message: %v", err)
+	}
+
+	err = pControl.InitPayment(info.PaymentHash, info)
+	if err != nil {
+		t.Fatalf("unable to send htlc message: %v", err)
+	}
+
+	attempt.Route.FinalHop().MPP = nil
+	err = pControl.RegisterAttempt(info.PaymentHash, attempt)
+	if err != nil {
+		t.Fatalf("unable to send htlc message: %v", err)
+	}
+
+	// Attempt to register an MPP attempt, which should fail.
+	b = *attempt
+	b.AttemptID = 1
+	b.Route.FinalHop().MPP = record.NewMPP(
+		info.Value, [32]byte{1},
+	)
+
+	err = pControl.RegisterAttempt(info.PaymentHash, &b)
+	if err != ErrNonMPPayment {
+		t.Fatalf("expected ErrNonMPPayment, got: %v", err)
+	}
+}
+
 // assertPaymentStatus retrieves the status of the payment referred to by hash
 // and compares it with the expected state.
 func assertPaymentStatus(t *testing.T, p *PaymentControl,
@@ -452,11 +840,16 @@ func assertPaymentStatus(t *testing.T, p *PaymentControl,
 	}
 }
 
+type htlcStatus struct {
+	*HTLCAttemptInfo
+	settle  *lntypes.Preimage
+	failure *HTLCFailReason
+}
+
 // assertPaymentInfo retrieves the payment referred to by hash and verifies the
 // expected values.
 func assertPaymentInfo(t *testing.T, p *PaymentControl, hash lntypes.Hash,
-	c *PaymentCreationInfo, aIdx int, a *HTLCAttemptInfo, s lntypes.Preimage,
-	f *FailureReason) {
+	c *PaymentCreationInfo, f *FailureReason, a *htlcStatus) {
 
 	t.Helper()
 
@@ -487,20 +880,35 @@ func assertPaymentInfo(t *testing.T, p *PaymentControl, hash lntypes.Hash,
 		return
 	}
 
-	htlc := payment.HTLCs[aIdx]
+	htlc := payment.HTLCs[a.AttemptID]
 	if err := assertRouteEqual(&htlc.Route, &a.Route); err != nil {
 		t.Fatal("routes do not match")
 	}
 
-	var zeroPreimage = lntypes.Preimage{}
-	if s != zeroPreimage {
-		if htlc.Settle.Preimage != s {
+	if htlc.AttemptID != a.AttemptID {
+		t.Fatalf("unnexpected attempt ID %v, expected %v",
+			htlc.AttemptID, a.AttemptID)
+	}
+
+	if a.failure != nil {
+		if htlc.Failure == nil {
+			t.Fatalf("expected HTLC to be failed")
+		}
+
+		if htlc.Failure.Reason != *a.failure {
+			t.Fatalf("expected HTLC failure %v, had %v",
+				*a.failure, htlc.Failure.Reason)
+		}
+	} else if htlc.Failure != nil {
+		t.Fatalf("expected no HTLC failure")
+	}
+
+	if a.settle != nil {
+		if htlc.Settle.Preimage != *a.settle {
 			t.Fatalf("Preimages don't match: %x vs %x",
-				htlc.Settle.Preimage, s)
-		}
-	} else {
-		if htlc.Settle != nil {
-			t.Fatal("expected no settle info")
+				htlc.Settle.Preimage, a.settle)
 		}
+	} else if htlc.Settle != nil {
+		t.Fatal("expected no settle info")
 	}
 }

channeldb/payments.go

@@ -123,7 +123,12 @@ const (
 	// LocalLiquidityInsufficient, RemoteCapacityInsufficient.
 )
 
-// String returns a human readable FailureReason
+// Error returns a human readable error string for the FailureReason.
+func (r FailureReason) Error() string {
+	return r.String()
+}
+
+// String returns a human readable FailureReason.
 func (r FailureReason) String() string {
 	switch r {
 	case FailureReasonTimeout:
@@ -247,6 +252,16 @@ func (db *DB) FetchPayments() ([]*MPPayment, error) {
 	return payments, nil
 }
 
+func fetchCreationInfo(bucket kvdb.ReadBucket) (*PaymentCreationInfo, error) {
+	b := bucket.Get(paymentCreationInfoKey)
+	if b == nil {
+		return nil, fmt.Errorf("creation info not found")
+	}
+
+	r := bytes.NewReader(b)
+	return deserializePaymentCreationInfo(r)
+}
+
 func fetchPayment(bucket kvdb.ReadBucket) (*MPPayment, error) {
 	seqBytes := bucket.Get(paymentSequenceKey)
 	if seqBytes == nil {
@@ -255,20 +270,8 @@ func fetchPayment(bucket kvdb.ReadBucket) (*MPPayment, error) {
 
 	sequenceNum := binary.BigEndian.Uint64(seqBytes)
 
-	// Get the payment status.
-	paymentStatus, err := fetchPaymentStatus(bucket)
-	if err != nil {
-		return nil, err
-	}
-
 	// Get the PaymentCreationInfo.
-	b := bucket.Get(paymentCreationInfoKey)
-	if b == nil {
-		return nil, fmt.Errorf("creation info not found")
-	}
-
-	r := bytes.NewReader(b)
-	creationInfo, err := deserializePaymentCreationInfo(r)
+	creationInfo, err := fetchCreationInfo(bucket)
 	if err != nil {
 		return nil, err
 
@@ -286,12 +289,50 @@ func fetchPayment(bucket kvdb.ReadBucket) (*MPPayment, error) {
 
 	// Get failure reason if available.
 	var failureReason *FailureReason
-	b = bucket.Get(paymentFailInfoKey)
+	b := bucket.Get(paymentFailInfoKey)
 	if b != nil {
 		reason := FailureReason(b[0])
 		failureReason = &reason
 	}
 
+	// Go through all HTLCs for this payment, noting whether we have any
+	// settled HTLC, and any still in-flight.
+	var inflight, settled bool
+	for _, h := range htlcs {
+		if h.Failure != nil {
+			continue
+		}
+
+		if h.Settle != nil {
+			settled = true
+			continue
+		}
+
+		// If any of the HTLCs are not failed nor settled, we
+		// still have inflight HTLCs.
+		inflight = true
+	}
+
+	// Use the DB state to determine the status of the payment.
+	var paymentStatus PaymentStatus
+
+	switch {
+
+	// If any of the the HTLCs did succeed and there are no HTLCs in
+	// flight, the payment succeeded.
+	case !inflight && settled:
+		paymentStatus = StatusSucceeded
+
+	// If we have no in-flight HTLCs, and the payment failure is set, the
+	// payment is considered failed.
+	case !inflight && failureReason != nil:
+		paymentStatus = StatusFailed
+
+	// Otherwise it is still in flight.
+	default:
+		paymentStatus = StatusInFlight
+	}
+
 	return &MPPayment{
 		sequenceNum:   sequenceNum,
 		Info:          creationInfo,
@@ -407,6 +448,8 @@ func (db *DB) DeletePayments() error {
 				return err
 			}
 
+			// If the status is InFlight, we cannot safely delete
+			// the payment information, so we return early.
 			if paymentStatus == StatusInFlight {
 				return nil
 			}