diff --git a/channeldb/payment_control_test.go b/channeldb/payment_control_test.go index ffce597f9..b80f18519 100644 --- a/channeldb/payment_control_test.go +++ b/channeldb/payment_control_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/rand" "crypto/sha256" + "errors" "fmt" "io" "reflect" @@ -67,7 +68,9 @@ func TestPaymentControlSwitchFail(t *testing.T) { require.NoError(t, err, "unable to send htlc message") assertPaymentIndex(t, pControl, info.PaymentIdentifier) - assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusInFlight) + assertPaymentStatus( + t, pControl, info.PaymentIdentifier, StatusInitiated, + ) assertPaymentInfo( t, pControl, info.PaymentIdentifier, info, nil, nil, ) @@ -98,7 +101,9 @@ func TestPaymentControlSwitchFail(t *testing.T) { assertPaymentIndex(t, pControl, info.PaymentIdentifier) assertNoIndex(t, pControl, payment.SequenceNum) - assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusInFlight) + assertPaymentStatus( + t, pControl, info.PaymentIdentifier, StatusInitiated, + ) assertPaymentInfo( t, pControl, info.PaymentIdentifier, info, nil, nil, ) @@ -119,7 +124,9 @@ func TestPaymentControlSwitchFail(t *testing.T) { if err != nil { t.Fatal(err) } - assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusInFlight) + assertPaymentStatus( + t, pControl, info.PaymentIdentifier, StatusInFlight, + ) htlc := &htlcStatus{ HTLCAttemptInfo: attempt, @@ -198,7 +205,9 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) { require.NoError(t, err, "unable to send htlc message") assertPaymentIndex(t, pControl, info.PaymentIdentifier) - assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusInFlight) + assertPaymentStatus( + t, pControl, info.PaymentIdentifier, StatusInitiated, + ) assertPaymentInfo( t, pControl, info.PaymentIdentifier, info, nil, nil, ) @@ -207,10 +216,9 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) { // payment hash, should result in error indicating that payment has // already been sent. err = pControl.InitPayment(info.PaymentIdentifier, info) - if err != ErrPaymentInFlight { - t.Fatalf("payment control wrong behaviour: " + - "double sending must trigger ErrPaymentInFlight error") - } + require.Equal(t, ErrPaymentExists, err, "payment control wrong "+ + "behaviour: init payment again must trigger ErrPaymentExists "+ + "error") // Record an attempt. _, err = pControl.RegisterAttempt(info.PaymentIdentifier, attempt) @@ -682,7 +690,9 @@ func TestPaymentControlMultiShard(t *testing.T) { } assertPaymentIndex(t, pControl, info.PaymentIdentifier) - assertPaymentStatus(t, pControl, info.PaymentIdentifier, StatusInFlight) + assertPaymentStatus( + t, pControl, info.PaymentIdentifier, StatusInitiated, + ) assertPaymentInfo( t, pControl, info.PaymentIdentifier, info, nil, nil, ) @@ -708,7 +718,8 @@ func TestPaymentControlMultiShard(t *testing.T) { t.Fatalf("unable to send htlc message: %v", err) } assertPaymentStatus( - t, pControl, info.PaymentIdentifier, StatusInFlight, + t, pControl, info.PaymentIdentifier, + StatusInFlight, ) htlc := &htlcStatus{ @@ -807,11 +818,14 @@ func TestPaymentControlMultiShard(t *testing.T) { // 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.PaymentIdentifier, StatusInFlight) + // The payment is now considered pending fail, since + // there is still an active HTLC. + assertPaymentStatus( + t, pControl, info.PaymentIdentifier, + StatusInFlight, + ) + } // Try to register yet another attempt. This should fail now // that the payment has reached a terminal condition. @@ -839,15 +853,12 @@ func TestPaymentControlMultiShard(t *testing.T) { Preimage: preimg, }, ) - if err != nil { - t.Fatalf("error shouldn't have been "+ - "received, got: %v", err) - } + require.NoError(t, err, "unable to settle") htlc.settle = &preimg assertPaymentInfo( - t, pControl, info.PaymentIdentifier, info, - firstFailReason, htlc, + t, pControl, info.PaymentIdentifier, + info, firstFailReason, htlc, ) } else { // Fail the attempt. @@ -875,9 +886,7 @@ func TestPaymentControlMultiShard(t *testing.T) { // syncing. failReason := FailureReasonPaymentDetails _, err = pControl.Fail(info.PaymentIdentifier, failReason) - if err != nil { - t.Fatalf("unable to fail payment hash: %v", err) - } + require.NoError(t, err, "unable to fail") } var ( @@ -1105,7 +1114,7 @@ func assertPaymentStatus(t *testing.T, p *PaymentControl, t.Helper() payment, err := p.FetchPayment(hash) - if err == ErrPaymentNotInitiated { + if errors.Is(err, ErrPaymentNotInitiated) { return } if err != nil { diff --git a/channeldb/payment_status.go b/channeldb/payment_status.go index a6c7c1038..45018449a 100644 --- a/channeldb/payment_status.go +++ b/channeldb/payment_status.go @@ -26,6 +26,9 @@ const ( StatusFailed PaymentStatus = 4 ) +// errPaymentStatusUnknown is returned when a payment has an unknown status. +var errPaymentStatusUnknown = fmt.Errorf("unknown payment status") + // String returns readable representation of payment status. func (ps PaymentStatus) String() string { switch ps { @@ -127,3 +130,123 @@ func (ps PaymentStatus) updatable() error { return fmt.Errorf("%w: %v", ErrUnknownPaymentStatus, ps) } } + +// decidePaymentStatus uses the payment's DB state to determine a memory status +// that's used by the payment router to decide following actions. +// Together, we use four variables to determine the payment's status, +// - inflight: whether there are any pending HTLCs. +// - settled: whether any of the HTLCs has been settled. +// - htlc failed: whether any of the HTLCs has been failed. +// - payment failed: whether the payment has been marked as failed. +// +// Based on the above variables, we derive the status using the following +// table, +// | inflight | settled | htlc failed | payment failed | status | +// |:--------:|:-------:|:-----------:|:--------------:|:--------------------:| +// | true | true | true | true | StatusInFlight | +// | true | true | true | false | StatusInFlight | +// | true | true | false | true | StatusInFlight | +// | true | true | false | false | StatusInFlight | +// | true | false | true | true | StatusInFlight | +// | true | false | true | false | StatusInFlight | +// | true | false | false | true | StatusInFlight | +// | true | false | false | false | StatusInFlight | +// | false | true | true | true | StatusSucceeded | +// | false | true | true | false | StatusSucceeded | +// | false | true | false | true | StatusSucceeded | +// | false | true | false | false | StatusSucceeded | +// | false | false | true | true | StatusFailed | +// | false | false | true | false | StatusInFlight | +// | false | false | false | true | StatusFailed | +// | false | false | false | false | StatusInitiated | +// +// When `inflight`, `settled`, `htlc failed`, and `payment failed` are false, +// this indicates the payment is newly created and hasn't made any HTLCs yet. +// When `inflight` and `settled` are false, `htlc failed` is true yet `payment +// failed` is false, this indicates all the payment's HTLCs have occurred a +// temporarily failure and the payment is still in-flight. +func decidePaymentStatus(htlcs []HTLCAttempt, + reason *FailureReason) (PaymentStatus, error) { + + var ( + inflight bool + htlcSettled bool + htlcFailed bool + paymentFailed bool + ) + + // If we have a failure reason, the payment is failed. + if reason != nil { + paymentFailed = true + } + + // Go through all HTLCs for this payment, check whether we have any + // settled HTLC, and any still in-flight. + for _, h := range htlcs { + if h.Failure != nil { + htlcFailed = true + continue + } + + if h.Settle != nil { + htlcSettled = 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. + switch { + // If we have inflight HTLCs, no matter we have settled or failed + // HTLCs, or the payment failed, we still consider it inflight so we + // inform upper systems to wait for the results. + case inflight: + return StatusInFlight, nil + + // If we have no in-flight HTLCs, and at least one of the HTLCs is + // settled, the payment succeeded. + // + // NOTE: when reaching this case, paymentFailed could be true, which + // means we have a conflicting state for this payment. We choose to + // mark the payment as succeeded because it's the receiver's + // responsibility to only settle the payment iff all HTLCs are + // received. + case htlcSettled: + return StatusSucceeded, nil + + // If we have no in-flight HTLCs, and the payment failure is set, the + // payment is considered failed. + // + // NOTE: when reaching this case, settled must be false. + case paymentFailed: + return StatusFailed, nil + + // If we have no in-flight HTLCs, yet the payment is NOT failed, it + // means all the HTLCs are failed. In this case we can attempt more + // HTLCs. + // + // NOTE: when reaching this case, both settled and paymentFailed must + // be false. + case htlcFailed: + return StatusInFlight, nil + + // If none of the HTLCs is either settled or failed, and we have no + // inflight HTLCs, this means the payment has no HTLCs created yet. + // + // NOTE: when reaching this case, both settled and paymentFailed must + // be false. + case !htlcFailed: + return StatusInitiated, nil + + // Otherwise an impossible state is reached. + // + // NOTE: we should never end up here. + default: + log.Error("Impossible payment state reached") + return 0, fmt.Errorf("%w: payment is corrupted", + errPaymentStatusUnknown) + } +} diff --git a/channeldb/payments.go b/channeldb/payments.go index 168a355cf..c41ee3d17 100644 --- a/channeldb/payments.go +++ b/channeldb/payments.go @@ -298,41 +298,10 @@ func fetchPayment(bucket kvdb.RBucket) (*MPPayment, error) { 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 + // Now determine the payment's status. + paymentStatus, err := decidePaymentStatus(htlcs, failureReason) + if err != nil { + return nil, err } return &MPPayment{