channeldb+routing: refactor payment lifecycle

.golangci.yml

@@ -249,3 +249,10 @@ issues:
         # if the returned value doesn't match the type, so there's no need to
         # check the convert.
         - forcetypeassert
+
+    - path: mock*
+      linters:
+        # forcetypeassert is skipped for the mock because the test would fail
+        # if the returned value doesn't match the type, so there's no need to
+        # check the convert.
+        - forcetypeassert

channeldb/duplicate_payments.go

@@ -59,22 +59,22 @@ type duplicateHTLCAttemptInfo struct {
 	route route.Route
 }
 
-// fetchDuplicatePaymentStatus fetches the payment status of the payment. If the
-// payment isn't found, it will default to "StatusUnknown".
-func fetchDuplicatePaymentStatus(bucket kvdb.RBucket) PaymentStatus {
+// fetchDuplicatePaymentStatus fetches the payment status of the payment. If
+// the payment isn't found, it will return error `ErrPaymentNotInitiated`.
+func fetchDuplicatePaymentStatus(bucket kvdb.RBucket) (PaymentStatus, error) {
 	if bucket.Get(duplicatePaymentSettleInfoKey) != nil {
-		return StatusSucceeded
+		return StatusSucceeded, nil
 	}
 
 	if bucket.Get(duplicatePaymentFailInfoKey) != nil {
-		return StatusFailed
+		return StatusFailed, nil
 	}
 
 	if bucket.Get(duplicatePaymentCreationInfoKey) != nil {
-		return StatusInFlight
+		return StatusInFlight, nil
 	}
 
-	return StatusUnknown
+	return 0, ErrPaymentNotInitiated
 }
 
 func deserializeDuplicateHTLCAttemptInfo(r io.Reader) (
@@ -138,7 +138,10 @@ func fetchDuplicatePayment(bucket kvdb.RBucket) (*MPPayment, error) {
 	sequenceNum := binary.BigEndian.Uint64(seqBytes)
 
 	// Get the payment status.
-	paymentStatus := fetchDuplicatePaymentStatus(bucket)
+	paymentStatus, err := fetchDuplicatePaymentStatus(bucket)
+	if err != nil {
+		return nil, err
+	}
 
 	// Get the PaymentCreationInfo.
 	b := bucket.Get(duplicatePaymentCreationInfoKey)

channeldb/mp_payment.go

@@ -3,6 +3,7 @@ package channeldb
 import (
 	"bytes"
 	"errors"
+	"fmt"
 	"io"
 	"math"
 	"time"
@@ -46,22 +47,24 @@ type HTLCAttemptInfo struct {
 	Hash *lntypes.Hash
 }
 
-// NewHtlcAttemptInfo creates a htlc attempt.
-func NewHtlcAttemptInfo(attemptID uint64, sessionKey *btcec.PrivateKey,
+// NewHtlcAttempt creates a htlc attempt.
+func NewHtlcAttempt(attemptID uint64, sessionKey *btcec.PrivateKey,
 	route route.Route, attemptTime time.Time,
-	hash *lntypes.Hash) *HTLCAttemptInfo {
+	hash *lntypes.Hash) *HTLCAttempt {
 
 	var scratch [btcec.PrivKeyBytesLen]byte
 	copy(scratch[:], sessionKey.Serialize())
 
-	return &HTLCAttemptInfo{
+	info := HTLCAttemptInfo{
 		AttemptID:        attemptID,
 		sessionKey:       scratch,
 		cachedSessionKey: sessionKey,
 		Route:            route,
 		AttemptTime:      attemptTime,
 		Hash:             hash,
 	}
+
+	return &HTLCAttempt{HTLCAttemptInfo: info}
 }
 
 // SessionKey returns the ephemeral key used for a htlc attempt. This function
@@ -147,6 +150,30 @@ type HTLCFailInfo struct {
 	FailureSourceIndex uint32
 }
 
+// MPPaymentState wraps a series of info needed for a given payment. This is a
+// memory representation of the payment's current state and is updated whenever
+// the payment is read from disk.
+type MPPaymentState struct {
+	// NumAttemptsInFlight specifies the number of HTLCs the payment is
+	// waiting results for.
+	NumAttemptsInFlight int
+
+	// RemainingAmt specifies how much more money to be sent.
+	RemainingAmt lnwire.MilliSatoshi
+
+	// FeesPaid specifies the total fees paid so far that can be used to
+	// calculate remaining fee budget.
+	FeesPaid lnwire.MilliSatoshi
+
+	// HasSettledHTLC is true if at least one of the payment's HTLCs is
+	// settled.
+	HasSettledHTLC bool
+
+	// PaymentFailed is true if the payment has been marked as failed with
+	// a reason.
+	PaymentFailed bool
+}
+
 // MPPayment is a wrapper around a payment's PaymentCreationInfo and
 // HTLCAttempts. All payments will have the PaymentCreationInfo set, any
 // HTLCs made in attempts to be completed will populated in the HTLCs slice.
@@ -175,15 +202,25 @@ type MPPayment struct {
 
 	// Status is the current PaymentStatus of this payment.
 	Status PaymentStatus
+
+	// State is the current state of the payment.
+	State *MPPaymentState
+}
+
+// Terminated returns a bool to specify whether the payment is in a terminal
+// state.
+func (m *MPPayment) Terminated() bool {
+	// If the payment is in terminal state, it cannot be updated.
+	return m.Status.updatable() != nil
 }
 
 // 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) {
+func (m *MPPayment) TerminalInfo() (*HTLCAttempt, *FailureReason) {
 	for _, h := range m.HTLCs {
 		if h.Settle != nil {
-			return h.Settle, nil
+			return &h, nil
 		}
 	}
 
@@ -225,6 +262,7 @@ func (m *MPPayment) InFlightHTLCs() []HTLCAttempt {
 
 // GetAttempt returns the specified htlc attempt on the payment.
 func (m *MPPayment) GetAttempt(id uint64) (*HTLCAttempt, error) {
+	// TODO(yy): iteration can be slow, make it into a tree or use BS.
 	for _, htlc := range m.HTLCs {
 		htlc := htlc
 		if htlc.AttemptID == id {
@@ -235,6 +273,229 @@ func (m *MPPayment) GetAttempt(id uint64) (*HTLCAttempt, error) {
 	return nil, errors.New("htlc attempt not found on payment")
 }
 
+// Registrable returns an error to specify whether adding more HTLCs to the
+// payment with its current status is allowed. A payment can accept new HTLC
+// registration when it's newly created, or none of its HTLCs is in a terminal
+// state.
+func (m *MPPayment) Registrable() error {
+	// If updating the payment is not allowed, we can't register new HTLCs.
+	// Otherwise, the status must be either `StatusInitiated` or
+	// `StatusInFlight`.
+	if err := m.Status.updatable(); err != nil {
+		return err
+	}
+
+	// Exit early if this is not inflight.
+	if m.Status != StatusInFlight {
+		return nil
+	}
+
+	// There are still inflight HTLCs and we need to check whether there
+	// 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
+	}
+
+	// If the payment is already failed, we won't allow adding more HTLCs.
+	if m.State.PaymentFailed {
+		return ErrPaymentPendingFailed
+	}
+
+	// Otherwise we can add more HTLCs.
+	return nil
+}
+
+// updateState creates and attaches a new MPPaymentState to the payment. It
+// also updates the payment's status based on its current state.
+func (m *MPPayment) updateState() error {
+	// Fetch the total amount and fees that has already been sent in
+	// settled and still in-flight shards.
+	sentAmt, fees := m.SentAmt()
+
+	// 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)
+	}
+
+	// Get any terminal info for this payment.
+	settle, failure := m.TerminalInfo()
+
+	// Now determine the payment's status.
+	status, err := decidePaymentStatus(m.HTLCs, m.FailureReason)
+	if err != nil {
+		return err
+	}
+
+	// Update the payment state and status.
+	m.State = &MPPaymentState{
+		NumAttemptsInFlight: len(m.InFlightHTLCs()),
+		RemainingAmt:        totalAmt - sentAmt,
+		FeesPaid:            fees,
+		HasSettledHTLC:      settle != nil,
+		PaymentFailed:       failure != nil,
+	}
+	m.Status = status
+
+	return nil
+}
+
+// UpdateState calls the internal method updateState. This is a temporary
+// method to be used by the tests in routing. Once the tests are updated to use
+// mocks, this method can be removed.
+//
+// TODO(yy): delete.
+func (m *MPPayment) UpdateState() error {
+	return m.updateState()
+}
+
+// NeedWaitAttempts decides whether we need to hold creating more HTLC attempts
+// and wait for the results of the payment's inflight HTLCs. Return an error if
+// the payment is in an unexpected state.
+func (m *MPPayment) NeedWaitAttempts() (bool, error) {
+	// Check when the remainingAmt is not zero, which means we have more
+	// money to be sent.
+	if m.State.RemainingAmt != 0 {
+		switch m.Status {
+		// If the payment is newly created, no need to wait for HTLC
+		// results.
+		case StatusInitiated:
+			return false, nil
+
+		// If we have inflight HTLCs, we'll check if we have terminal
+		// states to decide if we need to wait.
+		case StatusInFlight:
+			// We still have money to send, yet one of the HTLCs is
+			// settled, return an error as the peer is violating
+			// the protocol.
+			if m.State.HasSettledHTLC {
+				return false, fmt.Errorf("%w: settled HTLC "+
+					"but still have remaining amount %v",
+					ErrPaymentInternal,
+					m.State.RemainingAmt)
+			}
+
+			// Otherwise we don't need to wait for inflight HTLCs
+			// since we still have money to be sent.
+			return false, nil
+
+		// We need to send more money, yet the payment is already
+		// succeeded. Return an error in this case as the peer is
+		// violating the protocol.
+		case StatusSucceeded:
+			return false, fmt.Errorf("%w: payment already "+
+				"succeeded but still have remaining amount %v",
+				ErrPaymentInternal, m.State.RemainingAmt)
+
+		// The payment is failed and we have no inflight HTLCs, no need
+		// to wait.
+		case StatusFailed:
+			return false, nil
+
+		// Unknown payment status.
+		default:
+			return false, fmt.Errorf("%w: %s",
+				ErrUnknownPaymentStatus, m.Status)
+		}
+	}
+
+	// Now we determine whether we need to wait when the remainingAmt is
+	// already zero.
+	switch m.Status {
+	// 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)
+
+	// If the payment is inflight, we must wait.
+	//
+	// NOTE: an edge case is when all HTLCs are failed while the payment is
+	// not failed we'd still be in this inflight state. However, since the
+	// remainingAmt is zero here, it means we cannot be in that state as
+	// otherwise the remainingAmt would not be zero.
+	case StatusInFlight:
+		return true, nil
+
+	// If the payment is already in a terminal state, no need to wait.
+	case StatusSucceeded:
+	case StatusFailed:
+
+	// Unknown payment status.
+	default:
+		return false, fmt.Errorf("%w: %s", ErrUnknownPaymentStatus,
+			m.Status)
+	}
+
+	return false, nil
+}
+
+// GetState returns the internal state of the payment.
+func (m *MPPayment) GetState() *MPPaymentState {
+	return m.State
+}
+
+// Status returns the current status of the payment.
+func (m *MPPayment) GetStatus() PaymentStatus {
+	return m.Status
+}
+
+// GetPayment returns all the HTLCs for this payment.
+func (m *MPPayment) GetHTLCs() []HTLCAttempt {
+	return m.HTLCs
+}
+
+// GetFailureReason returns the failure reason.
+func (m *MPPayment) GetFailureReason() *FailureReason {
+	return m.FailureReason
+}
+
+// AllowMoreAttempts is used to decide whether we can safely attempt more HTLCs
+// for a given payment state. Return an error if the payment is in an
+// unexpected state.
+func (m *MPPayment) AllowMoreAttempts() (bool, error) {
+	// Now check whether the remainingAmt is zero or not. If we don't have
+	// any remainingAmt, no more HTLCs should be made.
+	if m.State.RemainingAmt == 0 {
+		// If the payment is newly created, yet we don't have any
+		// remainingAmt, return an error.
+		if m.Status == StatusInitiated {
+			return false, fmt.Errorf("%w: initiated payment has "+
+				"zero remainingAmt", ErrPaymentInternal)
+		}
+
+		// Otherwise, exit early since all other statuses with zero
+		// remainingAmt indicates no more HTLCs can be made.
+		return false, nil
+	}
+
+	// Otherwise, the remaining amount is not zero, we now decide whether
+	// to make more attempts based on the payment's current status.
+	//
+	// If at least one of the payment's attempts is settled, yet we haven't
+	// sent all the amount, it indicates something is wrong with the peer
+	// 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)
+	}
+
+	// Now check if we can register a new HTLC.
+	err := m.Registrable()
+	if err != nil {
+		log.Warnf("Payment(%v): cannot register HTLC attempt: %v, "+
+			"current status: %s", m.Info.PaymentIdentifier,
+			err, m.Status)
+
+		return false, nil
+	}
+
+	// Now we know we can register new HTLCs.
+	return true, nil
+}
+
 // serializeHTLCSettleInfo serializes the details of a settled htlc.
 func serializeHTLCSettleInfo(w io.Writer, s *HTLCSettleInfo) error {
 	if _, err := w.Write(s.Preimage[:]); err != nil {