channeldb+routing: store all payment htlcs

This commit converts the database structure of a payment so that it can
not just store the last htlc attempt, but all attempts that have been
made. This is a preparation for mpp sending.

In addition to that, we now also persist the fail time of an htlc. In a
later commit, the full failure reason will be added as well.

A key change is made to the control tower interface. Previously the
control tower wasn't aware of individual htlc outcomes. The payment
remained in-flight with the latest attempt recorded, but an outcome was
only set when the payment finished. With this commit, the outcome of
every htlc is expected by the control tower and recorded in the
database.

Co-authored-by: Johan T. Halseth <johanth@gmail.com>

channeldb/mp_payment.go

@@ -1,6 +1,7 @@
 package channeldb
 
 import (
+	"io"
 	"time"
 
 	"github.com/btcsuite/btcd/btcec"
@@ -92,3 +93,88 @@ type MPPayment struct {
 	// Status is the current PaymentStatus of this payment.
 	Status PaymentStatus
 }
+
+// serializeHTLCSettleInfo serializes the details of a settled htlc.
+func serializeHTLCSettleInfo(w io.Writer, s *HTLCSettleInfo) error {
+	if _, err := w.Write(s.Preimage[:]); err != nil {
+		return err
+	}
+
+	if err := serializeTime(w, s.SettleTime); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// deserializeHTLCSettleInfo deserializes the details of a settled htlc.
+func deserializeHTLCSettleInfo(r io.Reader) (*HTLCSettleInfo, error) {
+	s := &HTLCSettleInfo{}
+	if _, err := io.ReadFull(r, s.Preimage[:]); err != nil {
+		return nil, err
+	}
+
+	var err error
+	s.SettleTime, err = deserializeTime(r)
+	if err != nil {
+		return nil, err
+	}
+
+	return s, nil
+}
+
+// serializeHTLCFailInfo serializes the details of a failed htlc including the
+// wire failure.
+func serializeHTLCFailInfo(w io.Writer, f *HTLCFailInfo) error {
+	if err := serializeTime(w, f.FailTime); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// deserializeHTLCFailInfo deserializes the details of a failed htlc including
+// the wire failure.
+func deserializeHTLCFailInfo(r io.Reader) (*HTLCFailInfo, error) {
+	f := &HTLCFailInfo{}
+	var err error
+	f.FailTime, err = deserializeTime(r)
+	if err != nil {
+		return nil, err
+	}
+
+	return f, nil
+}
+
+// deserializeTime deserializes time as unix nanoseconds.
+func deserializeTime(r io.Reader) (time.Time, error) {
+	var scratch [8]byte
+	if _, err := io.ReadFull(r, scratch[:]); err != nil {
+		return time.Time{}, err
+	}
+
+	// Convert to time.Time. Interpret unix nano time zero as a zero
+	// time.Time value.
+	unixNano := byteOrder.Uint64(scratch[:])
+	if unixNano == 0 {
+		return time.Time{}, nil
+	}
+
+	return time.Unix(0, int64(unixNano)), nil
+}
+
+// serializeTime serializes time as unix nanoseconds.
+func serializeTime(w io.Writer, t time.Time) error {
+	var scratch [8]byte
+
+	// Convert to unix nano seconds, but only if time is non-zero. Calling
+	// UnixNano() on a zero time yields an undefined result.
+	var unixNano int64
+	if !t.IsZero() {
+		unixNano = t.UnixNano()
+	}
+
+	byteOrder.PutUint64(scratch[:], uint64(unixNano))
+	_, err := w.Write(scratch[:])
+	return err
+}

channeldb/payment_control.go

@@ -127,11 +127,11 @@ func (p *PaymentControl) InitPayment(paymentHash lntypes.Hash,
 			return err
 		}
 
-		// We'll delete any lingering attempt info to start with, in
+		// We'll delete any lingering HTLCs to start with, in case we
-		// case we are initializing a payment that was attempted
+		// are initializing a payment that was attempted earlier, but
-		// earlier, but left in a state where we could retry.
+		// left in a state where we could retry.
-		err = bucket.Delete(paymentAttemptInfoKey)
+		err = bucket.DeleteBucket(paymentHtlcsBucket)
-		if err != nil {
+		if err != nil && err != bbolt.ErrBucketNotFound {
 			return err
 		}
 
@@ -153,76 +153,113 @@ func (p *PaymentControl) RegisterAttempt(paymentHash lntypes.Hash,
 
 	// Serialize the information before opening the db transaction.
 	var a bytes.Buffer
-	if err := serializeHTLCAttemptInfo(&a, attempt); err != nil {
+	err := serializeHTLCAttemptInfo(&a, attempt)
+	if err != nil {
 		return err
 	}
-	attemptBytes := a.Bytes()
+	htlcInfoBytes := a.Bytes()
 
-	var updateErr error
+	htlcIDBytes := make([]byte, 8)
-	err := p.db.Batch(func(tx *bbolt.Tx) error {
+	binary.BigEndian.PutUint64(htlcIDBytes, attempt.AttemptID)
-		// Reset the update error, to avoid carrying over an error
-		// from a previous execution of the batched db transaction.
-		updateErr = nil
 
+	return p.db.Update(func(tx *bbolt.Tx) error {
+		// Get the payment bucket to register this new attempt in.
 		bucket, err := fetchPaymentBucket(tx, paymentHash)
-		if err == ErrPaymentNotInitiated {
+		if err != nil {
-			updateErr = ErrPaymentNotInitiated
-			return nil
-		} else if err != nil {
 			return err
 		}
 
 		// We can only register attempts for payments that are
 		// in-flight.
 		if err := ensureInFlight(bucket); err != nil {
-			updateErr = err
+			return err
-			return nil
 		}
 
-		// Add the payment attempt to the payments bucket.
+		htlcsBucket, err := bucket.CreateBucketIfNotExists(
-		return bucket.Put(paymentAttemptInfoKey, attemptBytes)
+			paymentHtlcsBucket,
-	})
+		)
 		if err != nil {
 			return err
 		}
 
-	return updateErr
+		// Create bucket for this attempt. Fail if the bucket already
-}
+		// exists.
-
+		htlcBucket, err := htlcsBucket.CreateBucket(htlcIDBytes)
-// Success transitions a payment into the Succeeded state. After invoking this
+		if err != nil {
-// method, InitPayment should always return an error to prevent us from making
-// duplicate payments to the same payment hash. The provided preimage is
-// atomically saved to the DB for record keeping.
-func (p *PaymentControl) Success(paymentHash lntypes.Hash,
-	preimage lntypes.Preimage) (*MPPayment, error) {
-
-	var (
-		updateErr error
-		payment   *MPPayment
-	)
-	err := p.db.Batch(func(tx *bbolt.Tx) error {
-		// Reset the update error, to avoid carrying over an error
-		// from a previous execution of the batched db transaction.
-		updateErr = nil
-		payment = nil
-
-		bucket, err := fetchPaymentBucket(tx, paymentHash)
-		if err == ErrPaymentNotInitiated {
-			updateErr = ErrPaymentNotInitiated
-			return nil
-		} else if err != nil {
 			return err
 		}
 
-		// We can only mark in-flight payments as succeeded.
+		return htlcBucket.Put(htlcAttemptInfoKey, htlcInfoBytes)
-		if err := ensureInFlight(bucket); err != nil {
+	})
-			updateErr = err
+}
-			return nil
+
+// SettleAttempt marks the given attempt settled with the preimage. If this is
+// a multi shard payment, this might implicitly mean that the full payment
+// succeeded.
+//
+// After invoking this method, InitPayment should always return an error to
+// prevent us from making duplicate payments to the same payment hash. The
+// provided preimage is atomically saved to the DB for record keeping.
+func (p *PaymentControl) SettleAttempt(hash lntypes.Hash,
+	attemptID uint64, settleInfo *HTLCSettleInfo) (*MPPayment, error) {
+
+	var b bytes.Buffer
+	if err := serializeHTLCSettleInfo(&b, settleInfo); err != nil {
+		return nil, err
+	}
+	settleBytes := b.Bytes()
+
+	return p.updateHtlcKey(hash, attemptID, htlcSettleInfoKey, settleBytes)
+}
+
+// FailAttempt marks the given payment attempt failed.
+func (p *PaymentControl) FailAttempt(hash lntypes.Hash,
+	attemptID uint64, failInfo *HTLCFailInfo) error {
+
+	var b bytes.Buffer
+	if err := serializeHTLCFailInfo(&b, failInfo); err != nil {
+		return err
+	}
+	failBytes := b.Bytes()
+
+	_, err := p.updateHtlcKey(hash, attemptID, htlcFailInfoKey, failBytes)
+	return err
+}
+
+// updateHtlcKey updates a database key for the specified htlc.
+func (p *PaymentControl) updateHtlcKey(paymentHash lntypes.Hash,
+	attemptID uint64, key, value []byte) (*MPPayment, error) {
+
+	htlcIDBytes := make([]byte, 8)
+	binary.BigEndian.PutUint64(htlcIDBytes, attemptID)
+
+	var payment *MPPayment
+	err := p.db.Batch(func(tx *bbolt.Tx) error {
+		// Fetch bucket that contains all information for the payment
+		// with this hash.
+		bucket, err := fetchPaymentBucket(tx, paymentHash)
+		if err != nil {
+			return err
 		}
 
-		// Record the successful payment info atomically to the
+		// We can only update keys of in-flight payments.
-		// payments record.
+		if err := ensureInFlight(bucket); err != nil {
-		err = bucket.Put(paymentSettleInfoKey, preimage[:])
+			return err
+		}
+
+		htlcsBucket := bucket.Bucket(paymentHtlcsBucket)
+		if htlcsBucket == nil {
+			return fmt.Errorf("htlcs bucket not found")
+		}
+
+		htlcBucket := htlcsBucket.Bucket(htlcIDBytes)
+		if htlcBucket == nil {
+			return fmt.Errorf("HTLC with ID %v not registered",
+				attemptID)
+		}
+
+		// Add or update the key for this htlc.
+		err = htlcBucket.Put(key, value)
 		if err != nil {
 			return err
 		}
@@ -235,7 +272,7 @@ func (p *PaymentControl) Success(paymentHash lntypes.Hash,
 		return nil, err
 	}
 
-	return payment, updateErr
+	return payment, err
 }
 
 // Fail transitions a payment into the Failed state, and records the reason the
@@ -278,7 +315,19 @@ func (p *PaymentControl) Fail(paymentHash lntypes.Hash,
 
 		// Retrieve attempt info for the notification, if available.
 		payment, err = fetchPayment(bucket)
+		if err != nil {
 			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 {
 		return nil, err
@@ -338,7 +387,6 @@ func fetchPaymentBucket(tx *bbolt.Tx, paymentHash lntypes.Hash) (
 	}
 
 	return bucket, nil
-
 }
 
 // nextPaymentSequence returns the next sequence number to store for a new
@@ -362,9 +410,22 @@ func nextPaymentSequence(tx *bbolt.Tx) ([]byte, error) {
 // fetchPaymentStatus fetches the payment status of the payment. If the payment
 // isn't found, it will default to "StatusUnknown".
 func fetchPaymentStatus(bucket *bbolt.Bucket) (PaymentStatus, error) {
-	if bucket.Get(paymentSettleInfoKey) != nil {
+	htlcsBucket := bucket.Bucket(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
 			}
+		}
+
+	}
 
 	if bucket.Get(paymentFailInfoKey) != nil {
 		return StatusFailed, nil
@@ -410,27 +471,16 @@ func ensureInFlight(bucket *bbolt.Bucket) error {
 	}
 }
 
-// fetchPaymentAttempt fetches the payment attempt from the bucket.
-func fetchPaymentAttempt(bucket *bbolt.Bucket) (*HTLCAttemptInfo, error) {
-	attemptData := bucket.Get(paymentAttemptInfoKey)
-	if attemptData == nil {
-		return nil, errNoAttemptInfo
-	}
-
-	r := bytes.NewReader(attemptData)
-	return deserializeHTLCAttemptInfo(r)
-}
-
 // InFlightPayment is a wrapper around a payment that has status InFlight.
 type InFlightPayment struct {
 	// Info is the PaymentCreationInfo of the in-flight payment.
 	Info *PaymentCreationInfo
 
-	// Attempt contains information about the last payment attempt that was
+	// Attempts is the set of payment attempts that was made to this
-	// made to this payment hash.
+	// payment hash.
 	//
-	// NOTE: Might be nil.
+	// NOTE: Might be empty.
-	Attempt *HTLCAttemptInfo
+	Attempts []HTLCAttemptInfo
 }
 
 // FetchInFlightPayments returns all payments with status InFlight.
@@ -473,13 +523,31 @@ func (p *PaymentControl) FetchInFlightPayments() ([]*InFlightPayment, error) {
 				return err
 			}
 
-			// Now get the attempt info. It could be that there is
+			htlcsBucket := bucket.Bucket(paymentHtlcsBucket)
-			// no attempt info yet.
+			if htlcsBucket == nil {
-			inFlight.Attempt, err = fetchPaymentAttempt(bucket)
+				return nil
-			if err != nil && err != errNoAttemptInfo {
+			}
+
+			// Fetch all HTLCs attempted for this payment.
+			htlcs, err := fetchHtlcAttempts(htlcsBucket)
+			if err != nil {
 				return err
 			}
 
+			// We only care about the static info for the HTLCs
+			// still in flight, so convert the result to a slice of
+			// HTLCAttemptInfos.
+			for _, h := range htlcs {
+				// Skip HTLCs not in flight.
+				if h.Settle != nil || h.Failure != nil {
+					continue
+				}
+
+				inFlight.Attempts = append(
+					inFlight.Attempts, h.HTLCAttemptInfo,
+				)
+			}
+
 			inFlights = append(inFlights, inFlight)
 			return nil
 		})

channeldb/payment_control_test.go

@@ -85,7 +85,7 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, nil, lntypes.Preimage{},
+		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
 		nil,
 	)
 
@@ -99,7 +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, nil, lntypes.Preimage{},
+		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
 		&failReason,
 	)
 
@@ -112,7 +112,7 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, nil, lntypes.Preimage{},
+		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
 		nil,
 	)
 
@@ -125,6 +125,13 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 		t.Fatalf("unable to register attempt: %v", err)
 	}
 
+	err = pControl.FailAttempt(
+		info.PaymentHash, 2, &HTLCFailInfo{},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	// Record another attempt.
 	attempt.AttemptID = 3
 	err = pControl.RegisterAttempt(info.PaymentHash, attempt)
@@ -133,19 +140,24 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 	}
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, attempt, lntypes.Preimage{},
+		t, pControl, info.PaymentHash, info, 0, attempt, lntypes.Preimage{},
 		nil,
 	)
 
 	// Settle the attempt and verify that status was changed to StatusSucceeded.
 	var payment *MPPayment
-	payment, err = pControl.Success(info.PaymentHash, preimg)
+	payment, err = pControl.SettleAttempt(
+		info.PaymentHash, 3,
+		&HTLCSettleInfo{
+			Preimage: preimg,
+		},
+	)
 	if err != nil {
 		t.Fatalf("error shouldn't have been received, got: %v", err)
 	}
 
-	if len(payment.HTLCs) != 1 {
+	if len(payment.HTLCs) != 2 {
-		t.Fatalf("payment should have one htlc, got: %d",
+		t.Fatalf("payment should have two htlcs, got: %d",
 			len(payment.HTLCs))
 	}
 
@@ -157,7 +169,7 @@ func TestPaymentControlSwitchFail(t *testing.T) {
 	}
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusSucceeded)
-	assertPaymentInfo(t, pControl, info.PaymentHash, info, attempt, preimg, nil)
+	assertPaymentInfo(t, pControl, info.PaymentHash, info, 1, attempt, preimg, nil)
 
 	// Attempt a final payment, which should now fail since the prior
 	// payment succeed.
@@ -193,7 +205,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, nil, lntypes.Preimage{},
+		t, pControl, info.PaymentHash, info, 0, nil, lntypes.Preimage{},
 		nil,
 	)
 
@@ -213,7 +225,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 	}
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 	assertPaymentInfo(
-		t, pControl, info.PaymentHash, info, attempt, lntypes.Preimage{},
+		t, pControl, info.PaymentHash, info, 0, attempt, lntypes.Preimage{},
 		nil,
 	)
 
@@ -225,11 +237,17 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) {
 	}
 
 	// After settling, the error should be ErrAlreadyPaid.
-	if _, err := pControl.Success(info.PaymentHash, preimg); err != nil {
+	_, err = pControl.SettleAttempt(
+		info.PaymentHash, 1,
+		&HTLCSettleInfo{
+			Preimage: preimg,
+		},
+	)
+	if err != nil {
 		t.Fatalf("error shouldn't have been received, got: %v", err)
 	}
 	assertPaymentStatus(t, pControl, info.PaymentHash, StatusSucceeded)
-	assertPaymentInfo(t, pControl, info.PaymentHash, info, attempt, preimg, nil)
+	assertPaymentInfo(t, pControl, info.PaymentHash, info, 0, attempt, preimg, nil)
 
 	err = pControl.InitPayment(info.PaymentHash, info)
 	if err != ErrAlreadyPaid {
@@ -255,7 +273,12 @@ func TestPaymentControlSuccessesWithoutInFlight(t *testing.T) {
 	}
 
 	// Attempt to complete the payment should fail.
-	_, err = pControl.Success(info.PaymentHash, preimg)
+	_, err = pControl.SettleAttempt(
+		info.PaymentHash, 0,
+		&HTLCSettleInfo{
+			Preimage: preimg,
+		},
+	)
 	if err != ErrPaymentNotInitiated {
 		t.Fatalf("expected ErrPaymentNotInitiated, got %v", err)
 	}
@@ -336,6 +359,15 @@ func TestPaymentControlDeleteNonInFligt(t *testing.T) {
 		}
 
 		if p.failed {
+			// Fail the payment attempt.
+			err := pControl.FailAttempt(
+				info.PaymentHash, attempt.AttemptID,
+				&HTLCFailInfo{},
+			)
+			if err != nil {
+				t.Fatalf("unable to fail htlc: %v", err)
+			}
+
 			// Fail the payment, which should moved it to Failed.
 			failReason := FailureReasonNoRoute
 			_, err = pControl.Fail(info.PaymentHash, failReason)
@@ -346,24 +378,29 @@ func TestPaymentControlDeleteNonInFligt(t *testing.T) {
 			// Verify the status is indeed Failed.
 			assertPaymentStatus(t, pControl, info.PaymentHash, StatusFailed)
 			assertPaymentInfo(
-				t, pControl, info.PaymentHash, info, attempt,
+				t, pControl, info.PaymentHash, info, 0, attempt,
 				lntypes.Preimage{}, &failReason,
 			)
 		} else if p.success {
 			// Verifies that status was changed to StatusSucceeded.
-			_, err := pControl.Success(info.PaymentHash, preimg)
+			_, err := pControl.SettleAttempt(
+				info.PaymentHash, 1,
+				&HTLCSettleInfo{
+					Preimage: preimg,
+				},
+			)
 			if err != nil {
 				t.Fatalf("error shouldn't have been received, got: %v", err)
 			}
 
 			assertPaymentStatus(t, pControl, info.PaymentHash, StatusSucceeded)
 			assertPaymentInfo(
-				t, pControl, info.PaymentHash, info, attempt, preimg, nil,
+				t, pControl, info.PaymentHash, info, 0, attempt, preimg, nil,
 			)
 		} else {
 			assertPaymentStatus(t, pControl, info.PaymentHash, StatusInFlight)
 			assertPaymentInfo(
-				t, pControl, info.PaymentHash, info, attempt,
+				t, pControl, info.PaymentHash, info, 0, attempt,
 				lntypes.Preimage{}, nil,
 			)
 		}
@@ -414,7 +451,7 @@ func assertPaymentStatus(t *testing.T, p *PaymentControl,
 // 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, a *HTLCAttemptInfo, s lntypes.Preimage,
+	c *PaymentCreationInfo, aIdx int, a *HTLCAttemptInfo, s lntypes.Preimage,
 	f *FailureReason) {
 
 	t.Helper()
@@ -446,7 +483,7 @@ func assertPaymentInfo(t *testing.T, p *PaymentControl, hash lntypes.Hash,
 		return
 	}
 
-	htlc := payment.HTLCs[0]
+	htlc := payment.HTLCs[aIdx]
 	if err := assertRouteEqual(&htlc.Route, &a.Route); err != nil {
 		t.Fatal("routes do not match")
 	}

channeldb/payments.go

@@ -30,9 +30,19 @@ var (
 	//      |-- <paymenthash>
 	//      |        |--sequence-key: <sequence number>
 	//      |        |--creation-info-key: <creation info>
-	//      |        |--attempt-info-key: <attempt info>
+	//      |        |--fail-info-key: <(optional) fail info>
-	//      |        |--settle-info-key: <settle info>
+	//      |        |
-	//      |        |--fail-info-key: <fail info>
+	//      |        |--payment-htlcs-bucket (shard-bucket)
+	//      |        |        |
+	//      |        |        |-- <htlc attempt ID>
+	//      |        |        |       |--htlc-attempt-info-key: <htlc attempt info>
+	//      |        |        |       |--htlc-settle-info-key: <(optional) settle info>
+	//      |        |        |       |--htlc-fail-info-key: <(optional) fail info>
+	//      |        |        |
+	//      |        |        |-- <htlc attempt ID>
+	//      |        |        |       |
+	//      |        |       ...     ...
+	//      |        |
 	//      |        |
 	//      |        |--duplicate-bucket (only for old, completed payments)
 	//      |                 |
@@ -62,14 +72,21 @@ var (
 	// store the creation info of the payment.
 	paymentCreationInfoKey = []byte("payment-creation-info")
 
-	// paymentAttemptInfoKey is a key used in the payment's sub-bucket to
+	// paymentHtlcsBucket is a bucket where we'll store the information
-	// store the info about the latest attempt that was done for the
+	// about the HTLCs that were attempted for a payment.
-	// payment in question.
+	paymentHtlcsBucket = []byte("payment-htlcs-bucket")
-	paymentAttemptInfoKey = []byte("payment-attempt-info")
 
-	// paymentSettleInfoKey is a key used in the payment's sub-bucket to
+	// htlcAttemptInfoKey is a key used in a HTLC's sub-bucket to store the
-	// store the settle info of the payment.
+	// info about the attempt that was done for the HTLC in question.
-	paymentSettleInfoKey = []byte("payment-settle-info")
+	htlcAttemptInfoKey = []byte("htlc-attempt-info")
+
+	// htlcSettleInfoKey is a key used in a HTLC's sub-bucket to store the
+	// settle info, if any.
+	htlcSettleInfoKey = []byte("htlc-settle-info")
+
+	// htlcFailInfoKey is a key used in a HTLC's sub-bucket to store
+	// failure information, if any.
+	htlcFailInfoKey = []byte("htlc-fail-info")
 
 	// paymentFailInfoKey is a key used in the payment's sub-bucket to
 	// store information about the reason a payment failed.
@@ -177,98 +194,6 @@ type PaymentCreationInfo struct {
 	PaymentRequest []byte
 }
 
-// Payment is a wrapper around a payment's PaymentCreationInfo,
-// HTLCAttemptInfo, and preimage. All payments will have the
-// PaymentCreationInfo set, the HTLCAttemptInfo will be set only if at least
-// one payment attempt has been made, while only completed payments will have a
-// non-zero payment preimage.
-type Payment struct {
-	// sequenceNum is a unique identifier used to sort the payments in
-	// order of creation.
-	sequenceNum uint64
-
-	// Status is the current PaymentStatus of this payment.
-	Status PaymentStatus
-
-	// Info holds all static information about this payment, and is
-	// populated when the payment is initiated.
-	Info *PaymentCreationInfo
-
-	// Attempt is the information about the last payment attempt made.
-	//
-	// NOTE: Can be nil if no attempt is yet made.
-	Attempt *HTLCAttemptInfo
-
-	// Preimage is the preimage of a successful payment. This serves as a
-	// proof of payment. It will only be non-nil for settled payments.
-	//
-	// NOTE: Can be nil if payment is not settled.
-	Preimage *lntypes.Preimage
-
-	// Failure is a failure reason code indicating the reason the payment
-	// failed. It is only non-nil for failed payments.
-	//
-	// NOTE: Can be nil if payment is not failed.
-	Failure *FailureReason
-}
-
-// ToMPPayment converts a legacy payment into an MPPayment.
-func (p *Payment) ToMPPayment() *MPPayment {
-	var (
-		htlcs   []HTLCAttempt
-		reason  *FailureReason
-		settle  *HTLCSettleInfo
-		failure *HTLCFailInfo
-	)
-
-	// Promote the payment failure to a proper fail struct, if it exists.
-	if p.Failure != nil {
-		// NOTE: FailTime is not set for legacy payments.
-		failure = &HTLCFailInfo{}
-		reason = p.Failure
-	}
-
-	// Promote the payment preimage to proper settle struct, if it exists.
-	if p.Preimage != nil {
-		// NOTE: SettleTime is not set for legacy payments.
-		settle = &HTLCSettleInfo{
-			Preimage: *p.Preimage,
-		}
-	}
-
-	// Either a settle or a failure may be set, but not both.
-	if settle != nil && failure != nil {
-		panic("htlc attempt has both settle and failure info")
-	}
-
-	// Populate a single HTLC on the MPPayment if an attempt exists on the
-	// legacy payment. If none exists we will leave the attempt info blank
-	// since we cannot recover it.
-	if p.Attempt != nil {
-		// NOTE: AttemptTime is not set for legacy payments.
-		htlcs = []HTLCAttempt{
-			{
-				HTLCAttemptInfo: *p.Attempt,
-				Settle:          settle,
-				Failure:         failure,
-			},
-		}
-	}
-
-	return &MPPayment{
-		sequenceNum: p.sequenceNum,
-		Info: &PaymentCreationInfo{
-			PaymentHash:    p.Info.PaymentHash,
-			Value:          p.Info.Value,
-			CreationTime:   p.Info.CreationTime,
-			PaymentRequest: p.Info.PaymentRequest,
-		},
-		HTLCs:         htlcs,
-		FailureReason: reason,
-		Status:        p.Status,
-	}
-}
-
 // FetchPayments returns all sent payments found in the DB.
 //
 // nolint: dupl
@@ -324,20 +249,15 @@ func (db *DB) FetchPayments() ([]*MPPayment, error) {
 }
 
 func fetchPayment(bucket *bbolt.Bucket) (*MPPayment, error) {
-	var (
-		err error
-		p   = &Payment{}
-	)
-
 	seqBytes := bucket.Get(paymentSequenceKey)
 	if seqBytes == nil {
 		return nil, fmt.Errorf("sequence number not found")
 	}
 
-	p.sequenceNum = binary.BigEndian.Uint64(seqBytes)
+	sequenceNum := binary.BigEndian.Uint64(seqBytes)
 
 	// Get the payment status.
-	p.Status, err = fetchPaymentStatus(bucket)
+	paymentStatus, err := fetchPaymentStatus(bucket)
 	if err != nil {
 		return nil, err
 	}
@@ -349,39 +269,118 @@ func fetchPayment(bucket *bbolt.Bucket) (*MPPayment, error) {
 	}
 
 	r := bytes.NewReader(b)
-	p.Info, err = deserializePaymentCreationInfo(r)
+	creationInfo, err := deserializePaymentCreationInfo(r)
 	if err != nil {
 		return nil, err
 
 	}
 
-	// Get the HTLCAttemptInfo. This can be unset.
+	var htlcs []HTLCAttempt
-	b = bucket.Get(paymentAttemptInfoKey)
+	htlcsBucket := bucket.Bucket(paymentHtlcsBucket)
-	if b != nil {
+	if htlcsBucket != nil {
-		r = bytes.NewReader(b)
+		// Get the payment attempts. This can be empty.
-		p.Attempt, err = deserializeHTLCAttemptInfo(r)
+		htlcs, err = fetchHtlcAttempts(htlcsBucket)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	// Get the payment preimage. This is only found for
-	// completed payments.
-	b = bucket.Get(paymentSettleInfoKey)
-	if b != nil {
-		var preimg lntypes.Preimage
-		copy(preimg[:], b[:])
-		p.Preimage = &preimg
-	}
-
 	// Get failure reason if available.
+	var failureReason *FailureReason
 	b = bucket.Get(paymentFailInfoKey)
 	if b != nil {
 		reason := FailureReason(b[0])
-		p.Failure = &reason
+		failureReason = &reason
 	}
 
-	return p.ToMPPayment(), nil
+	return &MPPayment{
+		sequenceNum:   sequenceNum,
+		Info:          creationInfo,
+		HTLCs:         htlcs,
+		FailureReason: failureReason,
+		Status:        paymentStatus,
+	}, nil
+}
+
+// fetchHtlcAttempts retrives all htlc attempts made for the payment found in
+// the given bucket.
+func fetchHtlcAttempts(bucket *bbolt.Bucket) ([]HTLCAttempt, error) {
+	htlcs := make([]HTLCAttempt, 0)
+
+	err := bucket.ForEach(func(k, _ []byte) error {
+		aid := byteOrder.Uint64(k)
+		htlcBucket := bucket.Bucket(k)
+
+		attemptInfo, err := fetchHtlcAttemptInfo(
+			htlcBucket,
+		)
+		if err != nil {
+			return err
+		}
+		attemptInfo.AttemptID = aid
+
+		htlc := HTLCAttempt{
+			HTLCAttemptInfo: *attemptInfo,
+		}
+
+		// Settle info might be nil.
+		htlc.Settle, err = fetchHtlcSettleInfo(htlcBucket)
+		if err != nil {
+			return err
+		}
+
+		// Failure info might be nil.
+		htlc.Failure, err = fetchHtlcFailInfo(htlcBucket)
+		if err != nil {
+			return err
+		}
+
+		htlcs = append(htlcs, htlc)
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return htlcs, nil
+}
+
+// fetchHtlcAttemptInfo fetches the payment attempt info for this htlc from the
+// bucket.
+func fetchHtlcAttemptInfo(bucket *bbolt.Bucket) (*HTLCAttemptInfo, error) {
+	b := bucket.Get(htlcAttemptInfoKey)
+	if b == nil {
+		return nil, errNoAttemptInfo
+	}
+
+	r := bytes.NewReader(b)
+	return deserializeHTLCAttemptInfo(r)
+}
+
+// fetchHtlcSettleInfo retrieves the settle info for the htlc. If the htlc isn't
+// settled, nil is returned.
+func fetchHtlcSettleInfo(bucket *bbolt.Bucket) (*HTLCSettleInfo, error) {
+	b := bucket.Get(htlcSettleInfoKey)
+	if b == nil {
+		// Settle info is optional.
+		return nil, nil
+	}
+
+	r := bytes.NewReader(b)
+	return deserializeHTLCSettleInfo(r)
+}
+
+// fetchHtlcFailInfo retrieves the failure info for the htlc. If the htlc hasn't
+// failed, nil is returned.
+func fetchHtlcFailInfo(bucket *bbolt.Bucket) (*HTLCFailInfo, error) {
+	b := bucket.Get(htlcFailInfoKey)
+	if b == nil {
+		// Fail info is optional.
+		return nil, nil
+	}
+
+	r := bytes.NewReader(b)
+	return deserializeHTLCFailInfo(r)
 }
 
 // DeletePayments deletes all completed and failed payments from the DB.
@@ -430,6 +429,7 @@ func (db *DB) DeletePayments() error {
 	})
 }
 
+// nolint: dupl
 func serializePaymentCreationInfo(w io.Writer, c *PaymentCreationInfo) error {
 	var scratch [8]byte
 
@@ -442,8 +442,7 @@ func serializePaymentCreationInfo(w io.Writer, c *PaymentCreationInfo) error {
 		return err
 	}
 
-	byteOrder.PutUint64(scratch[:], uint64(c.CreationTime.Unix()))
+	if err := serializeTime(w, c.CreationTime); err != nil {
-	if _, err := w.Write(scratch[:]); err != nil {
 		return err
 	}
 
@@ -473,10 +472,11 @@ func deserializePaymentCreationInfo(r io.Reader) (*PaymentCreationInfo, error) {
 	}
 	c.Value = lnwire.MilliSatoshi(byteOrder.Uint64(scratch[:]))
 
-	if _, err := io.ReadFull(r, scratch[:]); err != nil {
+	creationTime, err := deserializeTime(r)
+	if err != nil {
 		return nil, err
 	}
-	c.CreationTime = time.Unix(int64(byteOrder.Uint64(scratch[:])), 0)
+	c.CreationTime = creationTime
 
 	if _, err := io.ReadFull(r, scratch[:4]); err != nil {
 		return nil, err
@@ -495,7 +495,7 @@ func deserializePaymentCreationInfo(r io.Reader) (*PaymentCreationInfo, error) {
 }
 
 func serializeHTLCAttemptInfo(w io.Writer, a *HTLCAttemptInfo) error {
-	if err := WriteElements(w, a.AttemptID, a.SessionKey); err != nil {
+	if err := WriteElements(w, a.SessionKey); err != nil {
 		return err
 	}
 
@@ -503,12 +503,12 @@ func serializeHTLCAttemptInfo(w io.Writer, a *HTLCAttemptInfo) error {
 		return err
 	}
 
-	return nil
+	return serializeTime(w, a.AttemptTime)
 }
 
 func deserializeHTLCAttemptInfo(r io.Reader) (*HTLCAttemptInfo, error) {
 	a := &HTLCAttemptInfo{}
-	err := ReadElements(r, &a.AttemptID, &a.SessionKey)
+	err := ReadElements(r, &a.SessionKey)
 	if err != nil {
 		return nil, err
 	}
@@ -516,6 +516,12 @@ func deserializeHTLCAttemptInfo(r io.Reader) (*HTLCAttemptInfo, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	a.AttemptTime, err = deserializeTime(r)
+	if err != nil {
+		return nil, err
+	}
+
 	return a, nil
 }
 

channeldb/payments_test.go

@@ -70,6 +70,7 @@ func makeFakeInfo() (*PaymentCreationInfo, *HTLCAttemptInfo) {
 		AttemptID:   44,
 		SessionKey:  priv,
 		Route:       testRoute,
+		AttemptTime: time.Unix(100, 0),
 	}
 	return c, a
 }
@@ -113,29 +114,30 @@ func TestSentPaymentSerialization(t *testing.T) {
 		t.Fatalf("unable to serialize info: %v", err)
 	}
 
-	newAttemptInfo, err := deserializeHTLCAttemptInfo(&b)
+	newWireInfo, err := deserializeHTLCAttemptInfo(&b)
 	if err != nil {
 		t.Fatalf("unable to deserialize info: %v", err)
 	}
+	newWireInfo.AttemptID = s.AttemptID
 
 	// First we verify all the records match up porperly, as they aren't
 	// able to be properly compared using reflect.DeepEqual.
-	err = assertRouteEqual(&s.Route, &newAttemptInfo.Route)
+	err = assertRouteEqual(&s.Route, &newWireInfo.Route)
 	if err != nil {
 		t.Fatalf("Routes do not match after "+
 			"serialization/deserialization: %v", err)
 	}
 
 	// Clear routes to allow DeepEqual to compare the remaining fields.
-	newAttemptInfo.Route = route.Route{}
+	newWireInfo.Route = route.Route{}
 	s.Route = route.Route{}
 
-	if !reflect.DeepEqual(s, newAttemptInfo) {
+	if !reflect.DeepEqual(s, newWireInfo) {
 		s.SessionKey.Curve = nil
-		newAttemptInfo.SessionKey.Curve = nil
+		newWireInfo.SessionKey.Curve = nil
 		t.Fatalf("Payments do not match after "+
 			"serialization/deserialization %v vs %v",
-			spew.Sdump(s), spew.Sdump(newAttemptInfo),
+			spew.Sdump(s), spew.Sdump(newWireInfo),
 		)
 	}
 }

routing/control_tower.go

@@ -14,7 +14,6 @@ import (
 // restarts. Payments are transitioned through various payment states, and the
 // ControlTower interface provides access to driving the state transitions.
 type ControlTower interface {
-	// InitPayment atomically moves the payment into the InFlight state.
 	// This method checks that no suceeded payment exist for this payment
 	// hash.
 	InitPayment(lntypes.Hash, *channeldb.PaymentCreationInfo) error
@@ -22,17 +21,25 @@ type ControlTower interface {
 	// RegisterAttempt atomically records the provided HTLCAttemptInfo.
 	RegisterAttempt(lntypes.Hash, *channeldb.HTLCAttemptInfo) error
 
-	// Success transitions a payment into the Succeeded state. After
+	// SettleAttempt marks the given attempt settled with the preimage. If
-	// invoking this method, InitPayment should always return an error to
+	// this is a multi shard payment, this might implicitly mean the the
-	// prevent us from making duplicate payments to the same payment hash.
+	// full payment succeeded.
-	// The provided preimage is atomically saved to the DB for record
+	//
-	// keeping.
+	// After invoking this method, InitPayment should always return an
-	Success(lntypes.Hash, lntypes.Preimage) error
+	// error to prevent us from making duplicate payments to the same
+	// payment hash. The provided preimage is atomically saved to the DB
+	// for record keeping.
+	SettleAttempt(lntypes.Hash, uint64, *channeldb.HTLCSettleInfo) error
+
+	// FailAttempt marks the given payment attempt failed.
+	FailAttempt(lntypes.Hash, uint64, *channeldb.HTLCFailInfo) error
 
 	// Fail transitions a payment into the Failed state, and records the
-	// reason the payment failed. After invoking this method, InitPayment
+	// ultimate reason the payment failed. Note that this should only be
-	// should return nil on its next call for this payment hash, allowing
+	// called when all active active attempts are already failed. After
-	// the switch to make a subsequent payment.
+	// invoking this method, InitPayment should return nil on its next call
+	// for this payment hash, allowing the user to make a subsequent
+	// payment.
 	Fail(lntypes.Hash, channeldb.FailureReason) error
 
 	// FetchInFlightPayments returns all payments with status InFlight.
@@ -99,14 +106,13 @@ func (p *controlTower) RegisterAttempt(paymentHash lntypes.Hash,
 	return p.db.RegisterAttempt(paymentHash, attempt)
 }
 
-// Success transitions a payment into the Succeeded state. After invoking this
+// SettleAttempt marks the given attempt settled with the preimage. If
-// method, InitPayment should always return an error to prevent us from making
+// this is a multi shard payment, this might implicitly mean the the
-// duplicate payments to the same payment hash. The provided preimage is
+// full payment succeeded.
-// atomically saved to the DB for record keeping.
+func (p *controlTower) SettleAttempt(paymentHash lntypes.Hash,
-func (p *controlTower) Success(paymentHash lntypes.Hash,
+	attemptID uint64, settleInfo *channeldb.HTLCSettleInfo) error {
-	preimage lntypes.Preimage) error {
 
-	payment, err := p.db.Success(paymentHash, preimage)
+	payment, err := p.db.SettleAttempt(paymentHash, attemptID, settleInfo)
 	if err != nil {
 		return err
 	}
@@ -119,6 +125,13 @@ func (p *controlTower) Success(paymentHash lntypes.Hash,
 	return nil
 }
 
+// FailAttempt marks the given payment attempt failed.
+func (p *controlTower) FailAttempt(paymentHash lntypes.Hash,
+	attemptID uint64, failInfo *channeldb.HTLCFailInfo) error {
+
+	return p.db.FailAttempt(paymentHash, attemptID, failInfo)
+}
+
 // createSuccessResult creates a success result to send to subscribers.
 func createSuccessResult(htlcs []channeldb.HTLCAttempt) *PaymentResult {
 	// Extract any preimage from the list of HTLCs.

routing/control_tower_test.go

@@ -13,9 +13,8 @@ import (
 	"github.com/btcsuite/btcd/btcec"
 	"github.com/davecgh/go-spew/spew"
 	"github.com/lightningnetwork/lnd/channeldb"
-	"github.com/lightningnetwork/lnd/routing/route"
-
 	"github.com/lightningnetwork/lnd/lntypes"
+	"github.com/lightningnetwork/lnd/routing/route"
 )
 
 var (
@@ -111,7 +110,13 @@ func TestControlTowerSubscribeSuccess(t *testing.T) {
 	}
 
 	// Mark the payment as successful.
-	if err := pControl.Success(info.PaymentHash, preimg); err != nil {
+	err = pControl.SettleAttempt(
+		info.PaymentHash, attempt.AttemptID,
+		&channeldb.HTLCSettleInfo{
+			Preimage: preimg,
+		},
+	)
+	if err != nil {
 		t.Fatal(err)
 	}
 
@@ -213,6 +218,15 @@ func testPaymentControlSubscribeFail(t *testing.T, registerAttempt bool) {
 		if err != nil {
 			t.Fatal(err)
 		}
+
+		// Fail the payment attempt.
+		err := pControl.FailAttempt(
+			info.PaymentHash, attempt.AttemptID,
+			&channeldb.HTLCFailInfo{},
+		)
+		if err != nil {
+			t.Fatalf("unable to fail htlc: %v", err)
+		}
 	}
 
 	// Mark the payment as failed.

routing/mock_test.go

@@ -232,6 +232,7 @@ func (m *mockControlTower) InitPayment(phash lntypes.Hash,
 
 	m.inflights[phash] = channeldb.InFlightPayment{
 		Info:     c,
+		Attempts: make([]channeldb.HTLCAttemptInfo, 0),
 	}
 
 	return nil
@@ -252,20 +253,20 @@ func (m *mockControlTower) RegisterAttempt(phash lntypes.Hash,
 		return fmt.Errorf("not in flight")
 	}
 
-	p.Attempt = a
+	p.Attempts = append(p.Attempts, *a)
 	m.inflights[phash] = p
 
 	return nil
 }
 
-func (m *mockControlTower) Success(phash lntypes.Hash,
+func (m *mockControlTower) SettleAttempt(phash lntypes.Hash,
-	preimg lntypes.Preimage) error {
+	pid uint64, settleInfo *channeldb.HTLCSettleInfo) error {
 
 	m.Lock()
 	defer m.Unlock()
 
 	if m.success != nil {
-		m.success <- successArgs{preimg}
+		m.success <- successArgs{settleInfo.Preimage}
 	}
 
 	delete(m.inflights, phash)
@@ -310,3 +311,9 @@ func (m *mockControlTower) SubscribePayment(paymentHash lntypes.Hash) (
 
 	return false, nil, errors.New("not implemented")
 }
+
+func (m *mockControlTower) FailAttempt(hash lntypes.Hash, pid uint64,
+	failInfo *channeldb.HTLCFailInfo) error {
+
+	return nil
+}

routing/payment_lifecycle.go

@@ -58,6 +58,13 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 			// the DB, we send it.
 			sendErr := p.sendPaymentAttempt(firstHop, htlcAdd)
 			if sendErr != nil {
+				// TODO(joostjager): Distinguish unexpected
+				// internal errors from real send errors.
+				err = p.failAttempt()
+				if err != nil {
+					return [32]byte{}, nil, err
+				}
+
 				// We must inspect the error to know whether it
 				// was critical or not, to decide whether we
 				// should continue trying.
@@ -110,6 +117,11 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 				"the Switch, retrying.", p.attempt.AttemptID,
 				p.payment.PaymentHash)
 
+			err = p.failAttempt()
+			if err != nil {
+				return [32]byte{}, nil, err
+			}
+
 			// Reset the attempt to indicate we want to make a new
 			// attempt.
 			p.attempt = nil
@@ -146,6 +158,11 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 			log.Errorf("Attempt to send payment %x failed: %v",
 				p.payment.PaymentHash, result.Error)
 
+			err = p.failAttempt()
+			if err != nil {
+				return [32]byte{}, nil, err
+			}
+
 			// We must inspect the error to know whether it was
 			// critical or not, to decide whether we should
 			// continue trying.
@@ -174,7 +191,13 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 
 		// In case of success we atomically store the db payment and
 		// move the payment to the success state.
-		err = p.router.cfg.Control.Success(p.payment.PaymentHash, result.Preimage)
+		err = p.router.cfg.Control.SettleAttempt(
+			p.payment.PaymentHash, p.attempt.AttemptID,
+			&channeldb.HTLCSettleInfo{
+				Preimage:   result.Preimage,
+				SettleTime: p.router.cfg.Clock.Now(),
+			},
+		)
 		if err != nil {
 			log.Errorf("Unable to succeed payment "+
 				"attempt: %v", err)
@@ -340,6 +363,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID,
 	// the current attempt information.
 	p.attempt = &channeldb.HTLCAttemptInfo{
 		AttemptID:   attemptID,
+		AttemptTime: p.router.cfg.Clock.Now(),
 		SessionKey:  sessionKey,
 		Route:       *rt,
 	}
@@ -421,3 +445,15 @@ func (p *paymentLifecycle) handleSendError(sendErr error) error {
 	// Terminal state, return the error we encountered.
 	return sendErr
 }
+
+// failAttempt calls control tower to fail the current payment attempt.
+func (p *paymentLifecycle) failAttempt() error {
+	failInfo := &channeldb.HTLCFailInfo{
+		FailTime: p.router.cfg.Clock.Now(),
+	}
+
+	return p.router.cfg.Control.FailAttempt(
+		p.payment.PaymentHash, p.attempt.AttemptID,
+		failInfo,
+	)
+}

routing/router.go

@@ -541,7 +541,14 @@ func (r *ChannelRouter) Start() error {
 				PaymentHash: payment.Info.PaymentHash,
 			}
 
-			_, _, err := r.sendPayment(payment.Attempt, lPayment, paySession)
+			// TODO(joostjager): For mpp, possibly relaunch multiple
+			// in-flight htlcs here.
+			var attempt *channeldb.HTLCAttemptInfo
+			if len(payment.Attempts) > 0 {
+				attempt = &payment.Attempts[0]
+			}
+
+			_, _, err := r.sendPayment(attempt, lPayment, paySession)
 			if err != nil {
 				log.Errorf("Resuming payment with hash %v "+
 					"failed: %v.", payment.Info.PaymentHash, err)