From 1b788904f0d2d4c10b01632a6cd9b4475a614383 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Thu, 23 May 2019 20:05:30 +0200 Subject: [PATCH] channeldb+router: record payment failure reason in db --- channeldb/control_tower.go | 31 ++++++++++++++------ channeldb/control_tower_test.go | 52 ++++++++++++++++++++++++++++----- channeldb/payments.go | 34 +++++++++++++++++++-- routing/mock_test.go | 6 ++-- routing/payment_lifecycle.go | 6 ++-- 5 files changed, 106 insertions(+), 23 deletions(-) diff --git a/channeldb/control_tower.go b/channeldb/control_tower.go index 34136aa2..44611d60 100644 --- a/channeldb/control_tower.go +++ b/channeldb/control_tower.go @@ -56,10 +56,11 @@ type ControlTower interface { // keeping. Success(lntypes.Hash, lntypes.Preimage) error - // Fail transitions a payment into the Failed state. After invoking - // this method, InitPayment should return nil on its next call for this - // payment hash, allowing the switch to make a subsequent payment. - Fail(lntypes.Hash) error + // Fail transitions a payment into the Failed state, and records the + // reason the payment failed. After invoking this method, InitPayment + // should return nil on its next call for this payment hash, allowing + // the switch to make a subsequent payment. + Fail(lntypes.Hash, FailureReason) error // FetchInFlightPayments returns all payments with status InFlight. FetchInFlightPayments() ([]*InFlightPayment, error) @@ -165,7 +166,9 @@ func (p *paymentControl) InitPayment(paymentHash lntypes.Hash, return err } - return nil + // Also delete any lingering failure info now that we are + // re-attempting. + return bucket.Delete(paymentFailInfoKey) }) if err != nil { return nil @@ -255,10 +258,13 @@ func (p *paymentControl) Success(paymentHash lntypes.Hash, } -// Fail transitions a payment into the Failed state. After invoking this -// method, InitPayment should return nil on its next call for this payment -// hash, allowing the switch to make a subsequent payment. -func (p *paymentControl) Fail(paymentHash lntypes.Hash) error { +// Fail transitions a payment into the Failed state, and records the reason the +// payment failed. After invoking this method, InitPayment should return nil on +// its next call for this payment hash, allowing the switch to make a +// subsequent payment. +func (p *paymentControl) Fail(paymentHash lntypes.Hash, + reason FailureReason) error { + var updateErr error err := p.db.Batch(func(tx *bbolt.Tx) error { // Reset the update error, to avoid carrying over an error @@ -276,6 +282,13 @@ func (p *paymentControl) Fail(paymentHash lntypes.Hash) error { return nil } + // Put the failure reason in the bucket for record keeping. + v := []byte{byte(reason)} + err = bucket.Put(paymentFailInfoKey, v) + if err != nil { + return err + } + // A failed response was received for an InFlight payment, mark // it as Failed to allow subsequent attempts. return bucket.Put(paymentStatusKey, StatusFailed.Bytes()) diff --git a/channeldb/control_tower_test.go b/channeldb/control_tower_test.go index 612a058e..e8951831 100644 --- a/channeldb/control_tower_test.go +++ b/channeldb/control_tower_test.go @@ -88,10 +88,13 @@ func TestPaymentControlSwitchFail(t *testing.T) { assertPaymentStatus(t, db, info.PaymentHash, StatusInFlight) assertPaymentInfo( t, db, info.PaymentHash, info, nil, lntypes.Preimage{}, + nil, ) // Fail the payment, which should moved it to Failed. - if err := pControl.Fail(info.PaymentHash); err != nil { + failReason := FailureReasonNoRoute + err = pControl.Fail(info.PaymentHash, failReason) + if err != nil { t.Fatalf("unable to fail payment hash: %v", err) } @@ -99,6 +102,7 @@ func TestPaymentControlSwitchFail(t *testing.T) { assertPaymentStatus(t, db, info.PaymentHash, StatusFailed) assertPaymentInfo( t, db, info.PaymentHash, info, nil, lntypes.Preimage{}, + &failReason, ) // Sends the htlc again, which should succeed since the prior payment @@ -111,6 +115,7 @@ func TestPaymentControlSwitchFail(t *testing.T) { assertPaymentStatus(t, db, info.PaymentHash, StatusInFlight) assertPaymentInfo( t, db, info.PaymentHash, info, nil, lntypes.Preimage{}, + nil, ) // Record a new attempt. @@ -122,6 +127,7 @@ func TestPaymentControlSwitchFail(t *testing.T) { assertPaymentStatus(t, db, info.PaymentHash, StatusInFlight) assertPaymentInfo( t, db, info.PaymentHash, info, attempt, lntypes.Preimage{}, + nil, ) // Verifies that status was changed to StatusCompleted. @@ -130,7 +136,7 @@ func TestPaymentControlSwitchFail(t *testing.T) { } assertPaymentStatus(t, db, info.PaymentHash, StatusCompleted) - assertPaymentInfo(t, db, info.PaymentHash, info, attempt, preimg) + assertPaymentInfo(t, db, info.PaymentHash, info, attempt, preimg, nil) // Attempt a final payment, which should now fail since the prior // payment succeed. @@ -167,6 +173,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) { assertPaymentStatus(t, db, info.PaymentHash, StatusInFlight) assertPaymentInfo( t, db, info.PaymentHash, info, nil, lntypes.Preimage{}, + nil, ) // Try to initiate double sending of htlc message with the same @@ -186,6 +193,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) { assertPaymentStatus(t, db, info.PaymentHash, StatusInFlight) assertPaymentInfo( t, db, info.PaymentHash, info, attempt, lntypes.Preimage{}, + nil, ) // Sends base htlc message which initiate StatusInFlight. @@ -200,7 +208,7 @@ func TestPaymentControlSwitchDoubleSend(t *testing.T) { t.Fatalf("error shouldn't have been received, got: %v", err) } assertPaymentStatus(t, db, info.PaymentHash, StatusCompleted) - assertPaymentInfo(t, db, info.PaymentHash, info, attempt, preimg) + assertPaymentInfo(t, db, info.PaymentHash, info, attempt, preimg, nil) err = pControl.InitPayment(info.PaymentHash, info) if err != ErrAlreadyPaid { @@ -232,7 +240,10 @@ func TestPaymentControlSuccessesWithoutInFlight(t *testing.T) { } assertPaymentStatus(t, db, info.PaymentHash, StatusGrounded) - assertPaymentInfo(t, db, info.PaymentHash, nil, nil, lntypes.Preimage{}) + assertPaymentInfo( + t, db, info.PaymentHash, nil, nil, lntypes.Preimage{}, + nil, + ) } // TestPaymentControlFailsWithoutInFlight checks that a strict payment @@ -253,13 +264,15 @@ func TestPaymentControlFailsWithoutInFlight(t *testing.T) { } // Calling Fail should return an error. - err = pControl.Fail(info.PaymentHash) + err = pControl.Fail(info.PaymentHash, FailureReasonNoRoute) if err != ErrPaymentNotInitiated { t.Fatalf("expected ErrPaymentNotInitiated, got %v", err) } assertPaymentStatus(t, db, info.PaymentHash, StatusGrounded) - assertPaymentInfo(t, db, info.PaymentHash, nil, nil, lntypes.Preimage{}) + assertPaymentInfo( + t, db, info.PaymentHash, nil, nil, lntypes.Preimage{}, nil, + ) } func assertPaymentStatus(t *testing.T, db *DB, @@ -364,8 +377,29 @@ func checkSettleInfo(bucket *bbolt.Bucket, preimg lntypes.Preimage) error { return nil } +func checkFailInfo(bucket *bbolt.Bucket, failReason *FailureReason) error { + b := bucket.Get(paymentFailInfoKey) + switch { + case b == nil && failReason == nil: + return nil + case b == nil: + return fmt.Errorf("expected fail info not found") + case failReason == nil: + return fmt.Errorf("unexpected fail info found") + } + + failReason2 := FailureReason(b[0]) + if *failReason != failReason2 { + return fmt.Errorf("Failure infos don't match: %v vs %v", + *failReason, failReason2) + } + + return nil +} + func assertPaymentInfo(t *testing.T, db *DB, hash lntypes.Hash, - c *PaymentCreationInfo, a *PaymentAttemptInfo, s lntypes.Preimage) { + c *PaymentCreationInfo, a *PaymentAttemptInfo, s lntypes.Preimage, + f *FailureReason) { t.Helper() @@ -398,6 +432,10 @@ func assertPaymentInfo(t *testing.T, db *DB, hash lntypes.Hash, if err := checkSettleInfo(bucket, s); err != nil { return err } + + if err := checkFailInfo(bucket, f); err != nil { + return err + } return nil }) if err != nil { diff --git a/channeldb/payments.go b/channeldb/payments.go index fff9fcdc..036f25ff 100644 --- a/channeldb/payments.go +++ b/channeldb/payments.go @@ -81,6 +81,10 @@ var ( // store the settle info of the payment. paymentSettleInfoKey = []byte("payment-settle-info") + // paymentFailInfoKey is a key used in the payment's sub-bucket to + // store information about the reason a payment failed. + paymentFailInfoKey = []byte("payment-fail-info") + // paymentBucket is the name of the bucket within the database that // stores all data related to payments. // @@ -94,6 +98,21 @@ var ( paymentStatusBucket = []byte("payment-status") ) +// FailureReason encodes the reason a payment ultimately failed. +type FailureReason byte + +const ( + // FailureReasonTimeout indicates that the payment did timeout before a + // successful payment attempt was made. + FailureReasonTimeout FailureReason = 0 + + // FailureReasonNoRoute indicates no successful route to the + // destination was found during path finding. + FailureReasonNoRoute FailureReason = 1 + + // TODO(halseth): cancel state. +) + // PaymentStatus represent current status of payment type PaymentStatus byte @@ -113,8 +132,6 @@ const ( // StatusFailed is the status where a payment has been initiated and a // failure result has come back. StatusFailed PaymentStatus = 3 - - // TODO(halseth): timeout/cancel state? ) // Bytes returns status as slice of bytes. @@ -454,6 +471,12 @@ type Payment struct { // // NOTE: Can be nil if payment is not settled. PaymentPreimage *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 } // FetchPayments returns all sent payments found in the DB. @@ -570,6 +593,13 @@ func fetchPayment(bucket *bbolt.Bucket) (*Payment, error) { p.PaymentPreimage = &preimg } + // Get failure reason if available. + b = bucket.Get(paymentFailInfoKey) + if b != nil { + reason := FailureReason(b[0]) + p.Failure = &reason + } + return p, nil } diff --git a/routing/mock_test.go b/routing/mock_test.go index b8495428..6d98dd6e 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -167,6 +167,7 @@ type successArgs struct { } type failArgs struct { + reason channeldb.FailureReason } type mockControlTower struct { @@ -253,13 +254,14 @@ func (m *mockControlTower) Success(phash lntypes.Hash, return nil } -func (m *mockControlTower) Fail(phash lntypes.Hash) error { +func (m *mockControlTower) Fail(phash lntypes.Hash, + reason channeldb.FailureReason) error { m.Lock() defer m.Unlock() if m.fail != nil { - m.fail <- failArgs{} + m.fail <- failArgs{reason} } delete(m.inflights, phash) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 29b4bf1f..38883ca8 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -177,7 +177,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // Mark the payment as failed because of the // timeout. err := p.router.cfg.Control.Fail( - p.payment.PaymentHash, + p.payment.PaymentHash, channeldb.FailureReasonTimeout, ) if err != nil { return lnwire.ShortChannelID{}, nil, err @@ -208,7 +208,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // any of the routes we've found, then mark the payment // as permanently failed. saveErr := p.router.cfg.Control.Fail( - p.payment.PaymentHash, + p.payment.PaymentHash, channeldb.FailureReasonNoRoute, ) if saveErr != nil { return lnwire.ShortChannelID{}, nil, saveErr @@ -338,7 +338,7 @@ func (p *paymentLifecycle) handleSendError(sendErr error) error { // TODO(halseth): make payment codes for the actual reason we // don't continue path finding. err := p.router.cfg.Control.Fail( - p.payment.PaymentHash, + p.payment.PaymentHash, channeldb.FailureReasonNoRoute, ) if err != nil { return err