From c2301c14b2357edb33e86fbf7a7868862b1900d6 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:22 +0200 Subject: [PATCH 01/34] routing/payment_session: make NewPaymentSession take payment directly This commit moves supplying of the information in the LightningPayment to the initialization of the paymentSession, away from every call to RequestRoute. Instead the paymentSession will store this information internally, as it doesn't change between payment attempts. This is done to rid the RequestRoute call of the LightingPayment argument, as for SendToRoute calls, it is not needed to supply the next route. --- routing/mock_test.go | 9 +++------ routing/payment_lifecycle.go | 21 +++++++++----------- routing/payment_session.go | 33 ++++++++++++++++--------------- routing/payment_session_source.go | 7 ++++--- routing/payment_session_test.go | 23 ++++++++++----------- routing/router.go | 22 +++++++++------------ 6 files changed, 54 insertions(+), 61 deletions(-) diff --git a/routing/mock_test.go b/routing/mock_test.go index 6332bea8f..64a1e4f65 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -10,7 +10,6 @@ import ( "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" - "github.com/lightningnetwork/lnd/zpay32" ) type mockPaymentAttemptDispatcher struct { @@ -78,8 +77,8 @@ type mockPaymentSessionSource struct { var _ PaymentSessionSource = (*mockPaymentSessionSource)(nil) -func (m *mockPaymentSessionSource) NewPaymentSession(routeHints [][]zpay32.HopHint, - target route.Vertex) (PaymentSession, error) { +func (m *mockPaymentSessionSource) NewPaymentSession( + _ *LightningPayment) (PaymentSession, error) { return &mockPaymentSession{m.routes}, nil } @@ -123,9 +122,7 @@ type mockPaymentSession struct { var _ PaymentSession = (*mockPaymentSession)(nil) -func (m *mockPaymentSession) RequestRoute(payment *LightningPayment, - height uint32, finalCltvDelta uint16) (*route.Route, error) { - +func (m *mockPaymentSession) RequestRoute(height uint32) (*route.Route, error) { if len(m.routes) == 0 { return nil, fmt.Errorf("no routes") } diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 96441d179..58bafc7d9 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -29,15 +29,14 @@ func (e errNoRoute) Error() string { // paymentLifecycle holds all information about the current state of a payment // needed to resume if from any point. type paymentLifecycle struct { - router *ChannelRouter - payment *LightningPayment - paySession PaymentSession - timeoutChan <-chan time.Time - currentHeight int32 - finalCLTVDelta uint16 - attempt *channeldb.HTLCAttemptInfo - circuit *sphinx.Circuit - lastError error + router *ChannelRouter + payment *LightningPayment + paySession PaymentSession + timeoutChan <-chan time.Time + currentHeight int32 + attempt *channeldb.HTLCAttemptInfo + circuit *sphinx.Circuit + lastError error } // resumePayment resumes the paymentLifecycle from the current state. @@ -267,9 +266,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, } // Create a new payment attempt from the given payment session. - rt, err := p.paySession.RequestRoute( - p.payment, uint32(p.currentHeight), p.finalCLTVDelta, - ) + rt, err := p.paySession.RequestRoute(uint32(p.currentHeight)) if err != nil { log.Warnf("Failed to find route for payment %x: %v", p.payment.PaymentHash, err) diff --git a/routing/payment_session.go b/routing/payment_session.go index 47732e01a..fda93536e 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -24,8 +24,7 @@ var ( type PaymentSession interface { // RequestRoute returns the next route to attempt for routing the // specified HTLC payment to the target node. - RequestRoute(payment *LightningPayment, - height uint32, finalCltvDelta uint16) (*route.Route, error) + RequestRoute(height uint32) (*route.Route, error) } // paymentSession is used during an HTLC routings session to prune the local @@ -43,6 +42,8 @@ type paymentSession struct { sessionSource *SessionSource + payment *LightningPayment + preBuiltRoute *route.Route preBuiltRouteTried bool @@ -58,8 +59,7 @@ type paymentSession struct { // // NOTE: This function is safe for concurrent access. // NOTE: Part of the PaymentSession interface. -func (p *paymentSession) RequestRoute(payment *LightningPayment, - height uint32, finalCltvDelta uint16) (*route.Route, error) { +func (p *paymentSession) RequestRoute(height uint32) (*route.Route, error) { switch { @@ -77,12 +77,13 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, // Add BlockPadding to the finalCltvDelta so that the receiving node // does not reject the HTLC if some blocks are mined while it's in-flight. + finalCltvDelta := p.payment.FinalCLTVDelta finalCltvDelta += BlockPadding // We need to subtract the final delta before passing it into path // finding. The optimal path is independent of the final cltv delta and // the path finding algorithm is unaware of this value. - cltvLimit := payment.CltvLimit - uint32(finalCltvDelta) + cltvLimit := p.payment.CltvLimit - uint32(finalCltvDelta) // TODO(roasbeef): sync logic amongst dist sys @@ -93,13 +94,13 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, restrictions := &RestrictParams{ ProbabilitySource: ss.MissionControl.GetProbability, - FeeLimit: payment.FeeLimit, - OutgoingChannelID: payment.OutgoingChannelID, - LastHop: payment.LastHop, + FeeLimit: p.payment.FeeLimit, + OutgoingChannelID: p.payment.OutgoingChannelID, + LastHop: p.payment.LastHop, CltvLimit: cltvLimit, - DestCustomRecords: payment.DestCustomRecords, - DestFeatures: payment.DestFeatures, - PaymentAddr: payment.PaymentAddr, + DestCustomRecords: p.payment.DestCustomRecords, + DestFeatures: p.payment.DestFeatures, + PaymentAddr: p.payment.PaymentAddr, } // We'll also obtain a set of bandwidthHints from the lower layer for @@ -122,8 +123,8 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, bandwidthHints: bandwidthHints, }, restrictions, &ss.PathFindingConfig, - ss.SelfNode.PubKeyBytes, payment.Target, - payment.Amount, finalHtlcExpiry, + ss.SelfNode.PubKeyBytes, p.payment.Target, + p.payment.Amount, finalHtlcExpiry, ) if err != nil { return nil, err @@ -135,10 +136,10 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment, route, err := newRoute( sourceVertex, path, height, finalHopParams{ - amt: payment.Amount, + amt: p.payment.Amount, cltvDelta: finalCltvDelta, - records: payment.DestCustomRecords, - paymentAddr: payment.PaymentAddr, + records: p.payment.DestCustomRecords, + paymentAddr: p.payment.PaymentAddr, }, ) if err != nil { diff --git a/routing/payment_session_source.go b/routing/payment_session_source.go index 05295b58d..f3fd968e8 100644 --- a/routing/payment_session_source.go +++ b/routing/payment_session_source.go @@ -47,10 +47,10 @@ type SessionSource struct { // view from Mission Control. An optional set of routing hints can be provided // in order to populate additional edges to explore when finding a path to the // payment's destination. -func (m *SessionSource) NewPaymentSession(routeHints [][]zpay32.HopHint, - target route.Vertex) (PaymentSession, error) { +func (m *SessionSource) NewPaymentSession(p *LightningPayment) ( + PaymentSession, error) { - edges, err := RouteHintsToEdges(routeHints, target) + edges, err := RouteHintsToEdges(p.RouteHints, p.Target) if err != nil { return nil, err } @@ -70,6 +70,7 @@ func (m *SessionSource) NewPaymentSession(routeHints [][]zpay32.HopHint, additionalEdges: edges, getBandwidthHints: getBandwidthHints, sessionSource: m, + payment: p, pathFinder: findPath, }, nil } diff --git a/routing/payment_session_test.go b/routing/payment_session_test.go index 55549c448..6d795b89d 100644 --- a/routing/payment_session_test.go +++ b/routing/payment_session_test.go @@ -44,16 +44,6 @@ func TestRequestRoute(t *testing.T) { }, } - session := &paymentSession{ - getBandwidthHints: func() (map[uint64]lnwire.MilliSatoshi, - error) { - - return nil, nil - }, - sessionSource: sessionSource, - pathFinder: findPath, - } - cltvLimit := uint32(30) finalCltvDelta := uint16(8) @@ -62,7 +52,18 @@ func TestRequestRoute(t *testing.T) { FinalCLTVDelta: finalCltvDelta, } - route, err := session.RequestRoute(payment, height, finalCltvDelta) + session := &paymentSession{ + getBandwidthHints: func() (map[uint64]lnwire.MilliSatoshi, + error) { + + return nil, nil + }, + sessionSource: sessionSource, + payment: payment, + pathFinder: findPath, + } + + route, err := session.RequestRoute(height) if err != nil { t.Fatal(err) } diff --git a/routing/router.go b/routing/router.go index ca7a20f80..5ef4dcaa3 100644 --- a/routing/router.go +++ b/routing/router.go @@ -159,8 +159,7 @@ type PaymentSessionSource interface { // routes to the given target. An optional set of routing hints can be // provided in order to populate additional edges to explore when // finding a path to the payment's destination. - NewPaymentSession(routeHints [][]zpay32.HopHint, - target route.Vertex) (PaymentSession, error) + NewPaymentSession(p *LightningPayment) (PaymentSession, error) // NewPaymentSessionForRoute creates a new paymentSession instance that // is just used for failure reporting to missioncontrol, and will only @@ -1677,9 +1676,7 @@ func (r *ChannelRouter) preparePayment(payment *LightningPayment) ( // Before starting the HTLC routing attempt, we'll create a fresh // payment session which will report our errors back to mission // control. - paySession, err := r.cfg.SessionSource.NewPaymentSession( - payment.RouteHints, payment.Target, - ) + paySession, err := r.cfg.SessionSource.NewPaymentSession(payment) if err != nil { return nil, err } @@ -1813,14 +1810,13 @@ func (r *ChannelRouter) sendPayment( // Now set up a paymentLifecycle struct with these params, such that we // can resume the payment from the current state. p := &paymentLifecycle{ - router: r, - payment: payment, - paySession: paySession, - currentHeight: currentHeight, - finalCLTVDelta: uint16(payment.FinalCLTVDelta), - attempt: existingAttempt, - circuit: nil, - lastError: nil, + router: r, + payment: payment, + paySession: paySession, + currentHeight: currentHeight, + attempt: existingAttempt, + circuit: nil, + lastError: nil, } // If a timeout is specified, create a timeout channel. If no timeout is From f9eeb6b41ff5e78da97deff24be27add956586b5 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:22 +0200 Subject: [PATCH 02/34] routing/router+lifecycle: remove LightningPayment from payment lifecycle Now that the information needed is stored by the paymentSession, we no longer need to pass the LightningPayment into the payment lifecycle. --- routing/payment_lifecycle.go | 43 +++++++------ routing/router.go | 121 +++++++++++++++++++---------------- 2 files changed, 88 insertions(+), 76 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 58bafc7d9..a0cb566a6 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -8,6 +8,7 @@ import ( sphinx "github.com/lightningnetwork/lightning-onion" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/htlcswitch" + "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" ) @@ -30,7 +31,7 @@ func (e errNoRoute) Error() string { // needed to resume if from any point. type paymentLifecycle struct { router *ChannelRouter - payment *LightningPayment + paymentHash lntypes.Hash paySession PaymentSession timeoutChan <-chan time.Time currentHeight int32 @@ -84,7 +85,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // from an invalid route, because the sphinx packet has // been successfully generated before. _, c, err := generateSphinxPacket( - &p.attempt.Route, p.payment.PaymentHash[:], + &p.attempt.Route, p.paymentHash[:], p.attempt.SessionKey, ) if err != nil { @@ -103,7 +104,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // Now ask the switch to return the result of the payment when // available. resultChan, err := p.router.cfg.Payer.GetPaymentResult( - p.attempt.AttemptID, p.payment.PaymentHash, errorDecryptor, + p.attempt.AttemptID, p.paymentHash, errorDecryptor, ) switch { @@ -114,7 +115,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { case err == htlcswitch.ErrPaymentIDNotFound: log.Debugf("Payment ID %v for hash %x not found in "+ "the Switch, retrying.", p.attempt.AttemptID, - p.payment.PaymentHash) + p.paymentHash) err = p.failAttempt(err) if err != nil { @@ -155,7 +156,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // whether we should retry. if result.Error != nil { log.Errorf("Attempt to send payment %x failed: %v", - p.payment.PaymentHash, result.Error) + p.paymentHash, result.Error) err = p.failAttempt(result.Error) if err != nil { @@ -177,7 +178,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // We successfully got a payment result back from the switch. log.Debugf("Payment %x succeeded with pid=%v", - p.payment.PaymentHash, p.attempt.AttemptID) + p.paymentHash, p.attempt.AttemptID) // Report success to mission control. err = p.router.cfg.MissionControl.ReportPaymentSuccess( @@ -191,7 +192,7 @@ 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.SettleAttempt( - p.payment.PaymentHash, p.attempt.AttemptID, + p.paymentHash, p.attempt.AttemptID, &channeldb.HTLCSettleInfo{ Preimage: result.Preimage, SettleTime: p.router.cfg.Clock.Now(), @@ -243,7 +244,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, channeldb.FailureReasonTimeout, + p.paymentHash, channeldb.FailureReasonTimeout, ) if err != nil { return lnwire.ShortChannelID{}, nil, err @@ -269,7 +270,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, rt, err := p.paySession.RequestRoute(uint32(p.currentHeight)) if err != nil { log.Warnf("Failed to find route for payment %x: %v", - p.payment.PaymentHash, err) + p.paymentHash, err) // Convert error to payment-level failure. failure := errorToPaymentFailure(err) @@ -278,7 +279,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, failure, + p.paymentHash, failure, ) if saveErr != nil { return lnwire.ShortChannelID{}, nil, saveErr @@ -304,7 +305,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // with the htlcAdd message that we send directly to the // switch. onionBlob, c, err := generateSphinxPacket( - rt, p.payment.PaymentHash[:], sessionKey, + rt, p.paymentHash[:], sessionKey, ) // With SendToRoute, it can happen that the route exceeds protocol @@ -313,10 +314,10 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, err == sphinx.ErrMaxRoutingInfoSizeExceeded { log.Debugf("Invalid route provided for payment %x: %v", - p.payment.PaymentHash, err) + p.paymentHash, err) controlErr := p.router.cfg.Control.Fail( - p.payment.PaymentHash, channeldb.FailureReasonError, + p.paymentHash, channeldb.FailureReasonError, ) if controlErr != nil { return lnwire.ShortChannelID{}, nil, controlErr @@ -338,7 +339,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, htlcAdd := &lnwire.UpdateAddHTLC{ Amount: rt.TotalAmount, Expiry: rt.TotalTimeLock, - PaymentHash: p.payment.PaymentHash, + PaymentHash: p.paymentHash, } copy(htlcAdd.OnionBlob[:], onionBlob) @@ -371,7 +372,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, // such that we can query the Switch for its whereabouts. The // route is needed to handle the result when it eventually // comes back. - err = p.router.cfg.Control.RegisterAttempt(p.payment.PaymentHash, p.attempt) + err = p.router.cfg.Control.RegisterAttempt(p.paymentHash, p.attempt) if err != nil { return lnwire.ShortChannelID{}, nil, err } @@ -384,7 +385,7 @@ func (p *paymentLifecycle) sendPaymentAttempt(firstHop lnwire.ShortChannelID, htlcAdd *lnwire.UpdateAddHTLC) error { log.Tracef("Attempting to send payment %x (pid=%v), "+ - "using route: %v", p.payment.PaymentHash, p.attempt.AttemptID, + "using route: %v", p.paymentHash, p.attempt.AttemptID, newLogClosure(func() string { return spew.Sdump(p.attempt.Route) }), @@ -400,12 +401,12 @@ func (p *paymentLifecycle) sendPaymentAttempt(firstHop lnwire.ShortChannelID, if err != nil { log.Errorf("Failed sending attempt %d for payment "+ "%x to switch: %v", p.attempt.AttemptID, - p.payment.PaymentHash, err) + p.paymentHash, err) return err } log.Debugf("Payment %x (pid=%v) successfully sent to switch, route: %v", - p.payment.PaymentHash, p.attempt.AttemptID, &p.attempt.Route) + p.paymentHash, p.attempt.AttemptID, &p.attempt.Route) return nil } @@ -426,14 +427,14 @@ func (p *paymentLifecycle) handleSendError(sendErr error) error { } log.Debugf("Payment %x failed: final_outcome=%v, raw_err=%v", - p.payment.PaymentHash, *reason, sendErr) + p.paymentHash, *reason, sendErr) // Mark the payment failed with no route. // // TODO(halseth): make payment codes for the actual reason we don't // continue path finding. err := p.router.cfg.Control.Fail( - p.payment.PaymentHash, *reason, + p.paymentHash, *reason, ) if err != nil { return err @@ -451,7 +452,7 @@ func (p *paymentLifecycle) failAttempt(sendError error) error { ) return p.router.cfg.Control.FailAttempt( - p.payment.PaymentHash, p.attempt.AttemptID, + p.paymentHash, p.attempt.AttemptID, failInfo, ) } diff --git a/routing/router.go b/routing/router.go index 5ef4dcaa3..581671c18 100644 --- a/routing/router.go +++ b/routing/router.go @@ -531,15 +531,8 @@ func (r *ChannelRouter) Start() error { // We create a dummy, empty payment session such that // we won't make another payment attempt when the // result for the in-flight attempt is received. - // - // PayAttemptTime doesn't need to be set, as there is - // only a single attempt. paySession := r.cfg.SessionSource.NewPaymentSessionEmpty() - lPayment := &LightningPayment{ - PaymentHash: payment.Info.PaymentHash, - } - // TODO(joostjager): For mpp, possibly relaunch multiple // in-flight htlcs here. var attempt *channeldb.HTLCAttemptInfo @@ -547,7 +540,13 @@ func (r *ChannelRouter) Start() error { attempt = &payment.Attempts[0] } - _, _, err := r.sendPayment(attempt, lPayment, paySession) + // We pass in a zero timeout value, to indicate we + // don't need it to timeout. It will stop immediately + // after the existing attempt has finished anyway. + _, _, err := r.sendPayment( + attempt, + payment.Info.PaymentHash, 0, paySession, + ) if err != nil { log.Errorf("Resuming payment with hash %v "+ "failed: %v.", payment.Info.PaymentHash, err) @@ -1639,9 +1638,15 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, return [32]byte{}, nil, err } + log.Tracef("Dispatching SendPayment for lightning payment: %v", + spewPayment(payment)) + // Since this is the first time this payment is being made, we pass nil // for the existing attempt. - return r.sendPayment(nil, payment, paySession) + return r.sendPayment( + nil, payment.PaymentHash, + payment.PayAttemptTimeout, paySession, + ) } // SendPaymentAsync is the non-blocking version of SendPayment. The payment @@ -1658,7 +1663,13 @@ func (r *ChannelRouter) SendPaymentAsync(payment *LightningPayment) error { go func() { defer r.wg.Done() - _, _, err := r.sendPayment(nil, payment, paySession) + log.Tracef("Dispatching SendPayment for lightning payment: %v", + spewPayment(payment)) + + _, _, err := r.sendPayment( + nil, payment.PaymentHash, + payment.PayAttemptTimeout, paySession, + ) if err != nil { log.Errorf("Payment with hash %x failed: %v", payment.PaymentHash, err) @@ -1668,6 +1679,28 @@ func (r *ChannelRouter) SendPaymentAsync(payment *LightningPayment) error { return nil } +// spewPayment returns a log closures that provides a spewed string +// representation of the passed payment. +func spewPayment(payment *LightningPayment) logClosure { + return newLogClosure(func() string { + // Make a copy of the payment with a nilled Curve + // before spewing. + var routeHints [][]zpay32.HopHint + for _, routeHint := range payment.RouteHints { + var hopHints []zpay32.HopHint + for _, hopHint := range routeHint { + h := hopHint.Copy() + h.NodeID.Curve = nil + hopHints = append(hopHints, h) + } + routeHints = append(routeHints, hopHints) + } + p := *payment + p.RouteHints = routeHints + return spew.Sdump(p) + }) +} + // preparePayment creates the payment session and registers the payment with the // control tower. func (r *ChannelRouter) preparePayment(payment *LightningPayment) ( @@ -1726,20 +1759,17 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( return [32]byte{}, err } - // Create a (mostly) dummy payment, as the created payment session is - // not going to do path finding. - // TODO(halseth): sendPayment doesn't really need LightningPayment, make - // it take just needed fields instead. - // - // PayAttemptTime doesn't need to be set, as there is only a single - // attempt. - payment := &LightningPayment{ - PaymentHash: hash, - } + log.Tracef("Dispatching SendToRoute for hash %v: %v", + hash, newLogClosure(func() string { + return spew.Sdump(route) + }), + ) - // Since this is the first time this payment is being made, we pass nil - // for the existing attempt. - preimage, _, err := r.sendPayment(nil, payment, paySession) + // We set the timeout to a zero value, indicating the payment shouldn't + // timeout. It is only a single attempt, so no more attempts will be + // done anyway. Since this is the first time this payment is being + // made, we pass nil for the existing attempt. + preimage, _, err := r.sendPayment(nil, hash, 0, paySession) if err != nil { // SendToRoute should return a structured error. In case the // provided route fails, payment lifecycle will return a @@ -1759,13 +1789,13 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( return preimage, nil } -// sendPayment attempts to send a payment as described within the passed -// LightningPayment. This function is blocking and will return either: when the -// payment is successful, or all candidates routes have been attempted and -// resulted in a failed payment. If the payment succeeds, then a non-nil Route -// will be returned which describes the path the successful payment traversed -// within the network to reach the destination. Additionally, the payment -// preimage will also be returned. +// sendPayment attempts to send a payment to the passed payment hash. This +// function is blocking and will return either: when the payment is successful, +// or all candidates routes have been attempted and resulted in a failed +// payment. If the payment succeeds, then a non-nil Route will be returned +// which describes the path the successful payment traversed within the network +// to reach the destination. Additionally, the payment preimage will also be +// returned. // // The existing attempt argument should be set to nil if this is a payment that // haven't had any payment attempt sent to the switch yet. If it has had an @@ -1777,28 +1807,9 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // the ControlTower. func (r *ChannelRouter) sendPayment( existingAttempt *channeldb.HTLCAttemptInfo, - payment *LightningPayment, paySession PaymentSession) ( - [32]byte, *route.Route, error) { - - log.Tracef("Dispatching route for lightning payment: %v", - newLogClosure(func() string { - // Make a copy of the payment with a nilled Curve - // before spewing. - var routeHints [][]zpay32.HopHint - for _, routeHint := range payment.RouteHints { - var hopHints []zpay32.HopHint - for _, hopHint := range routeHint { - h := hopHint.Copy() - h.NodeID.Curve = nil - hopHints = append(hopHints, h) - } - routeHints = append(routeHints, hopHints) - } - p := *payment - p.RouteHints = routeHints - return spew.Sdump(p) - }), - ) + paymentHash lntypes.Hash, + timeout time.Duration, + paySession PaymentSession) ([32]byte, *route.Route, error) { // We'll also fetch the current block height so we can properly // calculate the required HTLC time locks within the route. @@ -1811,7 +1822,7 @@ func (r *ChannelRouter) sendPayment( // can resume the payment from the current state. p := &paymentLifecycle{ router: r, - payment: payment, + paymentHash: paymentHash, paySession: paySession, currentHeight: currentHeight, attempt: existingAttempt, @@ -1822,8 +1833,8 @@ func (r *ChannelRouter) sendPayment( // If a timeout is specified, create a timeout channel. If no timeout is // specified, the channel is left nil and will never abort the payment // loop. - if payment.PayAttemptTimeout != 0 { - p.timeoutChan = time.After(payment.PayAttemptTimeout) + if timeout != 0 { + p.timeoutChan = time.After(timeout) } return p.resumePayment() From 00903ef9f51f0d10636e7381332dba48a6d7a1c9 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:22 +0200 Subject: [PATCH 03/34] routing/payment_session: make RequestRoute take max amt, fee limit and active shards In preparation for doing pathfinding for routes sending a value less than the total payment amount, we let the payment session take the max amount to send and the fee limit as arguments to RequestRoute. --- routing/mock_test.go | 4 +++- routing/payment_lifecycle.go | 6 +++++- routing/payment_session.go | 19 +++++++++++++------ routing/payment_session_test.go | 6 +++++- routing/router.go | 25 ++++++++++++++++++------- 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/routing/mock_test.go b/routing/mock_test.go index 64a1e4f65..c6a780bb5 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -122,7 +122,9 @@ type mockPaymentSession struct { var _ PaymentSession = (*mockPaymentSession)(nil) -func (m *mockPaymentSession) RequestRoute(height uint32) (*route.Route, error) { +func (m *mockPaymentSession) RequestRoute(_, _ lnwire.MilliSatoshi, + _, height uint32) (*route.Route, error) { + if len(m.routes) == 0 { return nil, fmt.Errorf("no routes") } diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index a0cb566a6..f153d4b64 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -31,6 +31,8 @@ func (e errNoRoute) Error() string { // needed to resume if from any point. type paymentLifecycle struct { router *ChannelRouter + totalAmount lnwire.MilliSatoshi + feeLimit lnwire.MilliSatoshi paymentHash lntypes.Hash paySession PaymentSession timeoutChan <-chan time.Time @@ -267,7 +269,9 @@ func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, } // Create a new payment attempt from the given payment session. - rt, err := p.paySession.RequestRoute(uint32(p.currentHeight)) + rt, err := p.paySession.RequestRoute( + p.totalAmount, p.feeLimit, 0, uint32(p.currentHeight), + ) if err != nil { log.Warnf("Failed to find route for payment %x: %v", p.paymentHash, err) diff --git a/routing/payment_session.go b/routing/payment_session.go index fda93536e..a5c042667 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -23,8 +23,14 @@ var ( // information learned during the previous attempts. type PaymentSession interface { // RequestRoute returns the next route to attempt for routing the - // specified HTLC payment to the target node. - RequestRoute(height uint32) (*route.Route, error) + // specified HTLC payment to the target node. The returned route should + // carry at most maxAmt to the target node, and pay at most feeLimit in + // fees. It can carry less if the payment is MPP. The activeShards + // argument should be set to instruct the payment session about the + // number of in flight HTLCS for the payment, such that it can choose + // splitting strategy accordingly. + RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, + activeShards, height uint32) (*route.Route, error) } // paymentSession is used during an HTLC routings session to prune the local @@ -59,7 +65,8 @@ type paymentSession struct { // // NOTE: This function is safe for concurrent access. // NOTE: Part of the PaymentSession interface. -func (p *paymentSession) RequestRoute(height uint32) (*route.Route, error) { +func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, + activeShards, height uint32) (*route.Route, error) { switch { @@ -94,7 +101,7 @@ func (p *paymentSession) RequestRoute(height uint32) (*route.Route, error) { restrictions := &RestrictParams{ ProbabilitySource: ss.MissionControl.GetProbability, - FeeLimit: p.payment.FeeLimit, + FeeLimit: feeLimit, OutgoingChannelID: p.payment.OutgoingChannelID, LastHop: p.payment.LastHop, CltvLimit: cltvLimit, @@ -124,7 +131,7 @@ func (p *paymentSession) RequestRoute(height uint32) (*route.Route, error) { }, restrictions, &ss.PathFindingConfig, ss.SelfNode.PubKeyBytes, p.payment.Target, - p.payment.Amount, finalHtlcExpiry, + maxAmt, finalHtlcExpiry, ) if err != nil { return nil, err @@ -136,7 +143,7 @@ func (p *paymentSession) RequestRoute(height uint32) (*route.Route, error) { route, err := newRoute( sourceVertex, path, height, finalHopParams{ - amt: p.payment.Amount, + amt: maxAmt, cltvDelta: finalCltvDelta, records: p.payment.DestCustomRecords, paymentAddr: p.payment.PaymentAddr, diff --git a/routing/payment_session_test.go b/routing/payment_session_test.go index 6d795b89d..a91658043 100644 --- a/routing/payment_session_test.go +++ b/routing/payment_session_test.go @@ -50,6 +50,8 @@ func TestRequestRoute(t *testing.T) { payment := &LightningPayment{ CltvLimit: cltvLimit, FinalCLTVDelta: finalCltvDelta, + Amount: 1000, + FeeLimit: 1000, } session := &paymentSession{ @@ -63,7 +65,9 @@ func TestRequestRoute(t *testing.T) { pathFinder: findPath, } - route, err := session.RequestRoute(height) + route, err := session.RequestRoute( + payment.Amount, payment.FeeLimit, 0, height, + ) if err != nil { t.Fatal(err) } diff --git a/routing/router.go b/routing/router.go index 581671c18..18e1689b9 100644 --- a/routing/router.go +++ b/routing/router.go @@ -542,9 +542,11 @@ func (r *ChannelRouter) Start() error { // We pass in a zero timeout value, to indicate we // don't need it to timeout. It will stop immediately - // after the existing attempt has finished anyway. + // after the existing attempt has finished anyway. We + // also set a zero fee limit, as no more routes should + // be tried. _, _, err := r.sendPayment( - attempt, + attempt, payment.Info.Value, 0, payment.Info.PaymentHash, 0, paySession, ) if err != nil { @@ -1644,7 +1646,7 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, // Since this is the first time this payment is being made, we pass nil // for the existing attempt. return r.sendPayment( - nil, payment.PaymentHash, + nil, payment.Amount, payment.FeeLimit, payment.PaymentHash, payment.PayAttemptTimeout, paySession, ) } @@ -1667,8 +1669,9 @@ func (r *ChannelRouter) SendPaymentAsync(payment *LightningPayment) error { spewPayment(payment)) _, _, err := r.sendPayment( - nil, payment.PaymentHash, - payment.PayAttemptTimeout, paySession, + nil, payment.Amount, payment.FeeLimit, + payment.PaymentHash, payment.PayAttemptTimeout, + paySession, ) if err != nil { log.Errorf("Payment with hash %x failed: %v", @@ -1769,7 +1772,13 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // timeout. It is only a single attempt, so no more attempts will be // done anyway. Since this is the first time this payment is being // made, we pass nil for the existing attempt. - preimage, _, err := r.sendPayment(nil, hash, 0, paySession) + // We pass the route receiver amount as the total payment amount such + // that the payment loop will request a route for this amount. As fee + // limit we pass the route's total fees, since we already know this is + // the route that is going to be used. + preimage, _, err := r.sendPayment( + nil, amt, route.TotalFees(), hash, 0, paySession, + ) if err != nil { // SendToRoute should return a structured error. In case the // provided route fails, payment lifecycle will return a @@ -1807,7 +1816,7 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // the ControlTower. func (r *ChannelRouter) sendPayment( existingAttempt *channeldb.HTLCAttemptInfo, - paymentHash lntypes.Hash, + totalAmt, feeLimit lnwire.MilliSatoshi, paymentHash lntypes.Hash, timeout time.Duration, paySession PaymentSession) ([32]byte, *route.Route, error) { @@ -1822,6 +1831,8 @@ func (r *ChannelRouter) sendPayment( // can resume the payment from the current state. p := &paymentLifecycle{ router: r, + totalAmount: totalAmt, + feeLimit: feeLimit, paymentHash: paymentHash, paySession: paySession, currentHeight: currentHeight, From e61fcda6a946720aa3dc6b31ba4ccfabd8f4ee45 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:23 +0200 Subject: [PATCH 04/34] routing/payment_lifecycle: move requesting route out of createNewPaymentAttempt To prepare for having more than one payment attempt in flight at the same time, we decouple fetching the next route from crafting the payment attempt. --- routing/payment_lifecycle.go | 131 ++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 64 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index f153d4b64..49bf8dfd6 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -51,7 +51,72 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // If this payment had no existing payment attempt, we create // and send one now. if p.attempt == nil { - firstHop, htlcAdd, err := p.createNewPaymentAttempt() + // Before we attempt this next payment, we'll check to see if either + // we've gone past the payment attempt timeout, or the router is + // exiting. In either case, we'll stop this payment attempt short. If a + // timeout is not applicable, timeoutChan will be nil. + select { + case <-p.timeoutChan: + // Mark the payment as failed because of the + // timeout. + err := p.router.cfg.Control.Fail( + p.paymentHash, channeldb.FailureReasonTimeout, + ) + if err != nil { + return [32]byte{}, nil, err + } + + errStr := fmt.Sprintf("payment attempt not completed " + + "before timeout") + + return [32]byte{}, nil, newErr(ErrPaymentAttemptTimeout, errStr) + + // The payment will be resumed from the current state + // after restart. + case <-p.router.quit: + return [32]byte{}, nil, ErrRouterShuttingDown + + // Fall through if we haven't hit our time limit or are + // exiting. + default: + } + + // Create a new payment attempt from the given payment session. + rt, err := p.paySession.RequestRoute( + p.totalAmount, p.feeLimit, 0, uint32(p.currentHeight), + ) + if err != nil { + log.Warnf("Failed to find route for payment %x: %v", + p.paymentHash, err) + + // Convert error to payment-level failure. + failure := errorToPaymentFailure(err) + + // If we're unable to successfully make a payment using + // any of the routes we've found, then mark the payment + // as permanently failed. + saveErr := p.router.cfg.Control.Fail( + p.paymentHash, failure, + ) + if saveErr != nil { + return [32]byte{}, nil, saveErr + } + + // If there was an error already recorded for this + // payment, we'll return that. + if p.lastError != nil { + return [32]byte{}, nil, errNoRoute{lastError: p.lastError} + } + + // Terminal state, return. + return [32]byte{}, nil, err + } + + // Using the route received from the payment session, + // create a new shard to send. + firstHop, htlcAdd, err := p.createNewPaymentAttempt( + rt, + ) if err != nil { return [32]byte{}, nil, err } @@ -234,71 +299,9 @@ func errorToPaymentFailure(err error) channeldb.FailureReason { // createNewPaymentAttempt creates and stores a new payment attempt to the // database. -func (p *paymentLifecycle) createNewPaymentAttempt() (lnwire.ShortChannelID, +func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) (lnwire.ShortChannelID, *lnwire.UpdateAddHTLC, error) { - // Before we attempt this next payment, we'll check to see if either - // we've gone past the payment attempt timeout, or the router is - // exiting. In either case, we'll stop this payment attempt short. If a - // timeout is not applicable, timeoutChan will be nil. - select { - case <-p.timeoutChan: - // Mark the payment as failed because of the - // timeout. - err := p.router.cfg.Control.Fail( - p.paymentHash, channeldb.FailureReasonTimeout, - ) - if err != nil { - return lnwire.ShortChannelID{}, nil, err - } - - errStr := fmt.Sprintf("payment attempt not completed " + - "before timeout") - - return lnwire.ShortChannelID{}, nil, - newErr(ErrPaymentAttemptTimeout, errStr) - - case <-p.router.quit: - // The payment will be resumed from the current state - // after restart. - return lnwire.ShortChannelID{}, nil, ErrRouterShuttingDown - - default: - // Fall through if we haven't hit our time limit, or - // are expiring. - } - - // Create a new payment attempt from the given payment session. - rt, err := p.paySession.RequestRoute( - p.totalAmount, p.feeLimit, 0, uint32(p.currentHeight), - ) - if err != nil { - log.Warnf("Failed to find route for payment %x: %v", - p.paymentHash, err) - - // Convert error to payment-level failure. - failure := errorToPaymentFailure(err) - - // If we're unable to successfully make a payment using - // any of the routes we've found, then mark the payment - // as permanently failed. - saveErr := p.router.cfg.Control.Fail( - p.paymentHash, failure, - ) - if saveErr != nil { - return lnwire.ShortChannelID{}, nil, saveErr - } - - // If there was an error already recorded for this - // payment, we'll return that. - if p.lastError != nil { - return lnwire.ShortChannelID{}, nil, - errNoRoute{lastError: p.lastError} - } - // Terminal state, return. - return lnwire.ShortChannelID{}, nil, err - } - // Generate a new key to be used for this attempt. sessionKey, err := generateNewSessionKey() if err != nil { From 362072139129e8d6aa487e5e6460a3d57df21759 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:23 +0200 Subject: [PATCH 05/34] routing/payment_lifecycle: move attempt DB checkpointing into payment loop To prepare for multiple in flight payment attempts, we move checkpointing the payment attempt out of createNewPaymentAttempt and into the main payment lifecycle loop. We'll attempt to move all calls to the DB via the ControlTower into this loop, so we can more easily handle them in sequence. --- routing/payment_lifecycle.go | 45 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 49bf8dfd6..7ef02eeb5 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -114,12 +114,24 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // Using the route received from the payment session, // create a new shard to send. - firstHop, htlcAdd, err := p.createNewPaymentAttempt( + firstHop, htlcAdd, attempt, err := p.createNewPaymentAttempt( rt, ) if err != nil { return [32]byte{}, nil, err } + p.attempt = attempt + + // Before sending this HTLC to the switch, we checkpoint the + // fresh paymentID and route to the DB. This lets us know on + // startup the ID of the payment that we attempted to send, + // such that we can query the Switch for its whereabouts. The + // route is needed to handle the result when it eventually + // comes back. + err = p.router.cfg.Control.RegisterAttempt(p.paymentHash, attempt) + if err != nil { + return [32]byte{}, nil, err + } // Now that the attempt is created and checkpointed to // the DB, we send it. @@ -297,15 +309,15 @@ func errorToPaymentFailure(err error) channeldb.FailureReason { return channeldb.FailureReasonError } -// createNewPaymentAttempt creates and stores a new payment attempt to the -// database. -func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) (lnwire.ShortChannelID, - *lnwire.UpdateAddHTLC, error) { +// createNewPaymentAttempt creates a new payment attempt from the given route. +func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) ( + lnwire.ShortChannelID, *lnwire.UpdateAddHTLC, + *channeldb.HTLCAttemptInfo, error) { // Generate a new key to be used for this attempt. sessionKey, err := generateNewSessionKey() if err != nil { - return lnwire.ShortChannelID{}, nil, err + return lnwire.ShortChannelID{}, nil, nil, err } // Generate the raw encoded sphinx packet to be included along @@ -327,13 +339,13 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) (lnwire.Shor p.paymentHash, channeldb.FailureReasonError, ) if controlErr != nil { - return lnwire.ShortChannelID{}, nil, controlErr + return lnwire.ShortChannelID{}, nil, nil, controlErr } } // In any case, don't continue if there is an error. if err != nil { - return lnwire.ShortChannelID{}, nil, err + return lnwire.ShortChannelID{}, nil, nil, err } // Update our cached circuit with the newly generated @@ -361,30 +373,19 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) (lnwire.Shor // this HTLC. attemptID, err := p.router.cfg.NextPaymentID() if err != nil { - return lnwire.ShortChannelID{}, nil, err + return lnwire.ShortChannelID{}, nil, nil, err } // We now have all the information needed to populate // the current attempt information. - p.attempt = &channeldb.HTLCAttemptInfo{ + attempt := &channeldb.HTLCAttemptInfo{ AttemptID: attemptID, AttemptTime: p.router.cfg.Clock.Now(), SessionKey: sessionKey, Route: *rt, } - // Before sending this HTLC to the switch, we checkpoint the - // fresh attemptID and route to the DB. This lets us know on - // startup the ID of the payment that we attempted to send, - // such that we can query the Switch for its whereabouts. The - // route is needed to handle the result when it eventually - // comes back. - err = p.router.cfg.Control.RegisterAttempt(p.paymentHash, p.attempt) - if err != nil { - return lnwire.ShortChannelID{}, nil, err - } - - return firstHop, htlcAdd, nil + return firstHop, htlcAdd, attempt, nil } // sendPaymentAttempt attempts to send the current attempt to the switch. From 4485e8261f1eb8e1daa64b9c8d83f4674e824528 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:23 +0200 Subject: [PATCH 06/34] routing/payment_lifecycle: move Fail call to payment loop In our quest to move calls to the ControlTower into the main payment lifecycle loop, we move the edge case of a too long route out of createNewPaymentAttempt. --- routing/payment_lifecycle.go | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 7ef02eeb5..f2c533ec6 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -117,9 +117,27 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { firstHop, htlcAdd, attempt, err := p.createNewPaymentAttempt( rt, ) + // With SendToRoute, it can happen that the route exceeds protocol + // constraints. Mark the payment as failed with an internal error. + if err == route.ErrMaxRouteHopsExceeded || + err == sphinx.ErrMaxRoutingInfoSizeExceeded { + + log.Debugf("Invalid route provided for payment %x: %v", + p.paymentHash, err) + + controlErr := p.router.cfg.Control.Fail( + p.paymentHash, channeldb.FailureReasonError, + ) + if controlErr != nil { + return [32]byte{}, nil, controlErr + } + } + + // In any case, don't continue if there is an error. if err != nil { return [32]byte{}, nil, err } + p.attempt = attempt // Before sending this HTLC to the switch, we checkpoint the @@ -326,24 +344,6 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) ( onionBlob, c, err := generateSphinxPacket( rt, p.paymentHash[:], sessionKey, ) - - // With SendToRoute, it can happen that the route exceeds protocol - // constraints. Mark the payment as failed with an internal error. - if err == route.ErrMaxRouteHopsExceeded || - err == sphinx.ErrMaxRoutingInfoSizeExceeded { - - log.Debugf("Invalid route provided for payment %x: %v", - p.paymentHash, err) - - controlErr := p.router.cfg.Control.Fail( - p.paymentHash, channeldb.FailureReasonError, - ) - if controlErr != nil { - return lnwire.ShortChannelID{}, nil, nil, controlErr - } - } - - // In any case, don't continue if there is an error. if err != nil { return lnwire.ShortChannelID{}, nil, nil, err } From 79227bab3a05a25418c7b76470d1ed42293a7ad3 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:23 +0200 Subject: [PATCH 07/34] routing/route: define route.ReceiverAmt() method --- routing/route/route.go | 11 ++++++++++- routing/route/route_test.go | 23 ++++++++++++++++++++--- routing/router.go | 2 +- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/routing/route/route.go b/routing/route/route.go index f4de728fc..31fa3bf50 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -308,7 +308,16 @@ func (r *Route) TotalFees() lnwire.MilliSatoshi { return 0 } - return r.TotalAmount - r.Hops[len(r.Hops)-1].AmtToForward + return r.TotalAmount - r.ReceiverAmt() +} + +// ReceiverAmt is the amount received by the final hop of this route. +func (r *Route) ReceiverAmt() lnwire.MilliSatoshi { + if len(r.Hops) == 0 { + return 0 + } + + return r.Hops[len(r.Hops)-1].AmtToForward } // NewRouteFromHops creates a new Route structure from the minimally required diff --git a/routing/route/route_test.go b/routing/route/route_test.go index 2095430b8..991175f49 100644 --- a/routing/route/route_test.go +++ b/routing/route/route_test.go @@ -20,15 +20,24 @@ var ( func TestRouteTotalFees(t *testing.T) { t.Parallel() - // Make sure empty route returns a 0 fee. + // Make sure empty route returns a 0 fee, and zero amount. r := &Route{} if r.TotalFees() != 0 { t.Fatalf("expected 0 fees, got %v", r.TotalFees()) } + if r.ReceiverAmt() != 0 { + t.Fatalf("expected 0 amt, got %v", r.ReceiverAmt()) + } + + // Make sure empty route won't be allowed in the constructor. + amt := lnwire.MilliSatoshi(1000) + _, err := NewRouteFromHops(amt, 100, Vertex{}, []*Hop{}) + if err != ErrNoRouteHopsProvided { + t.Fatalf("expected ErrNoRouteHopsProvided, got %v", err) + } // For one-hop routes the fee should be 0, since the last node will // receive the full amount. - amt := lnwire.MilliSatoshi(1000) hops := []*Hop{ { PubKeyBytes: Vertex{}, @@ -37,7 +46,7 @@ func TestRouteTotalFees(t *testing.T) { AmtToForward: amt, }, } - r, err := NewRouteFromHops(amt, 100, Vertex{}, hops) + r, err = NewRouteFromHops(amt, 100, Vertex{}, hops) if err != nil { t.Fatal(err) } @@ -46,6 +55,10 @@ func TestRouteTotalFees(t *testing.T) { t.Fatalf("expected 0 fees, got %v", r.TotalFees()) } + if r.ReceiverAmt() != amt { + t.Fatalf("expected %v amt, got %v", amt, r.ReceiverAmt()) + } + // Append the route with a node, making the first one take a fee. fee := lnwire.MilliSatoshi(100) hops = append(hops, &Hop{ @@ -64,6 +77,10 @@ func TestRouteTotalFees(t *testing.T) { if r.TotalFees() != fee { t.Fatalf("expected %v fees, got %v", fee, r.TotalFees()) } + + if r.ReceiverAmt() != amt-fee { + t.Fatalf("expected %v amt, got %v", amt-fee, r.ReceiverAmt()) + } } var ( diff --git a/routing/router.go b/routing/router.go index 18e1689b9..9595b0054 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1746,7 +1746,7 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( paySession := r.cfg.SessionSource.NewPaymentSessionForRoute(route) // Calculate amount paid to receiver. - amt := route.TotalAmount - route.TotalFees() + amt := route.ReceiverAmt() // Record this payment hash with the ControlTower, ensuring it is not // already in-flight. From 6d9f9c31f42b5fe7c51f767b5b62a0f74fa0aa54 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:23 +0200 Subject: [PATCH 08/34] routing/router_test: remove preimage overwrite The test case's preimage was (mistakenly) overwritten after crafting the lightning payment, causing the parts of the testcases use the same preimage causing problems when we are using the payment hash and preimage in the mock control tower to distinguish paymennts. --- routing/router_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/routing/router_test.go b/routing/router_test.go index 42020b7ea..d8550ac73 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -3025,8 +3025,6 @@ func TestRouterPaymentStateMachine(t *testing.T) { PaymentHash: payHash, } - copy(preImage[:], bytes.Repeat([]byte{9}, 32)) - router.cfg.SessionSource = &mockPaymentSessionSource{ routes: test.routes, } From e1f4d89ad924decd22b81963021baf4ea07bdfcd Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:23 +0200 Subject: [PATCH 09/34] routing: add FetchPayment method to ControlTower This method is used to fetch a payment and all HTLC attempt that have been made for that payment. It will both be used to resume inflight attempts, and to fetch the final outcome of previous attempts. We also update the the mock control tower to mimic the real control tower, by letting it track multiple HTLC attempts for a given payment hash, laying the groundwork for later enabling it for MPP. --- routing/control_tower.go | 11 +++ routing/mock_test.go | 183 +++++++++++++++++++++++++++++++++------ 2 files changed, 169 insertions(+), 25 deletions(-) diff --git a/routing/control_tower.go b/routing/control_tower.go index 5ade611cc..da7d4d2bc 100644 --- a/routing/control_tower.go +++ b/routing/control_tower.go @@ -34,6 +34,10 @@ type ControlTower interface { // FailAttempt marks the given payment attempt failed. FailAttempt(lntypes.Hash, uint64, *channeldb.HTLCFailInfo) error + // FetchPayment fetches the payment corresponding to the given payment + // hash. + FetchPayment(paymentHash lntypes.Hash) (*channeldb.MPPayment, error) + // Fail transitions a payment into the Failed state, and records the // ultimate reason the payment failed. Note that this should only be // called when all active active attempts are already failed. After @@ -132,6 +136,13 @@ func (p *controlTower) FailAttempt(paymentHash lntypes.Hash, return p.db.FailAttempt(paymentHash, attemptID, failInfo) } +// FetchPayment fetches the payment corresponding to the given payment hash. +func (p *controlTower) FetchPayment(paymentHash lntypes.Hash) ( + *channeldb.MPPayment, error) { + + return p.db.FetchPayment(paymentHash) +} + // createSuccessResult creates a success result to send to subscribers. func createSuccessResult(htlcs []channeldb.HTLCAttempt) *PaymentResult { // Extract any preimage from the list of HTLCs. diff --git a/routing/mock_test.go b/routing/mock_test.go index c6a780bb5..c66c70b1e 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -188,9 +188,15 @@ type failArgs struct { reason channeldb.FailureReason } +type testPayment struct { + info channeldb.PaymentCreationInfo + attempts []channeldb.HTLCAttempt +} + type mockControlTower struct { - inflights map[lntypes.Hash]channeldb.InFlightPayment + payments map[lntypes.Hash]*testPayment successful map[lntypes.Hash]struct{} + failed map[lntypes.Hash]channeldb.FailureReason init chan initArgs register chan registerArgs @@ -205,8 +211,9 @@ var _ ControlTower = (*mockControlTower)(nil) func makeMockControlTower() *mockControlTower { return &mockControlTower{ - inflights: make(map[lntypes.Hash]channeldb.InFlightPayment), + payments: make(map[lntypes.Hash]*testPayment), successful: make(map[lntypes.Hash]struct{}), + failed: make(map[lntypes.Hash]channeldb.FailureReason), } } @@ -220,18 +227,22 @@ func (m *mockControlTower) InitPayment(phash lntypes.Hash, m.init <- initArgs{c} } + // Don't allow re-init a successful payment. if _, ok := m.successful[phash]; ok { - return fmt.Errorf("already successful") + return channeldb.ErrAlreadyPaid } - _, ok := m.inflights[phash] - if ok { - return fmt.Errorf("in flight") + _, failed := m.failed[phash] + _, ok := m.payments[phash] + + // If the payment is known, only allow re-init if failed. + if ok && !failed { + return channeldb.ErrPaymentInFlight } - m.inflights[phash] = channeldb.InFlightPayment{ - Info: c, - Attempts: make([]channeldb.HTLCAttemptInfo, 0), + delete(m.failed, phash) + m.payments[phash] = &testPayment{ + info: *c, } return nil @@ -247,13 +258,24 @@ func (m *mockControlTower) RegisterAttempt(phash lntypes.Hash, m.register <- registerArgs{a} } - p, ok := m.inflights[phash] - if !ok { - return fmt.Errorf("not in flight") + // Cannot register attempts for successful or failed payments. + if _, ok := m.successful[phash]; ok { + return channeldb.ErrPaymentAlreadySucceeded } - p.Attempts = append(p.Attempts, *a) - m.inflights[phash] = p + if _, ok := m.failed[phash]; ok { + return channeldb.ErrPaymentAlreadyFailed + } + + p, ok := m.payments[phash] + if !ok { + return channeldb.ErrPaymentNotInitiated + } + + p.attempts = append(p.attempts, channeldb.HTLCAttempt{ + HTLCAttemptInfo: *a, + }) + m.payments[phash] = p return nil } @@ -268,9 +290,69 @@ func (m *mockControlTower) SettleAttempt(phash lntypes.Hash, m.success <- successArgs{settleInfo.Preimage} } - delete(m.inflights, phash) - m.successful[phash] = struct{}{} - return nil + // Only allow setting attempts for payments not yet succeeded or + // failed. + if _, ok := m.successful[phash]; ok { + return channeldb.ErrPaymentAlreadySucceeded + } + + if _, ok := m.failed[phash]; ok { + return channeldb.ErrPaymentAlreadyFailed + } + + p, ok := m.payments[phash] + if !ok { + return channeldb.ErrPaymentNotInitiated + } + + // Find the attempt with this pid, and set the settle info. + for i, a := range p.attempts { + if a.AttemptID != pid { + continue + } + + p.attempts[i].Settle = settleInfo + + // Mark the payment successful on first settled attempt. + m.successful[phash] = struct{}{} + return nil + } + + return fmt.Errorf("pid not found") +} + +func (m *mockControlTower) FailAttempt(phash lntypes.Hash, pid uint64, + failInfo *channeldb.HTLCFailInfo) error { + + m.Lock() + defer m.Unlock() + + // Only allow failing attempts for payments not yet succeeded or + // failed. + if _, ok := m.successful[phash]; ok { + return channeldb.ErrPaymentAlreadySucceeded + } + + if _, ok := m.failed[phash]; ok { + return channeldb.ErrPaymentAlreadyFailed + } + + p, ok := m.payments[phash] + if !ok { + return channeldb.ErrPaymentNotInitiated + } + + // Find the attempt with this pid, and set the failure info. + for i, a := range p.attempts { + if a.AttemptID != pid { + continue + } + + p.attempts[i].Failure = failInfo + return nil + } + + return fmt.Errorf("pid not found") } func (m *mockControlTower) Fail(phash lntypes.Hash, @@ -283,10 +365,50 @@ func (m *mockControlTower) Fail(phash lntypes.Hash, m.fail <- failArgs{reason} } - delete(m.inflights, phash) + // Cannot fail already successful or failed payments. + if _, ok := m.successful[phash]; ok { + return channeldb.ErrPaymentAlreadySucceeded + } + + if _, ok := m.failed[phash]; ok { + return channeldb.ErrPaymentAlreadyFailed + } + + if _, ok := m.payments[phash]; !ok { + return channeldb.ErrPaymentNotInitiated + } + + m.failed[phash] = reason + return nil } +func (m *mockControlTower) FetchPayment(phash lntypes.Hash) ( + *channeldb.MPPayment, error) { + + m.Lock() + defer m.Unlock() + + p, ok := m.payments[phash] + if !ok { + return nil, channeldb.ErrPaymentNotInitiated + } + + mp := &channeldb.MPPayment{ + Info: &p.info, + } + + reason, ok := m.failed[phash] + if ok { + mp.FailureReason = &reason + } + + // Return a copy of the current attempts. + mp.HTLCs = append(mp.HTLCs, p.attempts...) + + return mp, nil +} + func (m *mockControlTower) FetchInFlightPayments() ( []*channeldb.InFlightPayment, error) { @@ -297,8 +419,25 @@ func (m *mockControlTower) FetchInFlightPayments() ( m.fetchInFlight <- struct{}{} } + // In flight are all payments not successful or failed. var fl []*channeldb.InFlightPayment - for _, ifl := range m.inflights { + for hash, p := range m.payments { + if _, ok := m.successful[hash]; ok { + continue + } + if _, ok := m.failed[hash]; ok { + continue + } + + var attempts []channeldb.HTLCAttemptInfo + for _, a := range p.attempts { + attempts = append(attempts, a.HTLCAttemptInfo) + } + ifl := channeldb.InFlightPayment{ + Info: &p.info, + Attempts: attempts, + } + fl = append(fl, &ifl) } @@ -310,9 +449,3 @@ 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 -} From 9bcf41d40116eac9f65d852c3c1501f3ddd8a5df Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 10/34] channeldb: make FailureReason Error() --- channeldb/payments.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/channeldb/payments.go b/channeldb/payments.go index a1ea379b2..adf5cf8c2 100644 --- a/channeldb/payments.go +++ b/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: From bcca1ab82137e57c2e4868a52f513d3ad2e5d5b3 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 11/34] routing/payment_lifeycle: remove payment level attempt and circuit We replace the cached attempt, and instead use the control tower (database) to fetch any in-flight attempt. This is done as a preparation for having multiple attempts in flight. In addition we remove the cached circuit, as it won't be applicable when multiple shards are in flight. Instead of tracking the attemp we consult the database on every iteration, and pick up any existing attempt. This also let us avoid having to pass in the existing attempts from the payment loop, as we just fetch them direclty. --- routing/payment_lifecycle.go | 137 +++++++++++++++++++++-------------- routing/router.go | 25 ++----- 2 files changed, 90 insertions(+), 72 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index f2c533ec6..4deddefe0 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -37,8 +37,6 @@ type paymentLifecycle struct { paySession PaymentSession timeoutChan <-chan time.Time currentHeight int32 - attempt *channeldb.HTLCAttemptInfo - circuit *sphinx.Circuit lastError error } @@ -47,10 +45,52 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // We'll continue until either our payment succeeds, or we encounter a // critical error during path finding. for { + // We start every iteration by fetching the lastest state of + // the payment from the ControlTower. This ensures that we will + // act on the latest available information, whether we are + // resuming an existing payment or just sent a new attempt. + payment, err := p.router.cfg.Control.FetchPayment( + p.paymentHash, + ) + if err != nil { + return [32]byte{}, nil, err + } + + // Go through the HTLCs for this payment, determining if there + // are any in flight or settled. + var ( + attempt *channeldb.HTLCAttemptInfo + settle *channeldb.HTLCAttempt + ) + for _, a := range payment.HTLCs { + a := a + + // We have a settled HTLC, and should return when all + // shards are back. + if a.Settle != nil { + settle = &a + continue + } + + // This HTLC already failed, ignore. + if a.Failure != nil { + continue + } + + // HTLC was neither setteld nor failed, it is still in + // flight. + attempt = &a.HTLCAttemptInfo + break + } + + // Terminal state, return the preimage and the route taken. + if attempt == nil && settle != nil { + return settle.Settle.Preimage, &settle.Route, nil + } // If this payment had no existing payment attempt, we create // and send one now. - if p.attempt == nil { + if attempt == nil { // Before we attempt this next payment, we'll check to see if either // we've gone past the payment attempt timeout, or the router is // exiting. In either case, we'll stop this payment attempt short. If a @@ -114,7 +154,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // Using the route received from the payment session, // create a new shard to send. - firstHop, htlcAdd, attempt, err := p.createNewPaymentAttempt( + firstHop, htlcAdd, a, err := p.createNewPaymentAttempt( rt, ) // With SendToRoute, it can happen that the route exceeds protocol @@ -138,7 +178,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return [32]byte{}, nil, err } - p.attempt = attempt + attempt = a // Before sending this HTLC to the switch, we checkpoint the // fresh paymentID and route to the DB. This lets us know on @@ -153,11 +193,13 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // Now that the attempt is created and checkpointed to // the DB, we send it. - sendErr := p.sendPaymentAttempt(firstHop, htlcAdd) + sendErr := p.sendPaymentAttempt( + attempt, firstHop, htlcAdd, + ) if sendErr != nil { // TODO(joostjager): Distinguish unexpected // internal errors from real send errors. - err = p.failAttempt(sendErr) + err = p.failAttempt(attempt, sendErr) if err != nil { return [32]byte{}, nil, err } @@ -165,7 +207,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // We must inspect the error to know whether it // was critical or not, to decide whether we // should continue trying. - err := p.handleSendError(sendErr) + err := p.handleSendError(attempt, sendErr) if err != nil { return [32]byte{}, nil, err } @@ -173,35 +215,29 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // Error was handled successfully, reset the // attempt to indicate we want to make a new // attempt. - p.attempt = nil continue } - } else { - // If this was a resumed attempt, we must regenerate the - // circuit. We don't need to check for errors resulting - // from an invalid route, because the sphinx packet has - // been successfully generated before. - _, c, err := generateSphinxPacket( - &p.attempt.Route, p.paymentHash[:], - p.attempt.SessionKey, - ) - if err != nil { - return [32]byte{}, nil, err - } - p.circuit = c + } + + // Regenerate the circuit for this attempt. + _, circuit, err := generateSphinxPacket( + &attempt.Route, p.paymentHash[:], attempt.SessionKey, + ) + if err != nil { + return [32]byte{}, nil, err } // Using the created circuit, initialize the error decrypter so we can // parse+decode any failures incurred by this payment within the // switch. errorDecryptor := &htlcswitch.SphinxErrorDecrypter{ - OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(p.circuit), + OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(circuit), } // Now ask the switch to return the result of the payment when // available. resultChan, err := p.router.cfg.Payer.GetPaymentResult( - p.attempt.AttemptID, p.paymentHash, errorDecryptor, + attempt.AttemptID, p.paymentHash, errorDecryptor, ) switch { @@ -211,23 +247,22 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // attempt, and wait for its result to be available. case err == htlcswitch.ErrPaymentIDNotFound: log.Debugf("Payment ID %v for hash %x not found in "+ - "the Switch, retrying.", p.attempt.AttemptID, + "the Switch, retrying.", attempt.AttemptID, p.paymentHash) - err = p.failAttempt(err) + err = p.failAttempt(attempt, err) if err != nil { return [32]byte{}, nil, err } // Reset the attempt to indicate we want to make a new // attempt. - p.attempt = nil continue // A critical, unexpected error was encountered. case err != nil: log.Errorf("Failed getting result for attemptID %d "+ - "from switch: %v", p.attempt.AttemptID, err) + "from switch: %v", attempt.AttemptID, err) return [32]byte{}, nil, err } @@ -255,7 +290,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { log.Errorf("Attempt to send payment %x failed: %v", p.paymentHash, result.Error) - err = p.failAttempt(result.Error) + err = p.failAttempt(attempt, result.Error) if err != nil { return [32]byte{}, nil, err } @@ -263,23 +298,22 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // We must inspect the error to know whether it was // critical or not, to decide whether we should // continue trying. - if err := p.handleSendError(result.Error); err != nil { + if err := p.handleSendError(attempt, result.Error); err != nil { return [32]byte{}, nil, err } // Error was handled successfully, reset the attempt to // indicate we want to make a new attempt. - p.attempt = nil continue } // We successfully got a payment result back from the switch. log.Debugf("Payment %x succeeded with pid=%v", - p.paymentHash, p.attempt.AttemptID) + p.paymentHash, attempt.AttemptID) // Report success to mission control. err = p.router.cfg.MissionControl.ReportPaymentSuccess( - p.attempt.AttemptID, &p.attempt.Route, + attempt.AttemptID, &attempt.Route, ) if err != nil { log.Errorf("Error reporting payment success to mc: %v", @@ -289,7 +323,7 @@ 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.SettleAttempt( - p.paymentHash, p.attempt.AttemptID, + p.paymentHash, attempt.AttemptID, &channeldb.HTLCSettleInfo{ Preimage: result.Preimage, SettleTime: p.router.cfg.Clock.Now(), @@ -300,12 +334,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { "attempt: %v", err) return [32]byte{}, nil, err } - - // Terminal state, return the preimage and the route - // taken. - return result.Preimage, &p.attempt.Route, nil } - } // errorToPaymentFailure takes a path finding error and converts it into a @@ -341,17 +370,13 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) ( // Generate the raw encoded sphinx packet to be included along // with the htlcAdd message that we send directly to the // switch. - onionBlob, c, err := generateSphinxPacket( + onionBlob, _, err := generateSphinxPacket( rt, p.paymentHash[:], sessionKey, ) if err != nil { return lnwire.ShortChannelID{}, nil, nil, err } - // Update our cached circuit with the newly generated - // one. - p.circuit = c - // Craft an HTLC packet to send to the layer 2 switch. The // metadata within this packet will be used to route the // payment through the network, starting with the first-hop. @@ -389,13 +414,14 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) ( } // sendPaymentAttempt attempts to send the current attempt to the switch. -func (p *paymentLifecycle) sendPaymentAttempt(firstHop lnwire.ShortChannelID, +func (p *paymentLifecycle) sendPaymentAttempt( + attempt *channeldb.HTLCAttemptInfo, firstHop lnwire.ShortChannelID, htlcAdd *lnwire.UpdateAddHTLC) error { log.Tracef("Attempting to send payment %x (pid=%v), "+ - "using route: %v", p.paymentHash, p.attempt.AttemptID, + "using route: %v", p.paymentHash, attempt.AttemptID, newLogClosure(func() string { - return spew.Sdump(p.attempt.Route) + return spew.Sdump(attempt.Route) }), ) @@ -404,27 +430,28 @@ func (p *paymentLifecycle) sendPaymentAttempt(firstHop lnwire.ShortChannelID, // such that we can resume waiting for the result after a // restart. err := p.router.cfg.Payer.SendHTLC( - firstHop, p.attempt.AttemptID, htlcAdd, + firstHop, attempt.AttemptID, htlcAdd, ) if err != nil { log.Errorf("Failed sending attempt %d for payment "+ - "%x to switch: %v", p.attempt.AttemptID, + "%x to switch: %v", attempt.AttemptID, p.paymentHash, err) return err } log.Debugf("Payment %x (pid=%v) successfully sent to switch, route: %v", - p.paymentHash, p.attempt.AttemptID, &p.attempt.Route) + p.paymentHash, attempt.AttemptID, &attempt.Route) return nil } // handleSendError inspects the given error from the Switch and determines // whether we should make another payment attempt. -func (p *paymentLifecycle) handleSendError(sendErr error) error { +func (p *paymentLifecycle) handleSendError(attempt *channeldb.HTLCAttemptInfo, + sendErr error) error { reason := p.router.processSendError( - p.attempt.AttemptID, &p.attempt.Route, sendErr, + attempt.AttemptID, &attempt.Route, sendErr, ) if reason == nil { // Save the forwarding error so it can be returned if @@ -453,14 +480,16 @@ func (p *paymentLifecycle) handleSendError(sendErr error) error { } // failAttempt calls control tower to fail the current payment attempt. -func (p *paymentLifecycle) failAttempt(sendError error) error { +func (p *paymentLifecycle) failAttempt(attempt *channeldb.HTLCAttemptInfo, + sendError error) error { + failInfo := marshallError( sendError, p.router.cfg.Clock.Now(), ) return p.router.cfg.Control.FailAttempt( - p.paymentHash, p.attempt.AttemptID, + p.paymentHash, attempt.AttemptID, failInfo, ) } diff --git a/routing/router.go b/routing/router.go index 9595b0054..20c25a87d 100644 --- a/routing/router.go +++ b/routing/router.go @@ -533,20 +533,13 @@ func (r *ChannelRouter) Start() error { // result for the in-flight attempt is received. paySession := r.cfg.SessionSource.NewPaymentSessionEmpty() - // TODO(joostjager): For mpp, possibly relaunch multiple - // in-flight htlcs here. - var attempt *channeldb.HTLCAttemptInfo - if len(payment.Attempts) > 0 { - attempt = &payment.Attempts[0] - } - // We pass in a zero timeout value, to indicate we // don't need it to timeout. It will stop immediately // after the existing attempt has finished anyway. We // also set a zero fee limit, as no more routes should // be tried. _, _, err := r.sendPayment( - attempt, payment.Info.Value, 0, + payment.Info.Value, 0, payment.Info.PaymentHash, 0, paySession, ) if err != nil { @@ -1646,7 +1639,7 @@ func (r *ChannelRouter) SendPayment(payment *LightningPayment) ([32]byte, // Since this is the first time this payment is being made, we pass nil // for the existing attempt. return r.sendPayment( - nil, payment.Amount, payment.FeeLimit, payment.PaymentHash, + payment.Amount, payment.FeeLimit, payment.PaymentHash, payment.PayAttemptTimeout, paySession, ) } @@ -1669,9 +1662,8 @@ func (r *ChannelRouter) SendPaymentAsync(payment *LightningPayment) error { spewPayment(payment)) _, _, err := r.sendPayment( - nil, payment.Amount, payment.FeeLimit, - payment.PaymentHash, payment.PayAttemptTimeout, - paySession, + payment.Amount, payment.FeeLimit, payment.PaymentHash, + payment.PayAttemptTimeout, paySession, ) if err != nil { log.Errorf("Payment with hash %x failed: %v", @@ -1770,14 +1762,14 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // We set the timeout to a zero value, indicating the payment shouldn't // timeout. It is only a single attempt, so no more attempts will be - // done anyway. Since this is the first time this payment is being - // made, we pass nil for the existing attempt. + // done anyway. + // // We pass the route receiver amount as the total payment amount such // that the payment loop will request a route for this amount. As fee // limit we pass the route's total fees, since we already know this is // the route that is going to be used. preimage, _, err := r.sendPayment( - nil, amt, route.TotalFees(), hash, 0, paySession, + amt, route.TotalFees(), hash, 0, paySession, ) if err != nil { // SendToRoute should return a structured error. In case the @@ -1815,7 +1807,6 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( // router will call this method for every payment still in-flight according to // the ControlTower. func (r *ChannelRouter) sendPayment( - existingAttempt *channeldb.HTLCAttemptInfo, totalAmt, feeLimit lnwire.MilliSatoshi, paymentHash lntypes.Hash, timeout time.Duration, paySession PaymentSession) ([32]byte, *route.Route, error) { @@ -1836,8 +1827,6 @@ func (r *ChannelRouter) sendPayment( paymentHash: paymentHash, paySession: paySession, currentHeight: currentHeight, - attempt: existingAttempt, - circuit: nil, lastError: nil, } From 9712dd1a7fec8c6cc1cfd0b59a8fd38b77f891e3 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 12/34] routing/payment_lifecycle: extract attempt sending logic Define shardHandler which is a struct holding what is needed to send attempts along given routes. The reason we define the logic on this struct instead of the paymentLifecycle is that we later will make SendToRoute calls not go through the payment lifecycle, but only using this struct. The launch shard is responsible for registering the attempt with the control tower, failing it if the launch fails. Note that it is NOT responsible for marking the _payment_ failed in case a terminal error is encountered. This is important since we will later reuse this method for SendToRoute, where whether to fail the payment cannot be decided on the shard level. --- routing/payment_lifecycle.go | 145 ++++++++++++++++++++++++----------- routing/router.go | 1 - 2 files changed, 99 insertions(+), 47 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 4deddefe0..801dad2c9 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -37,11 +37,15 @@ type paymentLifecycle struct { paySession PaymentSession timeoutChan <-chan time.Time currentHeight int32 - lastError error } // resumePayment resumes the paymentLifecycle from the current state. func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { + shardHandler := &shardHandler{ + router: p.router, + paymentHash: p.paymentHash, + } + // We'll continue until either our payment succeeds, or we encounter a // critical error during path finding. for { @@ -144,19 +148,20 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // If there was an error already recorded for this // payment, we'll return that. - if p.lastError != nil { - return [32]byte{}, nil, errNoRoute{lastError: p.lastError} + if shardHandler.lastError != nil { + return [32]byte{}, nil, errNoRoute{ + lastError: shardHandler.lastError, + } } // Terminal state, return. return [32]byte{}, nil, err } - // Using the route received from the payment session, - // create a new shard to send. - firstHop, htlcAdd, a, err := p.createNewPaymentAttempt( - rt, - ) + // With the route in hand, launch a new shard. + var outcome *launchOutcome + attempt, outcome, err = shardHandler.launchShard(rt) + // With SendToRoute, it can happen that the route exceeds protocol // constraints. Mark the payment as failed with an internal error. if err == route.ErrMaxRouteHopsExceeded || @@ -178,43 +183,21 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return [32]byte{}, nil, err } - attempt = a - - // Before sending this HTLC to the switch, we checkpoint the - // fresh paymentID and route to the DB. This lets us know on - // startup the ID of the payment that we attempted to send, - // such that we can query the Switch for its whereabouts. The - // route is needed to handle the result when it eventually - // comes back. - err = p.router.cfg.Control.RegisterAttempt(p.paymentHash, attempt) - if err != nil { - return [32]byte{}, nil, err - } - - // Now that the attempt is created and checkpointed to - // the DB, we send it. - sendErr := p.sendPaymentAttempt( - attempt, firstHop, htlcAdd, - ) - if sendErr != nil { - // TODO(joostjager): Distinguish unexpected - // internal errors from real send errors. - err = p.failAttempt(attempt, sendErr) - if err != nil { - return [32]byte{}, nil, err - } - + // We ew encountered a non-critical error when + // launching the shard, handle it + if outcome.err != nil { // We must inspect the error to know whether it // was critical or not, to decide whether we // should continue trying. - err := p.handleSendError(attempt, sendErr) + err = shardHandler.handleSendError( + attempt, outcome.err, + ) if err != nil { return [32]byte{}, nil, err } - // Error was handled successfully, reset the - // attempt to indicate we want to make a new - // attempt. + // Error was handled successfully, continue to + // make a new attempt. continue } } @@ -250,7 +233,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { "the Switch, retrying.", attempt.AttemptID, p.paymentHash) - err = p.failAttempt(attempt, err) + err = shardHandler.failAttempt(attempt, err) if err != nil { return [32]byte{}, nil, err } @@ -290,7 +273,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { log.Errorf("Attempt to send payment %x failed: %v", p.paymentHash, result.Error) - err = p.failAttempt(attempt, result.Error) + err = shardHandler.failAttempt(attempt, result.Error) if err != nil { return [32]byte{}, nil, err } @@ -298,7 +281,10 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // We must inspect the error to know whether it was // critical or not, to decide whether we should // continue trying. - if err := p.handleSendError(attempt, result.Error); err != nil { + err := shardHandler.handleSendError( + attempt, result.Error, + ) + if err != nil { return [32]byte{}, nil, err } @@ -337,6 +323,74 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { } } +// shardHandler holds what is necessary to send and collect the result of +// shards. +type shardHandler struct { + paymentHash lntypes.Hash + router *ChannelRouter + + lastError error +} + +// launchOutcome is a type returned from launchShard that indicates whether the +// shard was successfully send onto the network. +type launchOutcome struct { + // err is non-nil if a non-critical error was encountered when trying + // to send the shard, and we successfully updated the control tower to + // reflect this error. This can be errors like not enough local + // balance for the given route etc. + err error +} + +// launchShard creates and sends an HTLC attempt along the given route, +// registering it with the control tower before sending it. It returns the +// HTLCAttemptInfo that was created for the shard, along with a launchOutcome. +// The launchOutcome is used to indicate whether the attempt was successfully +// sent. If the launchOutcome wraps a non-nil error, it means that the attempt +// was not sent onto the network, so no result will be available in the future +// for it. +func (p *shardHandler) launchShard(rt *route.Route) (*channeldb.HTLCAttemptInfo, + *launchOutcome, error) { + + // Using the route received from the payment session, create a new + // shard to send. + firstHop, htlcAdd, attempt, err := p.createNewPaymentAttempt( + rt, + ) + if err != nil { + return nil, nil, err + } + + // Before sending this HTLC to the switch, we checkpoint the fresh + // paymentID and route to the DB. This lets us know on startup the ID + // of the payment that we attempted to send, such that we can query the + // Switch for its whereabouts. The route is needed to handle the result + // when it eventually comes back. + err = p.router.cfg.Control.RegisterAttempt(p.paymentHash, attempt) + if err != nil { + return nil, nil, err + } + + // Now that the attempt is created and checkpointed to the DB, we send + // it. + sendErr := p.sendPaymentAttempt(attempt, firstHop, htlcAdd) + if sendErr != nil { + // TODO(joostjager): Distinguish unexpected internal errors + // from real send errors. + err := p.failAttempt(attempt, sendErr) + if err != nil { + return nil, nil, err + } + + // Return a launchOutcome indicating the shard failed. + return attempt, &launchOutcome{ + err: sendErr, + }, nil + } + + return attempt, &launchOutcome{}, nil +} + // errorToPaymentFailure takes a path finding error and converts it into a // payment-level failure. func errorToPaymentFailure(err error) channeldb.FailureReason { @@ -357,7 +411,7 @@ func errorToPaymentFailure(err error) channeldb.FailureReason { } // createNewPaymentAttempt creates a new payment attempt from the given route. -func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) ( +func (p *shardHandler) createNewPaymentAttempt(rt *route.Route) ( lnwire.ShortChannelID, *lnwire.UpdateAddHTLC, *channeldb.HTLCAttemptInfo, error) { @@ -414,7 +468,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route) ( } // sendPaymentAttempt attempts to send the current attempt to the switch. -func (p *paymentLifecycle) sendPaymentAttempt( +func (p *shardHandler) sendPaymentAttempt( attempt *channeldb.HTLCAttemptInfo, firstHop lnwire.ShortChannelID, htlcAdd *lnwire.UpdateAddHTLC) error { @@ -447,7 +501,7 @@ func (p *paymentLifecycle) sendPaymentAttempt( // handleSendError inspects the given error from the Switch and determines // whether we should make another payment attempt. -func (p *paymentLifecycle) handleSendError(attempt *channeldb.HTLCAttemptInfo, +func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo, sendErr error) error { reason := p.router.processSendError( @@ -457,7 +511,6 @@ func (p *paymentLifecycle) handleSendError(attempt *channeldb.HTLCAttemptInfo, // Save the forwarding error so it can be returned if // this turns out to be the last attempt. p.lastError = sendErr - return nil } @@ -480,7 +533,7 @@ func (p *paymentLifecycle) handleSendError(attempt *channeldb.HTLCAttemptInfo, } // failAttempt calls control tower to fail the current payment attempt. -func (p *paymentLifecycle) failAttempt(attempt *channeldb.HTLCAttemptInfo, +func (p *shardHandler) failAttempt(attempt *channeldb.HTLCAttemptInfo, sendError error) error { failInfo := marshallError( diff --git a/routing/router.go b/routing/router.go index 20c25a87d..117bf7cbb 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1827,7 +1827,6 @@ func (r *ChannelRouter) sendPayment( paymentHash: paymentHash, paySession: paySession, currentHeight: currentHeight, - lastError: nil, } // If a timeout is specified, create a timeout channel. If no timeout is From 2c01e79eb554d72c1b50603eebc00edb376fa3e4 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 13/34] routing/payment_lifecycle: extract result collection into collectResult Fetching the final shard result will also be done for calls to SendToRoute, so we extract this code into a new method. We move the call to the ControlTower to set the payment level failure out into the payment loop, as this must be handled differently when multiple shards are in flight, and for SendToRoute. --- routing/payment_lifecycle.go | 244 ++++++++++++++++++++--------------- 1 file changed, 139 insertions(+), 105 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 801dad2c9..c386c49f0 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -202,124 +202,28 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { } } - // Regenerate the circuit for this attempt. - _, circuit, err := generateSphinxPacket( - &attempt.Route, p.paymentHash[:], attempt.SessionKey, - ) + // Whether this was an existing attempt or one we just sent, + // we'll now collect its result. We ignore the result for now + // if it is a success, as we will look it up in the control + // tower on the next loop iteration. + result, err := shardHandler.collectResult(attempt) if err != nil { return [32]byte{}, nil, err } - // Using the created circuit, initialize the error decrypter so we can - // parse+decode any failures incurred by this payment within the - // switch. - errorDecryptor := &htlcswitch.SphinxErrorDecrypter{ - OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(circuit), - } - - // Now ask the switch to return the result of the payment when - // available. - resultChan, err := p.router.cfg.Payer.GetPaymentResult( - attempt.AttemptID, p.paymentHash, errorDecryptor, - ) - switch { - - // If this attempt ID is unknown to the Switch, it means it was - // never checkpointed and forwarded by the switch before a - // restart. In this case we can safely send a new payment - // attempt, and wait for its result to be available. - case err == htlcswitch.ErrPaymentIDNotFound: - log.Debugf("Payment ID %v for hash %x not found in "+ - "the Switch, retrying.", attempt.AttemptID, - p.paymentHash) - - err = shardHandler.failAttempt(attempt, err) - if err != nil { - return [32]byte{}, nil, err - } - - // Reset the attempt to indicate we want to make a new - // attempt. - continue - - // A critical, unexpected error was encountered. - case err != nil: - log.Errorf("Failed getting result for attemptID %d "+ - "from switch: %v", attempt.AttemptID, err) - - return [32]byte{}, nil, err - } - - // The switch knows about this payment, we'll wait for a result - // to be available. - var ( - result *htlcswitch.PaymentResult - ok bool - ) - - select { - case result, ok = <-resultChan: - if !ok { - return [32]byte{}, nil, htlcswitch.ErrSwitchExiting - } - - case <-p.router.quit: - return [32]byte{}, nil, ErrRouterShuttingDown - } - - // In case of a payment failure, we use the error to decide - // whether we should retry. - if result.Error != nil { - log.Errorf("Attempt to send payment %x failed: %v", - p.paymentHash, result.Error) - - err = shardHandler.failAttempt(attempt, result.Error) - if err != nil { - return [32]byte{}, nil, err - } - + if result.err != nil { // We must inspect the error to know whether it was // critical or not, to decide whether we should // continue trying. - err := shardHandler.handleSendError( - attempt, result.Error, - ) + err = shardHandler.handleSendError(attempt, result.err) if err != nil { return [32]byte{}, nil, err } - // Error was handled successfully, reset the attempt to - // indicate we want to make a new attempt. + // Error was handled successfully, continue to make a + // new attempt. continue } - - // We successfully got a payment result back from the switch. - log.Debugf("Payment %x succeeded with pid=%v", - p.paymentHash, attempt.AttemptID) - - // Report success to mission control. - err = p.router.cfg.MissionControl.ReportPaymentSuccess( - attempt.AttemptID, &attempt.Route, - ) - if err != nil { - log.Errorf("Error reporting payment success to mc: %v", - err) - } - - // In case of success we atomically store the db payment and - // move the payment to the success state. - err = p.router.cfg.Control.SettleAttempt( - p.paymentHash, 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) - return [32]byte{}, nil, err - } } } @@ -391,6 +295,136 @@ func (p *shardHandler) launchShard(rt *route.Route) (*channeldb.HTLCAttemptInfo, return attempt, &launchOutcome{}, nil } +// shardResult holds the resulting outcome of a shard sent. +type shardResult struct { + // preimage is the payment preimage in case of a settled HTLC. Only set + // if err is non-nil. + preimage lntypes.Preimage + + // err indicates that the shard failed. + err error +} + +// collectResult waits for the result for the given attempt to be available +// from the Switch, then records the attempt outcome with the control tower. A +// shardResult is returned, indicating the final outcome of this HTLC attempt. +func (p *shardHandler) collectResult(attempt *channeldb.HTLCAttemptInfo) ( + *shardResult, error) { + + // Regenerate the circuit for this attempt. + _, circuit, err := generateSphinxPacket( + &attempt.Route, p.paymentHash[:], + attempt.SessionKey, + ) + if err != nil { + return nil, err + } + + // Using the created circuit, initialize the error decrypter so we can + // parse+decode any failures incurred by this payment within the + // switch. + errorDecryptor := &htlcswitch.SphinxErrorDecrypter{ + OnionErrorDecrypter: sphinx.NewOnionErrorDecrypter(circuit), + } + + // Now ask the switch to return the result of the payment when + // available. + resultChan, err := p.router.cfg.Payer.GetPaymentResult( + attempt.AttemptID, p.paymentHash, errorDecryptor, + ) + switch { + + // If this attempt ID is unknown to the Switch, it means it was never + // checkpointed and forwarded by the switch before a restart. In this + // case we can safely send a new payment attempt, and wait for its + // result to be available. + case err == htlcswitch.ErrPaymentIDNotFound: + log.Debugf("Payment ID %v for hash %x not found in "+ + "the Switch, retrying.", attempt.AttemptID, + p.paymentHash) + + cErr := p.failAttempt(attempt, err) + if cErr != nil { + return nil, cErr + } + + return &shardResult{ + err: err, + }, nil + + // A critical, unexpected error was encountered. + case err != nil: + log.Errorf("Failed getting result for attemptID %d "+ + "from switch: %v", attempt.AttemptID, err) + + return nil, err + } + + // The switch knows about this payment, we'll wait for a result to be + // available. + var ( + result *htlcswitch.PaymentResult + ok bool + ) + + select { + case result, ok = <-resultChan: + if !ok { + return nil, htlcswitch.ErrSwitchExiting + } + + case <-p.router.quit: + return nil, ErrRouterShuttingDown + } + + // In case of a payment failure, fail the attempt with the control + // tower and return. + if result.Error != nil { + log.Errorf("Attempt to send payment %x failed: %v", + p.paymentHash, result.Error) + + err := p.failAttempt(attempt, result.Error) + if err != nil { + return nil, err + } + + return &shardResult{ + err: result.Error, + }, nil + } + + // We successfully got a payment result back from the switch. + log.Debugf("Payment %x succeeded with pid=%v", + p.paymentHash, attempt.AttemptID) + + // Report success to mission control. + err = p.router.cfg.MissionControl.ReportPaymentSuccess( + attempt.AttemptID, &attempt.Route, + ) + if err != nil { + log.Errorf("Error reporting payment success to mc: %v", + err) + } + + // In case of success we atomically store settle result to the DB move + // the shard to the settled state. + err = p.router.cfg.Control.SettleAttempt( + p.paymentHash, 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) + return nil, err + } + + return &shardResult{ + preimage: result.Preimage, + }, nil +} + // errorToPaymentFailure takes a path finding error and converts it into a // payment-level failure. func errorToPaymentFailure(err error) channeldb.FailureReason { From 6cc162e0b0155fca694f0395286dec8e1aa5853b Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 14/34] router: make sendToRoute omit payment lifecycle Instead of having SendToRoute pull routes from the payment session in the payment lifecycle, we utilize the new methods on the paymentShard to launch and collect the result for this single route. This also let us remove the check for noRouteError, as we will always have the result from the tried attempt returned. A result of this is that we can finally remove lastError from the payment lifecycle (see next commits). --- routing/router.go | 74 ++++++++++++++++++++++++++++++----------------- 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/routing/router.go b/routing/router.go index 117bf7cbb..2ceefe940 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1734,9 +1734,6 @@ func (r *ChannelRouter) preparePayment(payment *LightningPayment) ( func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( lntypes.Preimage, error) { - // Create a payment session for just this route. - paySession := r.cfg.SessionSource.NewPaymentSessionForRoute(route) - // Calculate amount paid to receiver. amt := route.ReceiverAmt() @@ -1760,34 +1757,57 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( }), ) - // We set the timeout to a zero value, indicating the payment shouldn't - // timeout. It is only a single attempt, so no more attempts will be - // done anyway. - // - // We pass the route receiver amount as the total payment amount such - // that the payment loop will request a route for this amount. As fee - // limit we pass the route's total fees, since we already know this is - // the route that is going to be used. - preimage, _, err := r.sendPayment( - amt, route.TotalFees(), hash, 0, paySession, - ) + // Launch a shard along the given route. + sh := &shardHandler{ + router: r, + paymentHash: hash, + } + + var shardError error + attempt, outcome, err := sh.launchShard(route) if err != nil { - // SendToRoute should return a structured error. In case the - // provided route fails, payment lifecycle will return a - // noRouteError with the structured error embedded. - if noRouteError, ok := err.(errNoRoute); ok { - if noRouteError.lastError == nil { - return lntypes.Preimage{}, - errors.New("failure message missing") - } - - return lntypes.Preimage{}, noRouteError.lastError - } - return lntypes.Preimage{}, err } - return preimage, nil + switch { + // Failed to launch shard. + case outcome.err != nil: + shardError = outcome.err + + // Shard successfully launched, wait for the result to be available. + default: + result, err := sh.collectResult(attempt) + if err != nil { + return lntypes.Preimage{}, err + } + + // We got a successful result. + if result.err == nil { + return result.preimage, nil + } + + // The shard failed, break switch to handle it. + shardError = result.err + } + + // Since for SendToRoute we won't retry in case the shard fails, we'll + // mark the payment failed with the control tower immediately. Process + // the error to check if it maps into a terminal error code, if not use + // a generic NO_ROUTE error. + reason := r.processSendError( + attempt.AttemptID, &attempt.Route, shardError, + ) + if reason == nil { + r := channeldb.FailureReasonNoRoute + reason = &r + } + + err = r.cfg.Control.Fail(hash, *reason) + if err != nil { + return lntypes.Preimage{}, err + } + + return lntypes.Preimage{}, shardError } // sendPayment attempts to send a payment to the passed payment hash. This From a979b91b27d6533db8c02862c73b79aec3f065df Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 15/34] routing: remove errNoRoute and lastError Now that SendToRoute is no longer using the payment lifecycle, we remove the error structs and vars used to cache the last encountered error. For SendToRoute this will now be returned directly after a shard has failed. For SendPayment this means that the last error encountered durinng pathfinding no longer will be returned. All errors encounterd can instead be inspected from the HTLC list. --- routing/payment_lifecycle.go | 27 --------------------------- routing/router_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 24 insertions(+), 28 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index c386c49f0..40089479f 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -13,20 +13,6 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) -// errNoRoute is returned when all routes from the payment session have been -// attempted. -type errNoRoute struct { - // lastError is the error encountered during the last payment attempt, - // if at least one attempt has been made. - lastError error -} - -// Error returns a string representation of the error. -func (e errNoRoute) Error() string { - return fmt.Sprintf("unable to route payment to destination: %v", - e.lastError) -} - // paymentLifecycle holds all information about the current state of a payment // needed to resume if from any point. type paymentLifecycle struct { @@ -146,14 +132,6 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return [32]byte{}, nil, saveErr } - // If there was an error already recorded for this - // payment, we'll return that. - if shardHandler.lastError != nil { - return [32]byte{}, nil, errNoRoute{ - lastError: shardHandler.lastError, - } - } - // Terminal state, return. return [32]byte{}, nil, err } @@ -232,8 +210,6 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { type shardHandler struct { paymentHash lntypes.Hash router *ChannelRouter - - lastError error } // launchOutcome is a type returned from launchShard that indicates whether the @@ -542,9 +518,6 @@ func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo, attempt.AttemptID, &attempt.Route, sendErr, ) if reason == nil { - // Save the forwarding error so it can be returned if - // this turns out to be the last attempt. - p.lastError = sendErr return nil } diff --git a/routing/router_test.go b/routing/router_test.go index d8550ac73..d3c866ae9 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -792,10 +792,33 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { // The final error returned should also indicate that the peer wasn't // online (the last error we returned). - if !strings.Contains(err.Error(), "UnknownNextPeer") { + // TODO: proper err code + if !strings.Contains(err.Error(), "unable to find") { t.Fatalf("expected UnknownNextPeer instead got: %v", err) } + // Inspect the two attempts that were made before the payment failed. + p, err := ctx.router.cfg.Control.FetchPayment(payHash) + if err != nil { + t.Fatal(err) + } + + if len(p.HTLCs) != 2 { + t.Fatalf("expected two attempts got %v", len(p.HTLCs)) + } + + // We expect the first attempt to have failed with a + // TemporaryChannelFailure, the second with UnknownNextPeer. + msg := p.HTLCs[0].Failure.Message + if _, ok := msg.(*lnwire.FailTemporaryChannelFailure); !ok { + t.Fatalf("unexpected fail message: %T", msg) + } + + msg = p.HTLCs[1].Failure.Message + if _, ok := msg.(*lnwire.FailUnknownNextPeer); !ok { + t.Fatalf("unexpected fail message: %T", msg) + } + ctx.router.cfg.MissionControl.(*MissionControl).ResetHistory() // Next, we'll modify the SendToSwitch method to indicate that the From 4509c4f3a9430074b8daabdd5b99510159c28760 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:24 +0200 Subject: [PATCH 16/34] routing: move ErrMaxRouteHopsExceeded check Now that SendToRoute is no longer using the payment lifecycle, we move the max hop check out of the payment shard's launch() method, and return the error directly, such that it can be handled in SendToRoute. --- routing/payment_lifecycle.go | 18 ------------------ routing/router.go | 26 ++++++++++++++++++++++---- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 40089479f..802a80b21 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -139,24 +139,6 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // With the route in hand, launch a new shard. var outcome *launchOutcome attempt, outcome, err = shardHandler.launchShard(rt) - - // With SendToRoute, it can happen that the route exceeds protocol - // constraints. Mark the payment as failed with an internal error. - if err == route.ErrMaxRouteHopsExceeded || - err == sphinx.ErrMaxRoutingInfoSizeExceeded { - - log.Debugf("Invalid route provided for payment %x: %v", - p.paymentHash, err) - - controlErr := p.router.cfg.Control.Fail( - p.paymentHash, channeldb.FailureReasonError, - ) - if controlErr != nil { - return [32]byte{}, nil, controlErr - } - } - - // In any case, don't continue if there is an error. if err != nil { return [32]byte{}, nil, err } diff --git a/routing/router.go b/routing/router.go index 2ceefe940..7f860c893 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1731,11 +1731,11 @@ func (r *ChannelRouter) preparePayment(payment *LightningPayment) ( // SendToRoute attempts to send a payment with the given hash through the // provided route. This function is blocking and will return the obtained // preimage if the payment is successful or the full error in case of a failure. -func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( +func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, rt *route.Route) ( lntypes.Preimage, error) { // Calculate amount paid to receiver. - amt := route.ReceiverAmt() + amt := rt.ReceiverAmt() // Record this payment hash with the ControlTower, ensuring it is not // already in-flight. @@ -1753,7 +1753,7 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( log.Tracef("Dispatching SendToRoute for hash %v: %v", hash, newLogClosure(func() string { - return spew.Sdump(route) + return spew.Sdump(rt) }), ) @@ -1764,7 +1764,25 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) ( } var shardError error - attempt, outcome, err := sh.launchShard(route) + attempt, outcome, err := sh.launchShard(rt) + + // With SendToRoute, it can happen that the route exceeds protocol + // constraints. Mark the payment as failed with an internal error. + if err == route.ErrMaxRouteHopsExceeded || + err == sphinx.ErrMaxRoutingInfoSizeExceeded { + + log.Debugf("Invalid route provided for payment %x: %v", + hash, err) + + controlErr := r.cfg.Control.Fail( + hash, channeldb.FailureReasonError, + ) + if controlErr != nil { + return [32]byte{}, controlErr + } + } + + // In any case, don't continue if there is an error. if err != nil { return lntypes.Preimage{}, err } From 49efbefb431b76bf438493d4f15b41af2c046e56 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:25 +0200 Subject: [PATCH 17/34] routing/payment_session: remove prebuilt payment session Since we no longer use payment sessions for send to route, we remove the prebuilt one. --- routing/payment_lifecycle.go | 2 +- routing/payment_session.go | 23 ++++++----------------- routing/payment_session_source.go | 14 ++------------ routing/router.go | 5 ----- 4 files changed, 9 insertions(+), 35 deletions(-) diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 802a80b21..473244c02 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -391,7 +391,7 @@ func errorToPaymentFailure(err error) channeldb.FailureReason { errNoTlvPayload, errNoPaymentAddr, errNoPathFound, - errPrebuiltRouteTried: + errEmptyPaySession: return channeldb.FailureReasonNoRoute diff --git a/routing/payment_session.go b/routing/payment_session.go index a5c042667..f8c297552 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -13,9 +13,9 @@ import ( const BlockPadding uint16 = 3 var ( - // errPrebuiltRouteTried is returned when the single pre-built route - // failed and there is nothing more we can do. - errPrebuiltRouteTried = errors.New("pre-built route already tried") + // errEmptyPaySession is returned when the empty payment session is + // queried for a route. + errEmptyPaySession = errors.New("empty payment session") ) // PaymentSession is used during SendPayment attempts to provide routes to @@ -50,8 +50,7 @@ type paymentSession struct { payment *LightningPayment - preBuiltRoute *route.Route - preBuiltRouteTried bool + empty bool pathFinder pathFinder } @@ -68,18 +67,8 @@ type paymentSession struct { func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, activeShards, height uint32) (*route.Route, error) { - switch { - - // If we have a pre-built route, use that directly. - case p.preBuiltRoute != nil && !p.preBuiltRouteTried: - p.preBuiltRouteTried = true - - return p.preBuiltRoute, nil - - // If the pre-built route has been tried already, the payment session is - // over. - case p.preBuiltRoute != nil: - return nil, errPrebuiltRouteTried + if p.empty { + return nil, errEmptyPaySession } // Add BlockPadding to the finalCltvDelta so that the receiving node diff --git a/routing/payment_session_source.go b/routing/payment_session_source.go index f3fd968e8..3d6cfedf2 100644 --- a/routing/payment_session_source.go +++ b/routing/payment_session_source.go @@ -75,23 +75,13 @@ func (m *SessionSource) NewPaymentSession(p *LightningPayment) ( }, nil } -// NewPaymentSessionForRoute creates a new paymentSession instance that is just -// used for failure reporting to missioncontrol. -func (m *SessionSource) NewPaymentSessionForRoute(preBuiltRoute *route.Route) PaymentSession { - return &paymentSession{ - sessionSource: m, - preBuiltRoute: preBuiltRoute, - } -} - // NewPaymentSessionEmpty creates a new paymentSession instance that is empty, // and will be exhausted immediately. Used for failure reporting to // missioncontrol for resumed payment we don't want to make more attempts for. func (m *SessionSource) NewPaymentSessionEmpty() PaymentSession { return &paymentSession{ - sessionSource: m, - preBuiltRoute: &route.Route{}, - preBuiltRouteTried: true, + sessionSource: m, + empty: true, } } diff --git a/routing/router.go b/routing/router.go index 7f860c893..a5cf84881 100644 --- a/routing/router.go +++ b/routing/router.go @@ -161,11 +161,6 @@ type PaymentSessionSource interface { // finding a path to the payment's destination. NewPaymentSession(p *LightningPayment) (PaymentSession, error) - // NewPaymentSessionForRoute creates a new paymentSession instance that - // is just used for failure reporting to missioncontrol, and will only - // attempt the given route. - NewPaymentSessionForRoute(preBuiltRoute *route.Route) PaymentSession - // NewPaymentSessionEmpty creates a new paymentSession instance that is // empty, and will be exhausted immediately. Used for failure reporting // to missioncontrol for resumed payment we don't want to make more From 7b5c10814b98886d6f65c6c652150e8a69d6f797 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:25 +0200 Subject: [PATCH 18/34] routing/payment_lifecycle+channeldb: collect existing outcome first To move towards how we will handle existing attempt in case of MPP (collecting their outcome will be done in separate goroutines separate from the payment loop), we move to collect their outcome first. To easily fetch HTLCs that are still not resolved, we add the utility method InFlightHTLCs to channeldb.MPPayment. --- channeldb/mp_payment.go | 15 ++++++++++ routing/payment_lifecycle.go | 55 ++++++++++++++++++++++++------------ 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/channeldb/mp_payment.go b/channeldb/mp_payment.go index d46e59091..fa58b4989 100644 --- a/channeldb/mp_payment.go +++ b/channeldb/mp_payment.go @@ -131,6 +131,21 @@ type MPPayment struct { Status PaymentStatus } +// 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 { diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 473244c02..dacd4fc44 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -32,6 +32,23 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { paymentHash: p.paymentHash, } + // If we have an existing attempt, we'll start by collecting its result. + payment, err := p.router.cfg.Control.FetchPayment( + p.paymentHash, + ) + if err != nil { + return [32]byte{}, nil, err + } + + for _, a := range payment.InFlightHTLCs() { + a := a + + _, err := shardHandler.collectResult(&a.HTLCAttemptInfo) + if err != nil { + return [32]byte{}, nil, err + } + } + // We'll continue until either our payment succeeds, or we encounter a // critical error during path finding. for { @@ -160,29 +177,31 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { // make a new attempt. continue } - } - // Whether this was an existing attempt or one we just sent, - // we'll now collect its result. We ignore the result for now - // if it is a success, as we will look it up in the control - // tower on the next loop iteration. - result, err := shardHandler.collectResult(attempt) - if err != nil { - return [32]byte{}, nil, err - } - - if result.err != nil { - // We must inspect the error to know whether it was - // critical or not, to decide whether we should - // continue trying. - err = shardHandler.handleSendError(attempt, result.err) + // We'll collect the result of the shard just sent. We + // ignore the result for now if it is a success, as we + // will look it up in the control tower on the next + // loop iteration. + result, err := shardHandler.collectResult(attempt) if err != nil { return [32]byte{}, nil, err } - // Error was handled successfully, continue to make a - // new attempt. - continue + if result.err != nil { + // We must inspect the error to know whether it + // was critical or not, to decide whether we + // should continue trying. + err := shardHandler.handleSendError( + attempt, result.err, + ) + if err != nil { + return [32]byte{}, nil, err + } + + // Error was handled successfully, continue to + // make a new attempt. + continue + } } } } From 5adfc968df004f1248b9b0ecaa263c4d8db22169 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:25 +0200 Subject: [PATCH 19/34] routing/payment_lifecycle: return recorded errors In preparation for MPP we return the terminal errors recorded with the control tower. The reason is that we cannot return immediately when a shard fails for MPP, since there might be more shards in flight that we must wait for. For that reason we instead mark the payment failed in the control tower, then return this error when we inspect the payment, seeing it has been failed and there are no shards in flight. --- lntest/itest/lnd_test.go | 91 ++++++++++++++++++++++-------------- routing/errors.go | 4 -- routing/payment_lifecycle.go | 30 ++++++------ routing/router_test.go | 6 +-- 4 files changed, 72 insertions(+), 59 deletions(-) diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 1a1133997..70febd9fa 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -1915,8 +1915,9 @@ func testUpdateChannelPolicy(net *lntest.NetworkHarness, t *harnessTest) { // Alice knows about the channel policy of Carol and should therefore // not be able to find a path during routing. + expErr := channeldb.FailureReasonNoRoute.Error() if err == nil || - !strings.Contains(err.Error(), "unable to find a path") { + !strings.Contains(err.Error(), expErr) { t.Fatalf("expected payment to fail, instead got %v", err) } @@ -3995,6 +3996,41 @@ func assertAmountSent(amt btcutil.Amount, sndr, rcvr *lntest.HarnessNode) func() } } +// assertLastHTLCError checks that the last sent HTLC of the last payment sent +// by the given node failed with the expected failure code. +func assertLastHTLCError(t *harnessTest, node *lntest.HarnessNode, + code lnrpc.Failure_FailureCode) { + + req := &lnrpc.ListPaymentsRequest{ + IncludeIncomplete: true, + } + ctxt, _ := context.WithTimeout(context.Background(), defaultTimeout) + paymentsResp, err := node.ListPayments(ctxt, req) + if err != nil { + t.Fatalf("error when obtaining payments: %v", err) + } + + payments := paymentsResp.Payments + if len(payments) == 0 { + t.Fatalf("no payments found") + } + + payment := payments[len(payments)-1] + htlcs := payment.Htlcs + if len(htlcs) == 0 { + t.Fatalf("no htlcs") + } + + htlc := htlcs[len(htlcs)-1] + if htlc.Failure == nil { + t.Fatalf("expected failure") + } + + if htlc.Failure.Code != code { + t.Fatalf("expected failure %v, got %v", code, htlc.Failure.Code) + } +} + // testSphinxReplayPersistence verifies that replayed onion packets are rejected // by a remote peer after a restart. We use a combination of unsafe // configuration arguments to force Carol to replay the same sphinx packet after @@ -4134,11 +4170,10 @@ func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) { // Construct the response we expect after sending a duplicate packet // that fails due to sphinx replay detection. - replayErr := "InvalidOnionKey" - if !strings.Contains(resp.PaymentError, replayErr) { - t.Fatalf("received payment error: %v, expected %v", - resp.PaymentError, replayErr) + if resp.PaymentError == "" { + t.Fatalf("expected payment error") } + assertLastHTLCError(t, carol, lnrpc.Failure_INVALID_ONION_KEY) // Since the payment failed, the balance should still be left // unaltered. @@ -9452,12 +9487,11 @@ out: t.Fatalf("payment should have been rejected due to invalid " + "payment hash") } - expectedErrorCode := lnwire.CodeIncorrectOrUnknownPaymentDetails.String() - if !strings.Contains(resp.PaymentError, expectedErrorCode) { - // TODO(roasbeef): make into proper gRPC error code - t.Fatalf("payment should have failed due to unknown payment hash, "+ - "instead failed due to: %v", resp.PaymentError) - } + + assertLastHTLCError( + t, net.Alice, + lnrpc.Failure_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, + ) // The balances of all parties should be the same as initially since // the HTLC was canceled. @@ -9484,18 +9518,11 @@ out: t.Fatalf("payment should have been rejected due to wrong " + "HTLC amount") } - expectedErrorCode = lnwire.CodeIncorrectOrUnknownPaymentDetails.String() - if !strings.Contains(resp.PaymentError, expectedErrorCode) { - t.Fatalf("payment should have failed due to wrong amount, "+ - "instead failed due to: %v", resp.PaymentError) - } - // We'll also ensure that the encoded error includes the invlaid HTLC - // amount. - if !strings.Contains(resp.PaymentError, htlcAmt.String()) { - t.Fatalf("error didn't include expected payment amt of %v: "+ - "%v", htlcAmt, resp.PaymentError) - } + assertLastHTLCError( + t, net.Alice, + lnrpc.Failure_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS, + ) // The balances of all parties should be the same as initially since // the HTLC was canceled. @@ -9574,12 +9601,12 @@ out: if resp.PaymentError == "" { t.Fatalf("payment should fail due to insufficient "+ "capacity: %v", err) - } else if !strings.Contains(resp.PaymentError, - lnwire.CodeTemporaryChannelFailure.String()) { - t.Fatalf("payment should fail due to insufficient capacity, "+ - "instead: %v", resp.PaymentError) } + assertLastHTLCError( + t, net.Alice, lnrpc.Failure_TEMPORARY_CHANNEL_FAILURE, + ) + // Generate new invoice to not pay same invoice twice. ctxt, _ = context.WithTimeout(ctxb, defaultTimeout) carolInvoice, err = carol.AddInvoice(ctxt, invoiceReq) @@ -9616,11 +9643,8 @@ out: if resp.PaymentError == "" { t.Fatalf("payment should have failed") } - expectedErrorCode = lnwire.CodeUnknownNextPeer.String() - if !strings.Contains(resp.PaymentError, expectedErrorCode) { - t.Fatalf("payment should fail due to unknown hop, instead: %v", - resp.PaymentError) - } + + assertLastHTLCError(t, net.Alice, lnrpc.Failure_UNKNOWN_NEXT_PEER) // Finally, immediately close the channel. This function will also // block until the channel is closed and will additionally assert the @@ -9787,9 +9811,8 @@ func testRejectHTLC(net *lntest.NetworkHarness, t *harnessTest) { "should have been rejected, carol will not accept forwarded htlcs", ) } - if !strings.Contains(err.Error(), lnwire.CodeChannelDisabled.String()) { - t.Fatalf("error returned should have been Channel Disabled") - } + + assertLastHTLCError(t, net.Alice, lnrpc.Failure_CHANNEL_DISABLED) // Close all channels. ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) diff --git a/routing/errors.go b/routing/errors.go index 3166f7390..3beac229a 100644 --- a/routing/errors.go +++ b/routing/errors.go @@ -15,10 +15,6 @@ const ( // this update can't bring us something new, or because a node // announcement was given for node not found in any channel. ErrIgnored - - // ErrPaymentAttemptTimeout is an error that indicates that a payment - // attempt timed out before we were able to successfully route an HTLC. - ErrPaymentAttemptTimeout ) // routerError is a structure that represent the error inside the routing package, diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index dacd4fc44..6e98eb1b0 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -1,7 +1,6 @@ package routing import ( - "fmt" "time" "github.com/davecgh/go-spew/spew" @@ -95,6 +94,12 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return settle.Settle.Preimage, &settle.Route, nil } + // If the payment already is failed, and there is no in-flight + // HTLC, return immediately. + if attempt == nil && payment.FailureReason != nil { + return [32]byte{}, nil, *payment.FailureReason + } + // If this payment had no existing payment attempt, we create // and send one now. if attempt == nil { @@ -113,10 +118,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return [32]byte{}, nil, err } - errStr := fmt.Sprintf("payment attempt not completed " + - "before timeout") - - return [32]byte{}, nil, newErr(ErrPaymentAttemptTimeout, errStr) + continue // The payment will be resumed from the current state // after restart. @@ -149,8 +151,7 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return [32]byte{}, nil, saveErr } - // Terminal state, return. - return [32]byte{}, nil, err + continue } // With the route in hand, launch a new shard. @@ -511,7 +512,9 @@ func (p *shardHandler) sendPaymentAttempt( } // handleSendError inspects the given error from the Switch and determines -// whether we should make another payment attempt. +// whether we should make another payment attempt, or if it should be +// considered a terminal error. Terminal errors will be recorded with the +// control tower. func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo, sendErr error) error { @@ -525,19 +528,12 @@ func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo, log.Debugf("Payment %x failed: final_outcome=%v, raw_err=%v", p.paymentHash, *reason, sendErr) - // Mark the payment failed with no route. - // - // TODO(halseth): make payment codes for the actual reason we don't - // continue path finding. - err := p.router.cfg.Control.Fail( - p.paymentHash, *reason, - ) + err := p.router.cfg.Control.Fail(p.paymentHash, *reason) if err != nil { return err } - // Terminal state, return the error we encountered. - return sendErr + return nil } // failAttempt calls control tower to fail the current payment attempt. diff --git a/routing/router_test.go b/routing/router_test.go index d3c866ae9..08499c5a3 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -7,7 +7,6 @@ import ( "image/color" "math" "math/rand" - "strings" "sync/atomic" "testing" "time" @@ -792,9 +791,8 @@ func TestSendPaymentErrorPathPruning(t *testing.T) { // The final error returned should also indicate that the peer wasn't // online (the last error we returned). - // TODO: proper err code - if !strings.Contains(err.Error(), "unable to find") { - t.Fatalf("expected UnknownNextPeer instead got: %v", err) + if err != channeldb.FailureReasonNoRoute { + t.Fatalf("expected no route instead got: %v", err) } // Inspect the two attempts that were made before the payment failed. From f6c97daf0caafc8120b2ea55ca29e200e371901d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:25 +0200 Subject: [PATCH 20/34] channeldb/payment_control_test: simplify HTLC status assertion --- channeldb/payment_control_test.go | 127 ++++++++++++++++++++---------- 1 file changed, 87 insertions(+), 40 deletions(-) diff --git a/channeldb/payment_control_test.go b/channeldb/payment_control_test.go index 272de68c9..1228d6631 100644 --- a/channeldb/payment_control_test.go +++ b/channeldb/payment_control_test.go @@ -53,7 +53,7 @@ func genInfo() (*PaymentCreationInfo, *HTLCAttemptInfo, PaymentRequest: []byte("hola"), }, &HTLCAttemptInfo{ - AttemptID: 1, + AttemptID: 0, SessionKey: priv, Route: testRoute, }, preimage, nil @@ -85,8 +85,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 +98,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 +110,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 +182,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 +222,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 +240,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 +257,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 +266,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 +379,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 +405,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 +424,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, ) } } @@ -452,11 +479,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 +519,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") } } From 70202be5806c0da07d069445676974b58b7b6dd8 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:25 +0200 Subject: [PATCH 21/34] channeldb: make database logic MPP compatible This commit redefines how the control tower handles shard and payment level settles and failures. We now consider the payment in flight as long it has active shards, or it has no active shards but has not reached a terminal condition (settle of one of the shards, or a payment level failure has been encountered). We also make it possible to settle/fail shards regardless of the payment level status (since we must allow late shards recording their status even though we have already settled/failed the payment). Finally, we make it possible to Fail the payment when it is already failed. This is to allow multiple concurrent shards that reach terminal errors to mark the payment failed, without havinng to synchronize. --- channeldb/mp_payment.go | 13 ++++ channeldb/payment_control.go | 116 ++++++++++++++++++++--------------- channeldb/payments.go | 46 ++++++++++++-- routing/mock_test.go | 46 +++++--------- 4 files changed, 138 insertions(+), 83 deletions(-) diff --git a/channeldb/mp_payment.go b/channeldb/mp_payment.go index fa58b4989..bf2f65ef3 100644 --- a/channeldb/mp_payment.go +++ b/channeldb/mp_payment.go @@ -131,6 +131,19 @@ 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 +} + // InFlightHTLCs returns the HTLCs that are still in-flight, meaning they have // not been settled or failed. func (m *MPPayment) InFlightHTLCs() []HTLCAttempt { diff --git a/channeldb/payment_control.go b/channeldb/payment_control.go index 64d7a3919..81b4d9440 100644 --- a/channeldb/payment_control.go +++ b/channeldb/payment_control.go @@ -18,22 +18,33 @@ 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") + // errNoAttemptInfo is returned when no attempt info is stored yet. errNoAttemptInfo = errors.New("unable to find attempt info for " + "inflight payment") @@ -52,7 +63,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 +179,21 @@ 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 + } + + settle, fail := payment.TerminalInfo() + if settle != nil || fail != nil { + return ErrPaymentTerminal + } + htlcsBucket, err := bucket.CreateBucketIfNotExists( paymentHtlcsBucket, ) @@ -241,8 +261,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 +284,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 +335,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 +362,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 +464,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 diff --git a/channeldb/payments.go b/channeldb/payments.go index adf5cf8c2..c07005ab4 100644 --- a/channeldb/payments.go +++ b/channeldb/payments.go @@ -260,12 +260,6 @@ 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 { @@ -297,6 +291,44 @@ func fetchPayment(bucket kvdb.ReadBucket) (*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 + } + return &MPPayment{ sequenceNum: sequenceNum, Info: creationInfo, @@ -412,6 +444,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 } diff --git a/routing/mock_test.go b/routing/mock_test.go index c66c70b1e..be62d9102 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -290,16 +290,7 @@ func (m *mockControlTower) SettleAttempt(phash lntypes.Hash, m.success <- successArgs{settleInfo.Preimage} } - // Only allow setting attempts for payments not yet succeeded or - // failed. - if _, ok := m.successful[phash]; ok { - return channeldb.ErrPaymentAlreadySucceeded - } - - if _, ok := m.failed[phash]; ok { - return channeldb.ErrPaymentAlreadyFailed - } - + // Only allow setting attempts if the payment is known. p, ok := m.payments[phash] if !ok { return channeldb.ErrPaymentNotInitiated @@ -311,6 +302,13 @@ func (m *mockControlTower) SettleAttempt(phash lntypes.Hash, continue } + if a.Settle != nil { + return channeldb.ErrAttemptAlreadySettled + } + if a.Failure != nil { + return channeldb.ErrAttemptAlreadyFailed + } + p.attempts[i].Settle = settleInfo // Mark the payment successful on first settled attempt. @@ -327,16 +325,7 @@ func (m *mockControlTower) FailAttempt(phash lntypes.Hash, pid uint64, m.Lock() defer m.Unlock() - // Only allow failing attempts for payments not yet succeeded or - // failed. - if _, ok := m.successful[phash]; ok { - return channeldb.ErrPaymentAlreadySucceeded - } - - if _, ok := m.failed[phash]; ok { - return channeldb.ErrPaymentAlreadyFailed - } - + // Only allow failing attempts if the payment is known. p, ok := m.payments[phash] if !ok { return channeldb.ErrPaymentNotInitiated @@ -348,6 +337,13 @@ func (m *mockControlTower) FailAttempt(phash lntypes.Hash, pid uint64, continue } + if a.Settle != nil { + return channeldb.ErrAttemptAlreadySettled + } + if a.Failure != nil { + return channeldb.ErrAttemptAlreadyFailed + } + p.attempts[i].Failure = failInfo return nil } @@ -365,15 +361,7 @@ func (m *mockControlTower) Fail(phash lntypes.Hash, m.fail <- failArgs{reason} } - // Cannot fail already successful or failed payments. - if _, ok := m.successful[phash]; ok { - return channeldb.ErrPaymentAlreadySucceeded - } - - if _, ok := m.failed[phash]; ok { - return channeldb.ErrPaymentAlreadyFailed - } - + // Payment must be known. if _, ok := m.payments[phash]; !ok { return channeldb.ErrPaymentNotInitiated } From 3610824abd46ddb5e42a13711b06ab0fa61a5424 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:25 +0200 Subject: [PATCH 22/34] channeldb/payment_control_test: add TestPaymentControlMultiShard --- channeldb/payment_control_test.go | 247 ++++++++++++++++++++++++++++++ 1 file changed, 247 insertions(+) diff --git a/channeldb/payment_control_test.go b/channeldb/payment_control_test.go index 1228d6631..015aa2318 100644 --- a/channeldb/payment_control_test.go +++ b/channeldb/payment_control_test.go @@ -458,6 +458,253 @@ 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. + 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, + ) + } + + // 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) + }) + } +} + // 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, From 4d343bbb468ee36f6737379ab1b6bd517e709225 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:26 +0200 Subject: [PATCH 23/34] routing tests: move TestRouterPaymentStateMachine to own file (almost) PURE CODE MOVE The only code change is to change a few select cases from case _ <- channel: to case <- channel: to please the linter. The test is testing the payment lifecycle, so move it to payment_lifecycle_test.go --- routing/payment_lifecycle_test.go | 653 ++++++++++++++++++++++++++++++ routing/router_test.go | 632 ----------------------------- 2 files changed, 653 insertions(+), 632 deletions(-) create mode 100644 routing/payment_lifecycle_test.go diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go new file mode 100644 index 000000000..addca0057 --- /dev/null +++ b/routing/payment_lifecycle_test.go @@ -0,0 +1,653 @@ +package routing + +import ( + "crypto/rand" + "fmt" + "sync/atomic" + "testing" + "time" + + "github.com/btcsuite/btcutil" + "github.com/go-errors/errors" + "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/htlcswitch" + "github.com/lightningnetwork/lnd/lntypes" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/routing/route" +) + +// TestRouterPaymentStateMachine tests that the router interacts as expected +// with the ControlTower during a payment lifecycle, such that it payment +// attempts are not sent twice to the switch, and results are handled after a +// restart. +func TestRouterPaymentStateMachine(t *testing.T) { + t.Parallel() + + const startingBlockHeight = 101 + + // Setup two simple channels such that we can mock sending along this + // route. + chanCapSat := btcutil.Amount(100000) + testChannels := []*testChannel{ + symmetricTestChannel("a", "b", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), + }, 1), + symmetricTestChannel("b", "c", chanCapSat, &testChannelPolicy{ + Expiry: 144, + FeeRate: 400, + MinHTLC: 1, + MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), + }, 2), + } + + testGraph, err := createTestGraphFromChannels(testChannels, "a") + if err != nil { + t.Fatalf("unable to create graph: %v", err) + } + defer testGraph.cleanUp() + + hop1 := testGraph.aliasMap["b"] + hop2 := testGraph.aliasMap["c"] + hops := []*route.Hop{ + { + ChannelID: 1, + PubKeyBytes: hop1, + LegacyPayload: true, + }, + { + ChannelID: 2, + PubKeyBytes: hop2, + LegacyPayload: true, + }, + } + + // We create a simple route that we will supply every time the router + // requests one. + rt, err := route.NewRouteFromHops( + lnwire.MilliSatoshi(10000), 100, testGraph.aliasMap["a"], hops, + ) + if err != nil { + t.Fatalf("unable to create route: %v", err) + } + + // A payment state machine test case consists of several ordered steps, + // that we use for driving the scenario. + type testCase struct { + // steps is a list of steps to perform during the testcase. + steps []string + + // routes is the sequence of routes we will provide to the + // router when it requests a new route. + routes []*route.Route + } + + const ( + // routerInitPayment is a test step where we expect the router + // to call the InitPayment method on the control tower. + routerInitPayment = "Router:init-payment" + + // routerRegisterAttempt is a test step where we expect the + // router to call the RegisterAttempt method on the control + // tower. + routerRegisterAttempt = "Router:register-attempt" + + // routerSuccess is a test step where we expect the router to + // call the Success method on the control tower. + routerSuccess = "Router:success" + + // routerFail is a test step where we expect the router to call + // the Fail method on the control tower. + routerFail = "Router:fail" + + // sendToSwitchSuccess is a step where we expect the router to + // call send the payment attempt to the switch, and we will + // respond with a non-error, indicating that the payment + // attempt was successfully forwarded. + sendToSwitchSuccess = "SendToSwitch:success" + + // sendToSwitchResultFailure is a step where we expect the + // router to send the payment attempt to the switch, and we + // will respond with a forwarding error. This can happen when + // forwarding fail on our local links. + sendToSwitchResultFailure = "SendToSwitch:failure" + + // getPaymentResultSuccess is a test step where we expect the + // router to call the GetPaymentResult method, and we will + // respond with a successful payment result. + getPaymentResultSuccess = "GetPaymentResult:success" + + // getPaymentResultFailure is a test step where we expect the + // router to call the GetPaymentResult method, and we will + // respond with a forwarding error. + getPaymentResultFailure = "GetPaymentResult:failure" + + // resendPayment is a test step where we manually try to resend + // the same payment, making sure the router responds with an + // error indicating that it is already in flight. + resendPayment = "ResendPayment" + + // startRouter is a step where we manually start the router, + // used to test that it automatically will resume payments at + // startup. + startRouter = "StartRouter" + + // stopRouter is a test step where we manually make the router + // shut down. + stopRouter = "StopRouter" + + // paymentSuccess is a step where assert that we receive a + // successful result for the original payment made. + paymentSuccess = "PaymentSuccess" + + // paymentError is a step where assert that we receive an error + // for the original payment made. + paymentError = "PaymentError" + + // resentPaymentSuccess is a step where assert that we receive + // a successful result for a payment that was resent. + resentPaymentSuccess = "ResentPaymentSuccess" + + // resentPaymentError is a step where assert that we receive an + // error for a payment that was resent. + resentPaymentError = "ResentPaymentError" + ) + + tests := []testCase{ + { + // Tests a normal payment flow that succeeds. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + sendToSwitchSuccess, + getPaymentResultSuccess, + routerSuccess, + paymentSuccess, + }, + routes: []*route.Route{rt}, + }, + { + // A payment flow with a failure on the first attempt, + // but that succeeds on the second attempt. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + sendToSwitchSuccess, + + // Make the first sent attempt fail. + getPaymentResultFailure, + + // The router should retry. + routerRegisterAttempt, + sendToSwitchSuccess, + + // Make the second sent attempt succeed. + getPaymentResultSuccess, + routerSuccess, + paymentSuccess, + }, + routes: []*route.Route{rt, rt}, + }, + { + // A payment flow with a forwarding failure first time + // sending to the switch, but that succeeds on the + // second attempt. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + + // Make the first sent attempt fail. + sendToSwitchResultFailure, + + // The router should retry. + routerRegisterAttempt, + sendToSwitchSuccess, + + // Make the second sent attempt succeed. + getPaymentResultSuccess, + routerSuccess, + paymentSuccess, + }, + routes: []*route.Route{rt, rt}, + }, + { + // A payment that fails on the first attempt, and has + // only one route available to try. It will therefore + // fail permanently. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + sendToSwitchSuccess, + + // Make the first sent attempt fail. + getPaymentResultFailure, + + // Since there are no more routes to try, the + // payment should fail. + routerFail, + paymentError, + }, + routes: []*route.Route{rt}, + }, + { + // We expect the payment to fail immediately if we have + // no routes to try. + steps: []string{ + routerInitPayment, + routerFail, + paymentError, + }, + routes: []*route.Route{}, + }, + { + // A normal payment flow, where we attempt to resend + // the same payment after each step. This ensures that + // the router don't attempt to resend a payment already + // in flight. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + + // Manually resend the payment, the router + // should attempt to init with the control + // tower, but fail since it is already in + // flight. + resendPayment, + routerInitPayment, + resentPaymentError, + + // The original payment should proceed as + // normal. + sendToSwitchSuccess, + + // Again resend the payment and assert it's not + // allowed. + resendPayment, + routerInitPayment, + resentPaymentError, + + // Notify about a success for the original + // payment. + getPaymentResultSuccess, + routerSuccess, + + // Now that the original payment finished, + // resend it again to ensure this is not + // allowed. + resendPayment, + routerInitPayment, + resentPaymentError, + paymentSuccess, + }, + routes: []*route.Route{rt}, + }, + { + // Tests that the router is able to handle the + // receieved payment result after a restart. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + sendToSwitchSuccess, + + // Shut down the router. The original caller + // should get notified about this. + stopRouter, + paymentError, + + // Start the router again, and ensure the + // router registers the success with the + // control tower. + startRouter, + getPaymentResultSuccess, + routerSuccess, + }, + routes: []*route.Route{rt}, + }, + { + // Tests that we are allowed to resend a payment after + // it has permanently failed. + steps: []string{ + routerInitPayment, + routerRegisterAttempt, + sendToSwitchSuccess, + + // Resending the payment at this stage should + // not be allowed. + resendPayment, + routerInitPayment, + resentPaymentError, + + // Make the first attempt fail. + getPaymentResultFailure, + routerFail, + + // Since we have no more routes to try, the + // original payment should fail. + paymentError, + + // Now resend the payment again. This should be + // allowed, since the payment has failed. + resendPayment, + routerInitPayment, + routerRegisterAttempt, + sendToSwitchSuccess, + getPaymentResultSuccess, + routerSuccess, + resentPaymentSuccess, + }, + routes: []*route.Route{rt}, + }, + } + + // Create a mock control tower with channels set up, that we use to + // synchronize and listen for events. + control := makeMockControlTower() + control.init = make(chan initArgs) + control.register = make(chan registerArgs) + control.success = make(chan successArgs) + control.fail = make(chan failArgs) + control.fetchInFlight = make(chan struct{}) + + quit := make(chan struct{}) + defer close(quit) + + // setupRouter is a helper method that creates and starts the router in + // the desired configuration for this test. + setupRouter := func() (*ChannelRouter, chan error, + chan *htlcswitch.PaymentResult, chan error) { + + chain := newMockChain(startingBlockHeight) + chainView := newMockChainView(chain) + + // We set uo the use the following channels and a mock Payer to + // synchonize with the interaction to the Switch. + sendResult := make(chan error) + paymentResultErr := make(chan error) + paymentResult := make(chan *htlcswitch.PaymentResult) + + payer := &mockPayer{ + sendResult: sendResult, + paymentResult: paymentResult, + paymentResultErr: paymentResultErr, + } + + router, err := New(Config{ + Graph: testGraph.graph, + Chain: chain, + ChainView: chainView, + Control: control, + SessionSource: &mockPaymentSessionSource{}, + MissionControl: &mockMissionControl{}, + Payer: payer, + ChannelPruneExpiry: time.Hour * 24, + GraphPruneInterval: time.Hour * 2, + QueryBandwidth: func(e *channeldb.ChannelEdgeInfo) lnwire.MilliSatoshi { + return lnwire.NewMSatFromSatoshis(e.Capacity) + }, + NextPaymentID: func() (uint64, error) { + next := atomic.AddUint64(&uniquePaymentID, 1) + return next, nil + }, + Clock: clock.NewTestClock(time.Unix(1, 0)), + }) + if err != nil { + t.Fatalf("unable to create router %v", err) + } + + // On startup, the router should fetch all pending payments + // from the ControlTower, so assert that here. + errCh := make(chan error) + go func() { + close(errCh) + select { + case <-control.fetchInFlight: + return + case <-time.After(1 * time.Second): + errCh <- errors.New("router did not fetch in flight " + + "payments") + } + }() + + if err := router.Start(); err != nil { + t.Fatalf("unable to start router: %v", err) + } + + select { + case err := <-errCh: + if err != nil { + t.Fatalf("error in anonymous goroutine: %s", err) + } + case <-time.After(1 * time.Second): + t.Fatalf("did not fetch in flight payments at startup") + } + + return router, sendResult, paymentResult, paymentResultErr + } + + router, sendResult, getPaymentResult, getPaymentResultErr := setupRouter() + defer func() { + if err := router.Stop(); err != nil { + t.Fatal(err) + } + }() + + for _, test := range tests { + // Craft a LightningPayment struct. + var preImage lntypes.Preimage + if _, err := rand.Read(preImage[:]); err != nil { + t.Fatalf("unable to generate preimage") + } + + payHash := preImage.Hash() + + paymentAmt := lnwire.NewMSatFromSatoshis(1000) + payment := LightningPayment{ + Target: testGraph.aliasMap["c"], + Amount: paymentAmt, + FeeLimit: noFeeLimit, + PaymentHash: payHash, + } + + router.cfg.SessionSource = &mockPaymentSessionSource{ + routes: test.routes, + } + + router.cfg.MissionControl = &mockMissionControl{} + + // Send the payment. Since this is new payment hash, the + // information should be registered with the ControlTower. + paymentResult := make(chan error) + go func() { + _, _, err := router.SendPayment(&payment) + paymentResult <- err + }() + + var resendResult chan error + for _, step := range test.steps { + switch step { + + case routerInitPayment: + var args initArgs + select { + case args = <-control.init: + case <-time.After(1 * time.Second): + t.Fatalf("no init payment with control") + } + + if args.c == nil { + t.Fatalf("expected non-nil CreationInfo") + } + + // In this step we expect the router to make a call to + // register a new attempt with the ControlTower. + case routerRegisterAttempt: + var args registerArgs + select { + case args = <-control.register: + case <-time.After(1 * time.Second): + t.Fatalf("not registered with control") + } + + if args.a == nil { + t.Fatalf("expected non-nil AttemptInfo") + } + + // In this step we expect the router to call the + // ControlTower's Succcess method with the preimage. + case routerSuccess: + select { + case <-control.success: + case <-time.After(1 * time.Second): + t.Fatalf("not registered with control") + } + + // In this step we expect the router to call the + // ControlTower's Fail method, to indicate that the + // payment failed. + case routerFail: + select { + case <-control.fail: + case <-time.After(1 * time.Second): + t.Fatalf("not registered with control") + } + + // In this step we expect the SendToSwitch method to be + // called, and we respond with a nil-error. + case sendToSwitchSuccess: + select { + case sendResult <- nil: + case <-time.After(1 * time.Second): + t.Fatalf("unable to send result") + } + + // In this step we expect the SendToSwitch method to be + // called, and we respond with a forwarding error + case sendToSwitchResultFailure: + select { + case sendResult <- htlcswitch.NewForwardingError( + &lnwire.FailTemporaryChannelFailure{}, + 1, + ): + case <-time.After(1 * time.Second): + t.Fatalf("unable to send result") + } + + // In this step we expect the GetPaymentResult method + // to be called, and we respond with the preimage to + // complete the payment. + case getPaymentResultSuccess: + select { + case getPaymentResult <- &htlcswitch.PaymentResult{ + Preimage: preImage, + }: + case <-time.After(1 * time.Second): + t.Fatalf("unable to send result") + } + + // In this state we expect the GetPaymentResult method + // to be called, and we respond with a forwarding + // error, indicating that the router should retry. + case getPaymentResultFailure: + failure := htlcswitch.NewForwardingError( + &lnwire.FailTemporaryChannelFailure{}, + 1, + ) + + select { + case getPaymentResult <- &htlcswitch.PaymentResult{ + Error: failure, + }: + case <-time.After(1 * time.Second): + t.Fatalf("unable to get result") + } + + // In this step we manually try to resend the same + // payment, making sure the router responds with an + // error indicating that it is already in flight. + case resendPayment: + resendResult = make(chan error) + go func() { + _, _, err := router.SendPayment(&payment) + resendResult <- err + }() + + // In this step we manually stop the router. + case stopRouter: + select { + case getPaymentResultErr <- fmt.Errorf( + "shutting down"): + case <-time.After(1 * time.Second): + t.Fatalf("unable to send payment " + + "result error") + } + + if err := router.Stop(); err != nil { + t.Fatalf("unable to restart: %v", err) + } + + // In this step we manually start the router. + case startRouter: + router, sendResult, getPaymentResult, + getPaymentResultErr = setupRouter() + + // In this state we expect to receive an error for the + // original payment made. + case paymentError: + select { + case err := <-paymentResult: + if err == nil { + t.Fatalf("expected error") + } + + case <-time.After(1 * time.Second): + t.Fatalf("got no payment result") + } + + // In this state we expect the original payment to + // succeed. + case paymentSuccess: + select { + case err := <-paymentResult: + if err != nil { + t.Fatalf("did not expecte error %v", err) + } + + case <-time.After(1 * time.Second): + t.Fatalf("got no payment result") + } + + // In this state we expect to receive an error for the + // resent payment made. + case resentPaymentError: + select { + case err := <-resendResult: + if err == nil { + t.Fatalf("expected error") + } + + case <-time.After(1 * time.Second): + t.Fatalf("got no payment result") + } + + // In this state we expect the resent payment to + // succeed. + case resentPaymentSuccess: + select { + case err := <-resendResult: + if err != nil { + t.Fatalf("did not expect error %v", err) + } + + case <-time.After(1 * time.Second): + t.Fatalf("got no payment result") + } + + default: + t.Fatalf("unknown step %v", step) + } + } + } +} diff --git a/routing/router_test.go b/routing/router_test.go index 08499c5a3..0bb90d715 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -2,7 +2,6 @@ package routing import ( "bytes" - "errors" "fmt" "image/color" "math" @@ -2616,637 +2615,6 @@ func assertChannelsPruned(t *testing.T, graph *channeldb.ChannelGraph, } } -// TestRouterPaymentStateMachine tests that the router interacts as expected -// with the ControlTower during a payment lifecycle, such that it payment -// attempts are not sent twice to the switch, and results are handled after a -// restart. -func TestRouterPaymentStateMachine(t *testing.T) { - t.Parallel() - - const startingBlockHeight = 101 - - // Setup two simple channels such that we can mock sending along this - // route. - chanCapSat := btcutil.Amount(100000) - testChannels := []*testChannel{ - symmetricTestChannel("a", "b", chanCapSat, &testChannelPolicy{ - Expiry: 144, - FeeRate: 400, - MinHTLC: 1, - MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), - }, 1), - symmetricTestChannel("b", "c", chanCapSat, &testChannelPolicy{ - Expiry: 144, - FeeRate: 400, - MinHTLC: 1, - MaxHTLC: lnwire.NewMSatFromSatoshis(chanCapSat), - }, 2), - } - - testGraph, err := createTestGraphFromChannels(testChannels, "a") - if err != nil { - t.Fatalf("unable to create graph: %v", err) - } - defer testGraph.cleanUp() - - hop1 := testGraph.aliasMap["b"] - hop2 := testGraph.aliasMap["c"] - hops := []*route.Hop{ - { - ChannelID: 1, - PubKeyBytes: hop1, - LegacyPayload: true, - }, - { - ChannelID: 2, - PubKeyBytes: hop2, - LegacyPayload: true, - }, - } - - // We create a simple route that we will supply every time the router - // requests one. - rt, err := route.NewRouteFromHops( - lnwire.MilliSatoshi(10000), 100, testGraph.aliasMap["a"], hops, - ) - if err != nil { - t.Fatalf("unable to create route: %v", err) - } - - // A payment state machine test case consists of several ordered steps, - // that we use for driving the scenario. - type testCase struct { - // steps is a list of steps to perform during the testcase. - steps []string - - // routes is the sequence of routes we will provide to the - // router when it requests a new route. - routes []*route.Route - } - - const ( - // routerInitPayment is a test step where we expect the router - // to call the InitPayment method on the control tower. - routerInitPayment = "Router:init-payment" - - // routerRegisterAttempt is a test step where we expect the - // router to call the RegisterAttempt method on the control - // tower. - routerRegisterAttempt = "Router:register-attempt" - - // routerSuccess is a test step where we expect the router to - // call the Success method on the control tower. - routerSuccess = "Router:success" - - // routerFail is a test step where we expect the router to call - // the Fail method on the control tower. - routerFail = "Router:fail" - - // sendToSwitchSuccess is a step where we expect the router to - // call send the payment attempt to the switch, and we will - // respond with a non-error, indicating that the payment - // attempt was successfully forwarded. - sendToSwitchSuccess = "SendToSwitch:success" - - // sendToSwitchResultFailure is a step where we expect the - // router to send the payment attempt to the switch, and we - // will respond with a forwarding error. This can happen when - // forwarding fail on our local links. - sendToSwitchResultFailure = "SendToSwitch:failure" - - // getPaymentResultSuccess is a test step where we expect the - // router to call the GetPaymentResult method, and we will - // respond with a successful payment result. - getPaymentResultSuccess = "GetPaymentResult:success" - - // getPaymentResultFailure is a test step where we expect the - // router to call the GetPaymentResult method, and we will - // respond with a forwarding error. - getPaymentResultFailure = "GetPaymentResult:failure" - - // resendPayment is a test step where we manually try to resend - // the same payment, making sure the router responds with an - // error indicating that it is alreayd in flight. - resendPayment = "ResendPayment" - - // startRouter is a step where we manually start the router, - // used to test that it automatically will resume payments at - // startup. - startRouter = "StartRouter" - - // stopRouter is a test step where we manually make the router - // shut down. - stopRouter = "StopRouter" - - // paymentSuccess is a step where assert that we receive a - // successful result for the original payment made. - paymentSuccess = "PaymentSuccess" - - // paymentError is a step where assert that we receive an error - // for the original payment made. - paymentError = "PaymentError" - - // resentPaymentSuccess is a step where assert that we receive - // a successful result for a payment that was resent. - resentPaymentSuccess = "ResentPaymentSuccess" - - // resentPaymentError is a step where assert that we receive an - // error for a payment that was resent. - resentPaymentError = "ResentPaymentError" - ) - - tests := []testCase{ - { - // Tests a normal payment flow that succeeds. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - sendToSwitchSuccess, - getPaymentResultSuccess, - routerSuccess, - paymentSuccess, - }, - routes: []*route.Route{rt}, - }, - { - // A payment flow with a failure on the first attempt, - // but that succeeds on the second attempt. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - sendToSwitchSuccess, - - // Make the first sent attempt fail. - getPaymentResultFailure, - - // The router should retry. - routerRegisterAttempt, - sendToSwitchSuccess, - - // Make the second sent attempt succeed. - getPaymentResultSuccess, - routerSuccess, - paymentSuccess, - }, - routes: []*route.Route{rt, rt}, - }, - { - // A payment flow with a forwarding failure first time - // sending to the switch, but that succeeds on the - // second attempt. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - - // Make the first sent attempt fail. - sendToSwitchResultFailure, - - // The router should retry. - routerRegisterAttempt, - sendToSwitchSuccess, - - // Make the second sent attempt succeed. - getPaymentResultSuccess, - routerSuccess, - paymentSuccess, - }, - routes: []*route.Route{rt, rt}, - }, - { - // A payment that fails on the first attempt, and has - // only one route available to try. It will therefore - // fail permanently. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - sendToSwitchSuccess, - - // Make the first sent attempt fail. - getPaymentResultFailure, - - // Since there are no more routes to try, the - // payment should fail. - routerFail, - paymentError, - }, - routes: []*route.Route{rt}, - }, - { - // We expect the payment to fail immediately if we have - // no routes to try. - steps: []string{ - routerInitPayment, - routerFail, - paymentError, - }, - routes: []*route.Route{}, - }, - { - // A normal payment flow, where we attempt to resend - // the same payment after each step. This ensures that - // the router don't attempt to resend a payment already - // in flight. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - - // Manually resend the payment, the router - // should attempt to init with the control - // tower, but fail since it is already in - // flight. - resendPayment, - routerInitPayment, - resentPaymentError, - - // The original payment should proceed as - // normal. - sendToSwitchSuccess, - - // Again resend the payment and assert it's not - // allowed. - resendPayment, - routerInitPayment, - resentPaymentError, - - // Notify about a success for the original - // payment. - getPaymentResultSuccess, - routerSuccess, - - // Now that the original payment finished, - // resend it again to ensure this is not - // allowed. - resendPayment, - routerInitPayment, - resentPaymentError, - paymentSuccess, - }, - routes: []*route.Route{rt}, - }, - { - // Tests that the router is able to handle the - // receieved payment result after a restart. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - sendToSwitchSuccess, - - // Shut down the router. The original caller - // should get notified about this. - stopRouter, - paymentError, - - // Start the router again, and ensure the - // router registers the success with the - // control tower. - startRouter, - getPaymentResultSuccess, - routerSuccess, - }, - routes: []*route.Route{rt}, - }, - { - // Tests that we are allowed to resend a payment after - // it has permanently failed. - steps: []string{ - routerInitPayment, - routerRegisterAttempt, - sendToSwitchSuccess, - - // Resending the payment at this stage should - // not be allowed. - resendPayment, - routerInitPayment, - resentPaymentError, - - // Make the first attempt fail. - getPaymentResultFailure, - routerFail, - - // Since we have no more routes to try, the - // original payment should fail. - paymentError, - - // Now resend the payment again. This should be - // allowed, since the payment has failed. - resendPayment, - routerInitPayment, - routerRegisterAttempt, - sendToSwitchSuccess, - getPaymentResultSuccess, - routerSuccess, - resentPaymentSuccess, - }, - routes: []*route.Route{rt}, - }, - } - - // Create a mock control tower with channels set up, that we use to - // synchronize and listen for events. - control := makeMockControlTower() - control.init = make(chan initArgs) - control.register = make(chan registerArgs) - control.success = make(chan successArgs) - control.fail = make(chan failArgs) - control.fetchInFlight = make(chan struct{}) - - quit := make(chan struct{}) - defer close(quit) - - // setupRouter is a helper method that creates and starts the router in - // the desired configuration for this test. - setupRouter := func() (*ChannelRouter, chan error, - chan *htlcswitch.PaymentResult, chan error) { - - chain := newMockChain(startingBlockHeight) - chainView := newMockChainView(chain) - - // We set uo the use the following channels and a mock Payer to - // synchonize with the interaction to the Switch. - sendResult := make(chan error) - paymentResultErr := make(chan error) - paymentResult := make(chan *htlcswitch.PaymentResult) - - payer := &mockPayer{ - sendResult: sendResult, - paymentResult: paymentResult, - paymentResultErr: paymentResultErr, - } - - router, err := New(Config{ - Graph: testGraph.graph, - Chain: chain, - ChainView: chainView, - Control: control, - SessionSource: &mockPaymentSessionSource{}, - MissionControl: &mockMissionControl{}, - Payer: payer, - ChannelPruneExpiry: time.Hour * 24, - GraphPruneInterval: time.Hour * 2, - QueryBandwidth: func(e *channeldb.ChannelEdgeInfo) lnwire.MilliSatoshi { - return lnwire.NewMSatFromSatoshis(e.Capacity) - }, - NextPaymentID: func() (uint64, error) { - next := atomic.AddUint64(&uniquePaymentID, 1) - return next, nil - }, - Clock: clock.NewTestClock(time.Unix(1, 0)), - }) - if err != nil { - t.Fatalf("unable to create router %v", err) - } - - // On startup, the router should fetch all pending payments - // from the ControlTower, so assert that here. - errCh := make(chan error) - go func() { - close(errCh) - select { - case <-control.fetchInFlight: - return - case <-time.After(1 * time.Second): - errCh <- errors.New("router did not fetch in flight " + - "payments") - } - }() - - if err := router.Start(); err != nil { - t.Fatalf("unable to start router: %v", err) - } - - select { - case err := <-errCh: - if err != nil { - t.Fatalf("error in anonymous goroutine: %s", err) - } - case <-time.After(1 * time.Second): - t.Fatalf("did not fetch in flight payments at startup") - } - - return router, sendResult, paymentResult, paymentResultErr - } - - router, sendResult, getPaymentResult, getPaymentResultErr := setupRouter() - defer router.Stop() - - for _, test := range tests { - // Craft a LightningPayment struct. - var preImage lntypes.Preimage - if _, err := rand.Read(preImage[:]); err != nil { - t.Fatalf("unable to generate preimage") - } - - payHash := preImage.Hash() - - paymentAmt := lnwire.NewMSatFromSatoshis(1000) - payment := LightningPayment{ - Target: testGraph.aliasMap["c"], - Amount: paymentAmt, - FeeLimit: noFeeLimit, - PaymentHash: payHash, - } - - router.cfg.SessionSource = &mockPaymentSessionSource{ - routes: test.routes, - } - - router.cfg.MissionControl = &mockMissionControl{} - - // Send the payment. Since this is new payment hash, the - // information should be registered with the ControlTower. - paymentResult := make(chan error) - go func() { - _, _, err := router.SendPayment(&payment) - paymentResult <- err - }() - - var resendResult chan error - for _, step := range test.steps { - switch step { - - case routerInitPayment: - var args initArgs - select { - case args = <-control.init: - case <-time.After(1 * time.Second): - t.Fatalf("no init payment with control") - } - - if args.c == nil { - t.Fatalf("expected non-nil CreationInfo") - } - - // In this step we expect the router to make a call to - // register a new attempt with the ControlTower. - case routerRegisterAttempt: - var args registerArgs - select { - case args = <-control.register: - case <-time.After(1 * time.Second): - t.Fatalf("not registered with control") - } - - if args.a == nil { - t.Fatalf("expected non-nil AttemptInfo") - } - - // In this step we expect the router to call the - // ControlTower's Succcess method with the preimage. - case routerSuccess: - select { - case _ = <-control.success: - case <-time.After(1 * time.Second): - t.Fatalf("not registered with control") - } - - // In this step we expect the router to call the - // ControlTower's Fail method, to indicate that the - // payment failed. - case routerFail: - select { - case _ = <-control.fail: - case <-time.After(1 * time.Second): - t.Fatalf("not registered with control") - } - - // In this step we expect the SendToSwitch method to be - // called, and we respond with a nil-error. - case sendToSwitchSuccess: - select { - case sendResult <- nil: - case <-time.After(1 * time.Second): - t.Fatalf("unable to send result") - } - - // In this step we expect the SendToSwitch method to be - // called, and we respond with a forwarding error - case sendToSwitchResultFailure: - select { - case sendResult <- htlcswitch.NewForwardingError( - &lnwire.FailTemporaryChannelFailure{}, - 1, - ): - case <-time.After(1 * time.Second): - t.Fatalf("unable to send result") - } - - // In this step we expect the GetPaymentResult method - // to be called, and we respond with the preimage to - // complete the payment. - case getPaymentResultSuccess: - select { - case getPaymentResult <- &htlcswitch.PaymentResult{ - Preimage: preImage, - }: - case <-time.After(1 * time.Second): - t.Fatalf("unable to send result") - } - - // In this state we expect the GetPaymentResult method - // to be called, and we respond with a forwarding - // error, indicating that the router should retry. - case getPaymentResultFailure: - failure := htlcswitch.NewForwardingError( - &lnwire.FailTemporaryChannelFailure{}, - 1, - ) - - select { - case getPaymentResult <- &htlcswitch.PaymentResult{ - Error: failure, - }: - case <-time.After(1 * time.Second): - t.Fatalf("unable to get result") - } - - // In this step we manually try to resend the same - // payment, making sure the router responds with an - // error indicating that it is alreayd in flight. - case resendPayment: - resendResult = make(chan error) - go func() { - _, _, err := router.SendPayment(&payment) - resendResult <- err - }() - - // In this step we manually stop the router. - case stopRouter: - select { - case getPaymentResultErr <- fmt.Errorf( - "shutting down"): - case <-time.After(1 * time.Second): - t.Fatalf("unable to send payment " + - "result error") - } - - if err := router.Stop(); err != nil { - t.Fatalf("unable to restart: %v", err) - } - - // In this step we manually start the router. - case startRouter: - router, sendResult, getPaymentResult, - getPaymentResultErr = setupRouter() - - // In this state we expect to receive an error for the - // original payment made. - case paymentError: - select { - case err := <-paymentResult: - if err == nil { - t.Fatalf("expected error") - } - - case <-time.After(1 * time.Second): - t.Fatalf("got no payment result") - } - - // In this state we expect the original payment to - // succeed. - case paymentSuccess: - select { - case err := <-paymentResult: - if err != nil { - t.Fatalf("did not expecte error %v", err) - } - - case <-time.After(1 * time.Second): - t.Fatalf("got no payment result") - } - - // In this state we expect to receive an error for the - // resent payment made. - case resentPaymentError: - select { - case err := <-resendResult: - if err == nil { - t.Fatalf("expected error") - } - - case <-time.After(1 * time.Second): - t.Fatalf("got no payment result") - } - - // In this state we expect the resent payment to - // succeed. - case resentPaymentSuccess: - select { - case err := <-resendResult: - if err != nil { - t.Fatalf("did not expect error %v", err) - } - - case <-time.After(1 * time.Second): - t.Fatalf("got no payment result") - } - - default: - t.Fatalf("unknown step %v", step) - } - } - } -} - // TestSendToRouteStructuredError asserts that SendToRoute returns a structured // error. func TestSendToRouteStructuredError(t *testing.T) { From aa9c971dc0ebe1f05866187ec90336d8352af16a Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:26 +0200 Subject: [PATCH 24/34] routing/payment_lifecycle_test: extract route creation into method This also fixes a test bug that the manually created route didn't match the actual payment amount in the test cases, and adds some fees to the route. --- routing/payment_lifecycle_test.go | 49 +++++++++++++++++++------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index addca0057..5e62c33f9 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -17,6 +17,35 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) +// createTestRoute builds a route a->b->c paying the given amt to c. +func createTestRoute(amt lnwire.MilliSatoshi, + aliasMap map[string]route.Vertex) (*route.Route, error) { + + hopFee := lnwire.NewMSatFromSatoshis(3) + hop1 := aliasMap["b"] + hop2 := aliasMap["c"] + hops := []*route.Hop{ + { + ChannelID: 1, + PubKeyBytes: hop1, + LegacyPayload: true, + AmtToForward: amt + hopFee, + }, + { + ChannelID: 2, + PubKeyBytes: hop2, + LegacyPayload: true, + AmtToForward: amt, + }, + } + + // We create a simple route that we will supply every time the router + // requests one. + return route.NewRouteFromHops( + amt+2*hopFee, 100, aliasMap["a"], hops, + ) +} + // TestRouterPaymentStateMachine tests that the router interacts as expected // with the ControlTower during a payment lifecycle, such that it payment // attempts are not sent twice to the switch, and results are handled after a @@ -50,26 +79,11 @@ func TestRouterPaymentStateMachine(t *testing.T) { } defer testGraph.cleanUp() - hop1 := testGraph.aliasMap["b"] - hop2 := testGraph.aliasMap["c"] - hops := []*route.Hop{ - { - ChannelID: 1, - PubKeyBytes: hop1, - LegacyPayload: true, - }, - { - ChannelID: 2, - PubKeyBytes: hop2, - LegacyPayload: true, - }, - } + paymentAmt := lnwire.NewMSatFromSatoshis(1000) // We create a simple route that we will supply every time the router // requests one. - rt, err := route.NewRouteFromHops( - lnwire.MilliSatoshi(10000), 100, testGraph.aliasMap["a"], hops, - ) + rt, err := createTestRoute(paymentAmt, testGraph.aliasMap) if err != nil { t.Fatalf("unable to create route: %v", err) } @@ -443,7 +457,6 @@ func TestRouterPaymentStateMachine(t *testing.T) { payHash := preImage.Hash() - paymentAmt := lnwire.NewMSatFromSatoshis(1000) payment := LightningPayment{ Target: testGraph.aliasMap["c"], Amount: paymentAmt, From 2e63b518b78e90db053efae9c8b56f58a171c3f7 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:26 +0200 Subject: [PATCH 25/34] routing/payment_lifecycle_test+mock: set up listener for FailAttempt Also rename Success to SettleAttempt in the tests. --- routing/mock_test.go | 37 ++++++++------ routing/payment_lifecycle_test.go | 83 ++++++++++++++++++++----------- 2 files changed, 77 insertions(+), 43 deletions(-) diff --git a/routing/mock_test.go b/routing/mock_test.go index be62d9102..594c14834 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -176,15 +176,19 @@ type initArgs struct { c *channeldb.PaymentCreationInfo } -type registerArgs struct { +type registerAttemptArgs struct { a *channeldb.HTLCAttemptInfo } -type successArgs struct { +type settleAttemptArgs struct { preimg lntypes.Preimage } -type failArgs struct { +type failAttemptArgs struct { + reason *channeldb.HTLCFailInfo +} + +type failPaymentArgs struct { reason channeldb.FailureReason } @@ -198,11 +202,12 @@ type mockControlTower struct { successful map[lntypes.Hash]struct{} failed map[lntypes.Hash]channeldb.FailureReason - init chan initArgs - register chan registerArgs - success chan successArgs - fail chan failArgs - fetchInFlight chan struct{} + init chan initArgs + registerAttempt chan registerAttemptArgs + settleAttempt chan settleAttemptArgs + failAttempt chan failAttemptArgs + failPayment chan failPaymentArgs + fetchInFlight chan struct{} sync.Mutex } @@ -254,8 +259,8 @@ func (m *mockControlTower) RegisterAttempt(phash lntypes.Hash, m.Lock() defer m.Unlock() - if m.register != nil { - m.register <- registerArgs{a} + if m.registerAttempt != nil { + m.registerAttempt <- registerAttemptArgs{a} } // Cannot register attempts for successful or failed payments. @@ -286,8 +291,8 @@ func (m *mockControlTower) SettleAttempt(phash lntypes.Hash, m.Lock() defer m.Unlock() - if m.success != nil { - m.success <- successArgs{settleInfo.Preimage} + if m.settleAttempt != nil { + m.settleAttempt <- settleAttemptArgs{settleInfo.Preimage} } // Only allow setting attempts if the payment is known. @@ -325,6 +330,10 @@ func (m *mockControlTower) FailAttempt(phash lntypes.Hash, pid uint64, m.Lock() defer m.Unlock() + if m.failAttempt != nil { + m.failAttempt <- failAttemptArgs{failInfo} + } + // Only allow failing attempts if the payment is known. p, ok := m.payments[phash] if !ok { @@ -357,8 +366,8 @@ func (m *mockControlTower) Fail(phash lntypes.Hash, m.Lock() defer m.Unlock() - if m.fail != nil { - m.fail <- failArgs{reason} + if m.failPayment != nil { + m.failPayment <- failPaymentArgs{reason} } // Payment must be known. diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index 5e62c33f9..4c371b066 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -109,13 +109,18 @@ func TestRouterPaymentStateMachine(t *testing.T) { // tower. routerRegisterAttempt = "Router:register-attempt" - // routerSuccess is a test step where we expect the router to - // call the Success method on the control tower. - routerSuccess = "Router:success" + // routerSettleAttempt is a test step where we expect the + // router to call the SettleAttempt method on the control + // tower. + routerSettleAttempt = "Router:settle-attempt" - // routerFail is a test step where we expect the router to call - // the Fail method on the control tower. - routerFail = "Router:fail" + // routerFailAttempt is a test step where we expect the router + // to call the FailAttempt method on the control tower. + routerFailAttempt = "Router:fail-attempt" + + // routerFailPayment is a test step where we expect the router + // to call the Fail method on the control tower. + routerFailPayment = "Router:fail-payment" // sendToSwitchSuccess is a step where we expect the router to // call send the payment attempt to the switch, and we will @@ -178,7 +183,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { routerRegisterAttempt, sendToSwitchSuccess, getPaymentResultSuccess, - routerSuccess, + routerSettleAttempt, paymentSuccess, }, routes: []*route.Route{rt}, @@ -193,6 +198,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Make the first sent attempt fail. getPaymentResultFailure, + routerFailAttempt, // The router should retry. routerRegisterAttempt, @@ -200,7 +206,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Make the second sent attempt succeed. getPaymentResultSuccess, - routerSuccess, + routerSettleAttempt, paymentSuccess, }, routes: []*route.Route{rt, rt}, @@ -215,6 +221,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Make the first sent attempt fail. sendToSwitchResultFailure, + routerFailAttempt, // The router should retry. routerRegisterAttempt, @@ -222,7 +229,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Make the second sent attempt succeed. getPaymentResultSuccess, - routerSuccess, + routerSettleAttempt, paymentSuccess, }, routes: []*route.Route{rt, rt}, @@ -238,10 +245,11 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Make the first sent attempt fail. getPaymentResultFailure, + routerFailAttempt, // Since there are no more routes to try, the // payment should fail. - routerFail, + routerFailPayment, paymentError, }, routes: []*route.Route{rt}, @@ -251,7 +259,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // no routes to try. steps: []string{ routerInitPayment, - routerFail, + routerFailPayment, paymentError, }, routes: []*route.Route{}, @@ -286,7 +294,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Notify about a success for the original // payment. getPaymentResultSuccess, - routerSuccess, + routerSettleAttempt, // Now that the original payment finished, // resend it again to ensure this is not @@ -316,7 +324,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // control tower. startRouter, getPaymentResultSuccess, - routerSuccess, + routerSettleAttempt, }, routes: []*route.Route{rt}, }, @@ -336,10 +344,11 @@ func TestRouterPaymentStateMachine(t *testing.T) { // Make the first attempt fail. getPaymentResultFailure, - routerFail, + routerFailAttempt, // Since we have no more routes to try, the // original payment should fail. + routerFailPayment, paymentError, // Now resend the payment again. This should be @@ -349,7 +358,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { routerRegisterAttempt, sendToSwitchSuccess, getPaymentResultSuccess, - routerSuccess, + routerSettleAttempt, resentPaymentSuccess, }, routes: []*route.Route{rt}, @@ -360,9 +369,10 @@ func TestRouterPaymentStateMachine(t *testing.T) { // synchronize and listen for events. control := makeMockControlTower() control.init = make(chan initArgs) - control.register = make(chan registerArgs) - control.success = make(chan successArgs) - control.fail = make(chan failArgs) + control.registerAttempt = make(chan registerAttemptArgs) + control.settleAttempt = make(chan settleAttemptArgs) + control.failAttempt = make(chan failAttemptArgs) + control.failPayment = make(chan failPaymentArgs) control.fetchInFlight = make(chan struct{}) quit := make(chan struct{}) @@ -497,11 +507,12 @@ func TestRouterPaymentStateMachine(t *testing.T) { // In this step we expect the router to make a call to // register a new attempt with the ControlTower. case routerRegisterAttempt: - var args registerArgs + var args registerAttemptArgs select { - case args = <-control.register: + case args = <-control.registerAttempt: case <-time.After(1 * time.Second): - t.Fatalf("not registered with control") + t.Fatalf("attempt not registered " + + "with control") } if args.a == nil { @@ -509,22 +520,35 @@ func TestRouterPaymentStateMachine(t *testing.T) { } // In this step we expect the router to call the - // ControlTower's Succcess method with the preimage. - case routerSuccess: + // ControlTower's SettleAttempt method with the preimage. + case routerSettleAttempt: select { - case <-control.success: + case <-control.settleAttempt: case <-time.After(1 * time.Second): - t.Fatalf("not registered with control") + t.Fatalf("attempt settle not " + + "registered with control") + } + + // In this step we expect the router to call the + // ControlTower's FailAttempt method with a HTLC fail + // info. + case routerFailAttempt: + select { + case <-control.failAttempt: + case <-time.After(1 * time.Second): + t.Fatalf("attempt fail not " + + "registered with control") } // In this step we expect the router to call the // ControlTower's Fail method, to indicate that the // payment failed. - case routerFail: + case routerFailPayment: select { - case <-control.fail: + case <-control.failPayment: case <-time.After(1 * time.Second): - t.Fatalf("not registered with control") + t.Fatalf("payment fail not " + + "registered with control") } // In this step we expect the SendToSwitch method to be @@ -625,7 +649,8 @@ func TestRouterPaymentStateMachine(t *testing.T) { select { case err := <-paymentResult: if err != nil { - t.Fatalf("did not expecte error %v", err) + t.Fatalf("did not expect "+ + "error %v", err) } case <-time.After(1 * time.Second): From 0fd71cd5965171a8925ebcdeb139868a69d5c3e0 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:26 +0200 Subject: [PATCH 26/34] routing/payment_lifecycle_test: add step for terminal failure And modify the MissionControl mock to return a non-nil failure reason in this case. --- routing/mock_test.go | 7 ++++++ routing/payment_lifecycle_test.go | 38 +++++++++++++++++++++++++------ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/routing/mock_test.go b/routing/mock_test.go index 594c14834..36523a973 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -101,6 +101,13 @@ func (m *mockMissionControl) ReportPaymentFail(paymentID uint64, rt *route.Route failureSourceIdx *int, failure lnwire.FailureMessage) ( *channeldb.FailureReason, error) { + // Report a permanent failure if this is an error caused + // by incorrect details. + if failure.Code() == lnwire.CodeIncorrectOrUnknownPaymentDetails { + reason := channeldb.FailureReasonPaymentDetails + return &reason, nil + } + return nil, nil } diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index 4c371b066..7e492ea75 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -139,10 +139,16 @@ func TestRouterPaymentStateMachine(t *testing.T) { // respond with a successful payment result. getPaymentResultSuccess = "GetPaymentResult:success" - // getPaymentResultFailure is a test step where we expect the + // getPaymentResultTempFailure is a test step where we expect the // router to call the GetPaymentResult method, and we will - // respond with a forwarding error. - getPaymentResultFailure = "GetPaymentResult:failure" + // respond with a forwarding error, expecting the router to retry. + getPaymentResultTempFailure = "GetPaymentResult:temp-failure" + + // getPaymentResultTerminalFailure is a test step where we + // expect the router to call the GetPaymentResult method, and + // we will respond with a terminal error, expecting the router + // to stop making payment attempts. + getPaymentResultTerminalFailure = "GetPaymentResult:terminal-failure" // resendPayment is a test step where we manually try to resend // the same payment, making sure the router responds with an @@ -197,7 +203,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { sendToSwitchSuccess, // Make the first sent attempt fail. - getPaymentResultFailure, + getPaymentResultTempFailure, routerFailAttempt, // The router should retry. @@ -244,7 +250,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { sendToSwitchSuccess, // Make the first sent attempt fail. - getPaymentResultFailure, + getPaymentResultTempFailure, routerFailAttempt, // Since there are no more routes to try, the @@ -343,7 +349,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { resentPaymentError, // Make the first attempt fail. - getPaymentResultFailure, + getPaymentResultTempFailure, routerFailAttempt, // Since we have no more routes to try, the @@ -587,7 +593,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { // In this state we expect the GetPaymentResult method // to be called, and we respond with a forwarding // error, indicating that the router should retry. - case getPaymentResultFailure: + case getPaymentResultTempFailure: failure := htlcswitch.NewForwardingError( &lnwire.FailTemporaryChannelFailure{}, 1, @@ -601,6 +607,24 @@ func TestRouterPaymentStateMachine(t *testing.T) { t.Fatalf("unable to get result") } + // In this state we expect the router to call the + // GetPaymentResult method, and we will respond with a + // terminal error, indiating the router should stop + // making payment attempts. + case getPaymentResultTerminalFailure: + failure := htlcswitch.NewForwardingError( + &lnwire.FailIncorrectDetails{}, + 1, + ) + + select { + case getPaymentResult <- &htlcswitch.PaymentResult{ + Error: failure, + }: + case <-time.After(1 * time.Second): + t.Fatalf("unable to get result") + } + // In this step we manually try to resend the same // payment, making sure the router responds with an // error indicating that it is already in flight. From 7b318a4be7808b7f40e10e38864a95c2fc2a05b5 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:26 +0200 Subject: [PATCH 27/34] routing/payment_lifecycle+channeldb: enable multi shard send This commit finally enables MP payments within the payment lifecycle (used for SendPayment). This is done by letting the loop launch shards as long as there is value remaining to send, inspecting the outcomes for the sent shards when the full payment amount has been filled. The method channeldb.MPPayment.SentAmt() is added to easily look up how much value we have sent for the payment. --- channeldb/mp_payment.go | 18 ++ routing/payment_lifecycle.go | 426 ++++++++++++++++++++++++----------- 2 files changed, 316 insertions(+), 128 deletions(-) diff --git a/channeldb/mp_payment.go b/channeldb/mp_payment.go index bf2f65ef3..e359ff03d 100644 --- a/channeldb/mp_payment.go +++ b/channeldb/mp_payment.go @@ -144,6 +144,24 @@ func (m *MPPayment) TerminalInfo() (*HTLCSettleInfo, *FailureReason) { 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 { diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 6e98eb1b0..3a0eb75b8 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -1,6 +1,8 @@ package routing import ( + "fmt" + "sync" "time" "github.com/davecgh/go-spew/spew" @@ -24,14 +26,74 @@ type paymentLifecycle struct { currentHeight int32 } +// payemntState holds a number of key insights learned from a given MPPayment +// that we use to determine what to do on each payment loop iteration. +type paymentState struct { + numShardsInFlight int + remainingAmt lnwire.MilliSatoshi + remainingFees lnwire.MilliSatoshi + terminate bool +} + +// paymentState uses the passed payment to find the latest information we need +// to act on every iteration of the payment loop. +func (p *paymentLifecycle) paymentState(payment *channeldb.MPPayment) ( + *paymentState, error) { + + // Fetch the total amount and fees that has already been sent in + // settled and still in-flight shards. + sentAmt, fees := payment.SentAmt() + + // Sanity check we haven't sent a value larger than the payment amount. + if sentAmt > p.totalAmount { + return nil, fmt.Errorf("amount sent %v exceeds "+ + "total amount %v", sentAmt, p.totalAmount) + } + + // We'll subtract the used fee from our fee budget, but allow the fees + // of the already sent shards to exceed our budget (can happen after + // restarts). + feeBudget := p.feeLimit + if fees <= feeBudget { + feeBudget -= fees + } else { + feeBudget = 0 + } + + // Get any terminal info for this payment. + settle, failure := payment.TerminalInfo() + + // If either an HTLC settled, or the payment has a payment level + // failure recorded, it means we should terminate the moment all shards + // have returned with a result. + terminate := settle != nil || failure != nil + + activeShards := payment.InFlightHTLCs() + return &paymentState{ + numShardsInFlight: len(activeShards), + remainingAmt: p.totalAmount - sentAmt, + remainingFees: feeBudget, + terminate: terminate, + }, nil +} + // resumePayment resumes the paymentLifecycle from the current state. func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { shardHandler := &shardHandler{ router: p.router, paymentHash: p.paymentHash, + shardErrors: make(chan error), + quit: make(chan struct{}), } - // If we have an existing attempt, we'll start by collecting its result. + // When the payment lifecycle loop exits, we make sure to signal any + // sub goroutine of the shardHandler to exit, then wait for them to + // return. + defer shardHandler.stop() + + // If we had any existing attempts outstanding, we'll start by spinning + // up goroutines that'll collect their results and deliver them to the + // lifecycle loop below. payment, err := p.router.cfg.Control.FetchPayment( p.paymentHash, ) @@ -42,15 +104,21 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { for _, a := range payment.InFlightHTLCs() { a := a - _, err := shardHandler.collectResult(&a.HTLCAttemptInfo) - if err != nil { - return [32]byte{}, nil, err - } + log.Debugf("Resuming payment shard %v for hash %v", + a.AttemptID, p.paymentHash) + + shardHandler.collectResultAsync(&a.HTLCAttemptInfo) } // We'll continue until either our payment succeeds, or we encounter a // critical error during path finding. for { + // Start by quickly checking if there are any outcomes already + // available to handle before we reevaluate our state. + if err := shardHandler.checkShards(); err != nil { + return [32]byte{}, nil, err + } + // We start every iteration by fetching the lastest state of // the payment from the ControlTower. This ensures that we will // act on the latest available information, whether we are @@ -62,90 +130,97 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { return [32]byte{}, nil, err } - // Go through the HTLCs for this payment, determining if there - // are any in flight or settled. - var ( - attempt *channeldb.HTLCAttemptInfo - settle *channeldb.HTLCAttempt - ) - for _, a := range payment.HTLCs { - a := a - - // We have a settled HTLC, and should return when all - // shards are back. - if a.Settle != nil { - settle = &a - continue - } - - // This HTLC already failed, ignore. - if a.Failure != nil { - continue - } - - // HTLC was neither setteld nor failed, it is still in - // flight. - attempt = &a.HTLCAttemptInfo - break + // Using this latest state of the payment, calculate + // information about our active shards and terminal conditions. + state, err := p.paymentState(payment) + if err != nil { + return [32]byte{}, nil, err } - // Terminal state, return the preimage and the route taken. - if attempt == nil && settle != nil { - return settle.Settle.Preimage, &settle.Route, nil - } + log.Debugf("Payment %v in state terminate=%v, "+ + "active_shards=%v, rem_value=%v, fee_limit=%v", + p.paymentHash, state.terminate, state.numShardsInFlight, + state.remainingAmt, state.remainingFees) - // If the payment already is failed, and there is no in-flight - // HTLC, return immediately. - if attempt == nil && payment.FailureReason != nil { - return [32]byte{}, nil, *payment.FailureReason - } + switch { - // If this payment had no existing payment attempt, we create - // and send one now. - if attempt == nil { - // Before we attempt this next payment, we'll check to see if either - // we've gone past the payment attempt timeout, or the router is - // exiting. In either case, we'll stop this payment attempt short. If a - // timeout is not applicable, timeoutChan will be nil. - select { - case <-p.timeoutChan: - // Mark the payment as failed because of the - // timeout. - err := p.router.cfg.Control.Fail( - p.paymentHash, channeldb.FailureReasonTimeout, - ) - if err != nil { - return [32]byte{}, nil, err + // We have a terminal condition and no active shards, we are + // ready to exit. + case state.terminate && state.numShardsInFlight == 0: + // Find the first successful shard and return + // the preimage and route. + for _, a := range payment.HTLCs { + if a.Settle != nil { + return a.Settle.Preimage, &a.Route, nil } - - continue - - // The payment will be resumed from the current state - // after restart. - case <-p.router.quit: - return [32]byte{}, nil, ErrRouterShuttingDown - - // Fall through if we haven't hit our time limit or are - // exiting. - default: } - // Create a new payment attempt from the given payment session. - rt, err := p.paySession.RequestRoute( - p.totalAmount, p.feeLimit, 0, uint32(p.currentHeight), + // Payment failed. + return [32]byte{}, nil, *payment.FailureReason + + // If we either reached a terminal error condition (but had + // active shards still) or there is no remaining value to send, + // we'll wait for a shard outcome. + case state.terminate || state.remainingAmt == 0: + // We still have outstanding shards, so wait for a new + // outcome to be available before re-evaluating our + // state. + if err := shardHandler.waitForShard(); err != nil { + return [32]byte{}, nil, err + } + continue + } + + // Before we attempt any new shard, we'll check to see if + // either we've gone past the payment attempt timeout, or the + // router is exiting. In either case, we'll stop this payment + // attempt short. If a timeout is not applicable, timeoutChan + // will be nil. + select { + case <-p.timeoutChan: + log.Warnf("payment attempt not completed before " + + "timeout") + + // By marking the payment failed with the control + // tower, no further shards will be launched and we'll + // return with an error the moment all active shards + // have finished. + saveErr := p.router.cfg.Control.Fail( + p.paymentHash, channeldb.FailureReasonTimeout, ) - if err != nil { - log.Warnf("Failed to find route for payment %x: %v", - p.paymentHash, err) + if saveErr != nil { + return [32]byte{}, nil, saveErr + } - // Convert error to payment-level failure. - failure := errorToPaymentFailure(err) + continue + + case <-p.router.quit: + return [32]byte{}, nil, ErrRouterShuttingDown + + // Fall through if we haven't hit our time limit. + default: + } + + // Create a new payment attempt from the given payment session. + rt, err := p.paySession.RequestRoute( + state.remainingAmt, state.remainingFees, + uint32(state.numShardsInFlight), uint32(p.currentHeight), + ) + if err != nil { + log.Warnf("Failed to find route for payment %x: %v", + p.paymentHash, err) + + // There is no route to try, and we have no active + // shards. This means that there is no way for us to + // send the payment, so mark it failed with no route. + if state.numShardsInFlight == 0 { + failureCode := errorToPaymentFailure(err) + log.Debugf("Marking payment %v permanently "+ + "failed with no route: %v", + p.paymentHash, failureCode) - // If we're unable to successfully make a payment using - // any of the routes we've found, then mark the payment - // as permanently failed. saveErr := p.router.cfg.Control.Fail( - p.paymentHash, failure, + p.paymentHash, failureCode, ) if saveErr != nil { return [32]byte{}, nil, saveErr @@ -154,56 +229,45 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { continue } - // With the route in hand, launch a new shard. - var outcome *launchOutcome - attempt, outcome, err = shardHandler.launchShard(rt) - if err != nil { + // We still have active shards, we'll wait for an + // outcome to be available before retrying. + if err := shardHandler.waitForShard(); err != nil { return [32]byte{}, nil, err } - - // We ew encountered a non-critical error when - // launching the shard, handle it - if outcome.err != nil { - // We must inspect the error to know whether it - // was critical or not, to decide whether we - // should continue trying. - err = shardHandler.handleSendError( - attempt, outcome.err, - ) - if err != nil { - return [32]byte{}, nil, err - } - - // Error was handled successfully, continue to - // make a new attempt. - continue - } - - // We'll collect the result of the shard just sent. We - // ignore the result for now if it is a success, as we - // will look it up in the control tower on the next - // loop iteration. - result, err := shardHandler.collectResult(attempt) - if err != nil { - return [32]byte{}, nil, err - } - - if result.err != nil { - // We must inspect the error to know whether it - // was critical or not, to decide whether we - // should continue trying. - err := shardHandler.handleSendError( - attempt, result.err, - ) - if err != nil { - return [32]byte{}, nil, err - } - - // Error was handled successfully, continue to - // make a new attempt. - continue - } + continue } + + // We found a route to try, launch a new shard. + attempt, outcome, err := shardHandler.launchShard(rt) + if err != nil { + return [32]byte{}, nil, err + } + + // If we encountered a non-critical error when launching the + // shard, handle it. + if outcome.err != nil { + log.Warnf("Failed to launch shard %v for "+ + "payment %v: %v", attempt.AttemptID, + p.paymentHash, outcome.err) + + // We must inspect the error to know whether it was + // critical or not, to decide whether we should + // continue trying. + err := shardHandler.handleSendError( + attempt, outcome.err, + ) + if err != nil { + return [32]byte{}, nil, err + } + + // Error was handled successfully, continue to make a + // new attempt. + continue + } + + // Now that the shard was successfully sent, launch a go + // routine that will handle its result when its back. + shardHandler.collectResultAsync(attempt) } } @@ -212,6 +276,60 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { type shardHandler struct { paymentHash lntypes.Hash router *ChannelRouter + + // shardErrors is a channel where errors collected by calling + // collectResultAsync will be delivered. These results are meant to be + // inspected by calling waitForShard or checkShards, and the channel + // doesn't need to be initiated if the caller is using the sync + // collectResult directly. + shardErrors chan error + + // quit is closed to signal the sub goroutines of the payment lifecycle + // to stop. + quit chan struct{} + wg sync.WaitGroup +} + +// stop signals any active shard goroutine to exit and waits for them to exit. +func (p *shardHandler) stop() { + close(p.quit) + p.wg.Wait() +} + +// waitForShard blocks until any of the outstanding shards return. +func (p *shardHandler) waitForShard() error { + select { + case err := <-p.shardErrors: + return err + + case <-p.quit: + return fmt.Errorf("shard handler quitting") + + case <-p.router.quit: + return ErrRouterShuttingDown + } +} + +// checkShards is a non-blocking method that check if any shards has finished +// their execution. +func (p *shardHandler) checkShards() error { + for { + select { + case err := <-p.shardErrors: + if err != nil { + return err + } + + case <-p.quit: + return fmt.Errorf("shard handler quitting") + + case <-p.router.quit: + return ErrRouterShuttingDown + + default: + return nil + } + } } // launchOutcome is a type returned from launchShard that indicates whether the @@ -283,6 +401,55 @@ type shardResult struct { err error } +// collectResultAsync launches a goroutine that will wait for the result of the +// given HTLC attempt to be available then handle its result. Note that it will +// fail the payment with the control tower if a terminal error is encountered. +func (p *shardHandler) collectResultAsync(attempt *channeldb.HTLCAttemptInfo) { + p.wg.Add(1) + go func() { + defer p.wg.Done() + + // Block until the result is available. + result, err := p.collectResult(attempt) + if err != nil { + if err != ErrRouterShuttingDown && + err != htlcswitch.ErrSwitchExiting { + + log.Errorf("Error collecting result for "+ + "shard %v for payment %v: %v", + attempt.AttemptID, p.paymentHash, err) + } + + select { + case p.shardErrors <- err: + case <-p.router.quit: + case <-p.quit: + } + return + } + + // If a non-critical error was encountered handle it and mark + // the payment failed if the failure was terminal. + if result.err != nil { + err := p.handleSendError(attempt, result.err) + if err != nil { + select { + case p.shardErrors <- err: + case <-p.router.quit: + case <-p.quit: + } + return + } + } + + select { + case p.shardErrors <- nil: + case <-p.router.quit: + case <-p.quit: + } + }() +} + // collectResult waits for the result for the given attempt to be available // from the Switch, then records the attempt outcome with the control tower. A // shardResult is returned, indicating the final outcome of this HTLC attempt. @@ -353,14 +520,14 @@ func (p *shardHandler) collectResult(attempt *channeldb.HTLCAttemptInfo) ( case <-p.router.quit: return nil, ErrRouterShuttingDown + + case <-p.quit: + return nil, fmt.Errorf("shard handler exiting") } // In case of a payment failure, fail the attempt with the control // tower and return. if result.Error != nil { - log.Errorf("Attempt to send payment %x failed: %v", - p.paymentHash, result.Error) - err := p.failAttempt(attempt, result.Error) if err != nil { return nil, err @@ -540,6 +707,9 @@ func (p *shardHandler) handleSendError(attempt *channeldb.HTLCAttemptInfo, func (p *shardHandler) failAttempt(attempt *channeldb.HTLCAttemptInfo, sendError error) error { + log.Warnf("Attempt %v for payment %v failed: %v", attempt.AttemptID, + p.paymentHash, sendError) + failInfo := marshallError( sendError, p.router.cfg.Clock.Now(), From 431372c0cfc18242a9a8470f7673a797ef29a2fc Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:26 +0200 Subject: [PATCH 28/34] routing/payment_lifecycle_test: add MPP test cases --- routing/payment_lifecycle_test.go | 225 +++++++++++++++++++++++++++--- 1 file changed, 204 insertions(+), 21 deletions(-) diff --git a/routing/payment_lifecycle_test.go b/routing/payment_lifecycle_test.go index 7e492ea75..e83ac17f4 100644 --- a/routing/payment_lifecycle_test.go +++ b/routing/payment_lifecycle_test.go @@ -17,6 +17,8 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) +const stepTimeout = 5 * time.Second + // createTestRoute builds a route a->b->c paying the given amt to c. func createTestRoute(amt lnwire.MilliSatoshi, aliasMap map[string]route.Vertex) (*route.Route, error) { @@ -88,6 +90,11 @@ func TestRouterPaymentStateMachine(t *testing.T) { t.Fatalf("unable to create route: %v", err) } + shard, err := createTestRoute(paymentAmt/4, testGraph.aliasMap) + if err != nil { + t.Fatalf("unable to create route: %v", err) + } + // A payment state machine test case consists of several ordered steps, // that we use for driving the scenario. type testCase struct { @@ -369,17 +376,193 @@ func TestRouterPaymentStateMachine(t *testing.T) { }, routes: []*route.Route{rt}, }, + + // ===================================== + // || MPP scenarios || + // ===================================== + { + // Tests a simple successful MP payment of 4 shards. + steps: []string{ + routerInitPayment, + + // shard 0 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 1 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 2 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 3 + routerRegisterAttempt, + sendToSwitchSuccess, + + // All shards succeed. + getPaymentResultSuccess, + getPaymentResultSuccess, + getPaymentResultSuccess, + getPaymentResultSuccess, + + // Router should settle them all. + routerSettleAttempt, + routerSettleAttempt, + routerSettleAttempt, + routerSettleAttempt, + + // And the final result is obviously + // successful. + paymentSuccess, + }, + routes: []*route.Route{shard, shard, shard, shard}, + }, + { + // An MP payment scenario where we need several extra + // attempts before the payment finally settle. + steps: []string{ + routerInitPayment, + + // shard 0 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 1 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 2 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 3 + routerRegisterAttempt, + sendToSwitchSuccess, + + // First two shards fail, two new ones are sent. + getPaymentResultTempFailure, + getPaymentResultTempFailure, + routerFailAttempt, + routerFailAttempt, + + routerRegisterAttempt, + sendToSwitchSuccess, + routerRegisterAttempt, + sendToSwitchSuccess, + + // The four shards settle. + getPaymentResultSuccess, + getPaymentResultSuccess, + getPaymentResultSuccess, + getPaymentResultSuccess, + routerSettleAttempt, + routerSettleAttempt, + routerSettleAttempt, + routerSettleAttempt, + + // Overall payment succeeds. + paymentSuccess, + }, + routes: []*route.Route{ + shard, shard, shard, shard, shard, shard, + }, + }, + { + // An MP payment scenario where 3 of the shards fail. + // However the last shard settle, which means we get + // the preimage and should consider the overall payment + // a success. + steps: []string{ + routerInitPayment, + + // shard 0 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 1 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 2 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 3 + routerRegisterAttempt, + sendToSwitchSuccess, + + // 3 shards fail, and should be failed by the + // router. + getPaymentResultTempFailure, + getPaymentResultTempFailure, + getPaymentResultTempFailure, + routerFailAttempt, + routerFailAttempt, + routerFailAttempt, + + // The fourth shard succeed against all odds, + // making the overall payment succeed. + getPaymentResultSuccess, + routerSettleAttempt, + paymentSuccess, + }, + routes: []*route.Route{shard, shard, shard, shard}, + }, + { + // An MP payment scenario a shard fail with a terminal + // error, causing the router to stop attempting. + steps: []string{ + routerInitPayment, + + // shard 0 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 1 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 2 + routerRegisterAttempt, + sendToSwitchSuccess, + + // shard 3 + routerRegisterAttempt, + sendToSwitchSuccess, + + // The first shard fail with a terminal error. + getPaymentResultTerminalFailure, + routerFailAttempt, + routerFailPayment, + + // Remaining 3 shards fail. + getPaymentResultTempFailure, + getPaymentResultTempFailure, + getPaymentResultTempFailure, + routerFailAttempt, + routerFailAttempt, + routerFailAttempt, + + // Payment fails. + paymentError, + }, + routes: []*route.Route{ + shard, shard, shard, shard, shard, shard, + }, + }, } // Create a mock control tower with channels set up, that we use to // synchronize and listen for events. control := makeMockControlTower() - control.init = make(chan initArgs) - control.registerAttempt = make(chan registerAttemptArgs) - control.settleAttempt = make(chan settleAttemptArgs) - control.failAttempt = make(chan failAttemptArgs) - control.failPayment = make(chan failPaymentArgs) - control.fetchInFlight = make(chan struct{}) + control.init = make(chan initArgs, 20) + control.registerAttempt = make(chan registerAttemptArgs, 20) + control.settleAttempt = make(chan settleAttemptArgs, 20) + control.failAttempt = make(chan failAttemptArgs, 20) + control.failPayment = make(chan failPaymentArgs, 20) + control.fetchInFlight = make(chan struct{}, 20) quit := make(chan struct{}) defer close(quit) @@ -502,7 +685,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { var args initArgs select { case args = <-control.init: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("no init payment with control") } @@ -516,7 +699,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { var args registerAttemptArgs select { case args = <-control.registerAttempt: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("attempt not registered " + "with control") } @@ -530,7 +713,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case routerSettleAttempt: select { case <-control.settleAttempt: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("attempt settle not " + "registered with control") } @@ -541,7 +724,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case routerFailAttempt: select { case <-control.failAttempt: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("attempt fail not " + "registered with control") } @@ -552,7 +735,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case routerFailPayment: select { case <-control.failPayment: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("payment fail not " + "registered with control") } @@ -562,7 +745,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case sendToSwitchSuccess: select { case sendResult <- nil: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("unable to send result") } @@ -574,7 +757,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { &lnwire.FailTemporaryChannelFailure{}, 1, ): - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("unable to send result") } @@ -586,7 +769,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case getPaymentResult <- &htlcswitch.PaymentResult{ Preimage: preImage, }: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("unable to send result") } @@ -603,7 +786,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case getPaymentResult <- &htlcswitch.PaymentResult{ Error: failure, }: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("unable to get result") } @@ -621,7 +804,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { case getPaymentResult <- &htlcswitch.PaymentResult{ Error: failure, }: - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("unable to get result") } @@ -640,7 +823,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { select { case getPaymentResultErr <- fmt.Errorf( "shutting down"): - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("unable to send payment " + "result error") } @@ -663,7 +846,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { t.Fatalf("expected error") } - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("got no payment result") } @@ -677,7 +860,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { "error %v", err) } - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("got no payment result") } @@ -690,7 +873,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { t.Fatalf("expected error") } - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("got no payment result") } @@ -703,7 +886,7 @@ func TestRouterPaymentStateMachine(t *testing.T) { t.Fatalf("did not expect error %v", err) } - case <-time.After(1 * time.Second): + case <-time.After(stepTimeout): t.Fatalf("got no payment result") } From 36a80b4d51e2a94ebe5ba68abb14a0a4e4ab8e35 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:27 +0200 Subject: [PATCH 29/34] routing/router: enable MPP sends for SendToRoute This commit enables MPP sends for SendToRoute, by allowing launching another payment attempt if the hash is already registered with the ControlTower. We also set the total payment amount of of the payment from mpp record, to indicate that the shard value might be different from the total payment value. We only mark non-MPP payments as failed in the database after encountering a failure, since we might want to try more shards for MPP. For now this means that MPP sendToRoute payments will be failed only after a restart has happened. --- routing/router.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/routing/router.go b/routing/router.go index a5cf84881..cac3e69d4 100644 --- a/routing/router.go +++ b/routing/router.go @@ -1732,6 +1732,14 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, rt *route.Route) ( // Calculate amount paid to receiver. amt := rt.ReceiverAmt() + // If this is meant as a MP payment shard, we set the amount + // for the creating info to the total amount of the payment. + finalHop := rt.Hops[len(rt.Hops)-1] + mpp := finalHop.MPP + if mpp != nil { + amt = mpp.TotalMsat() + } + // Record this payment hash with the ControlTower, ensuring it is not // already in-flight. info := &channeldb.PaymentCreationInfo{ @@ -1742,7 +1750,13 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, rt *route.Route) ( } err := r.cfg.Control.InitPayment(hash, info) - if err != nil { + switch { + // If this is an MPP attempt and the hash is already registered with + // the database, we can go on to launch the shard. + case err == channeldb.ErrPaymentInFlight && mpp != nil: + + // Any other error is not tolerated. + case err != nil: return [32]byte{}, err } From 9a1ec950bd9198a6e334d553a096914882670b5f Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:27 +0200 Subject: [PATCH 30/34] channeldb/payments: extract common info fetch into fetchCreationInfo --- channeldb/payment_control.go | 9 +-------- channeldb/payments.go | 20 ++++++++++++-------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/channeldb/payment_control.go b/channeldb/payment_control.go index 81b4d9440..d0bbae755 100644 --- a/channeldb/payment_control.go +++ b/channeldb/payment_control.go @@ -548,14 +548,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 } diff --git a/channeldb/payments.go b/channeldb/payments.go index c07005ab4..6c5730435 100644 --- a/channeldb/payments.go +++ b/channeldb/payments.go @@ -252,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 { @@ -261,13 +271,7 @@ func fetchPayment(bucket kvdb.ReadBucket) (*MPPayment, error) { sequenceNum := binary.BigEndian.Uint64(seqBytes) // 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 @@ -285,7 +289,7 @@ 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 From 864e64e725b520fa22c771b755904b82bd43060c Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:27 +0200 Subject: [PATCH 31/34] channeldb: validate MPP options when registering attempts We add validation making sure we are not trying to register MPP shards for non-MPP payments, and vice versa. We also add validtion of total sent amount against payment value, and matching MPP options. We also add methods for copying Route/Hop, since it is useful to use for modifying the route amount in the test. --- channeldb/payment_control.go | 74 ++++++++++++++++++ channeldb/payment_control_test.go | 122 +++++++++++++++++++++++++++++- routing/control_tower_test.go | 2 +- routing/route/route.go | 38 ++++++++++ 4 files changed, 231 insertions(+), 5 deletions(-) diff --git a/channeldb/payment_control.go b/channeldb/payment_control.go index d0bbae755..ca5b6998b 100644 --- a/channeldb/payment_control.go +++ b/channeldb/payment_control.go @@ -45,6 +45,32 @@ var ( // 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") @@ -189,11 +215,59 @@ func (p *PaymentControl) RegisterAttempt(paymentHash lntypes.Hash, 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, ) diff --git a/channeldb/payment_control_test.go b/channeldb/payment_control_test.go index 015aa2318..abd2722a5 100644 --- a/channeldb/payment_control_test.go +++ b/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: 0, SessionKey: priv, - Route: testRoute, + Route: *testRoute.Copy(), }, preimage, nil } @@ -504,7 +505,15 @@ func TestPaymentControlMultiShard(t *testing.T) { ) // Create three unique attempts we'll use for the test, and - // register them with the payment control. + // 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 @@ -527,6 +536,17 @@ func TestPaymentControlMultiShard(t *testing.T) { ) } + // 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 @@ -612,7 +632,7 @@ func TestPaymentControlMultiShard(t *testing.T) { // Try to register yet another attempt. This should fail now // that the payment has reached a terminal condition. - b := *attempt + b = *attempt b.AttemptID = 3 err = pControl.RegisterAttempt(info.PaymentHash, &b) if err != ErrPaymentTerminal { @@ -705,6 +725,100 @@ func TestPaymentControlMultiShard(t *testing.T) { } } +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, diff --git a/routing/control_tower_test.go b/routing/control_tower_test.go index 6bc8ffd7d..82dc2706f 100644 --- a/routing/control_tower_test.go +++ b/routing/control_tower_test.go @@ -324,7 +324,7 @@ func genInfo() (*channeldb.PaymentCreationInfo, *channeldb.HTLCAttemptInfo, rhash := sha256.Sum256(preimage[:]) return &channeldb.PaymentCreationInfo{ PaymentHash: rhash, - Value: 1, + Value: testRoute.ReceiverAmt(), CreationTime: time.Unix(time.Now().Unix(), 0), PaymentRequest: []byte("hola"), }, diff --git a/routing/route/route.go b/routing/route/route.go index 31fa3bf50..63944af18 100644 --- a/routing/route/route.go +++ b/routing/route/route.go @@ -129,6 +129,23 @@ type Hop struct { LegacyPayload bool } +// Copy returns a deep copy of the Hop. +func (h *Hop) Copy() *Hop { + c := *h + + if h.MPP != nil { + m := *h.MPP + c.MPP = &m + } + + if h.AMP != nil { + a := *h.AMP + c.AMP = &a + } + + return &c +} + // PackHopPayload writes to the passed io.Writer, the series of byes that can // be placed directly into the per-hop payload (EOB) for this hop. This will // include the required routing fields, as well as serializing any of the @@ -287,6 +304,18 @@ type Route struct { Hops []*Hop } +// Copy returns a deep copy of the Route. +func (r *Route) Copy() *Route { + c := *r + + c.Hops = make([]*Hop, len(r.Hops)) + for i := range r.Hops { + c.Hops[i] = r.Hops[i].Copy() + } + + return &c +} + // HopFee returns the fee charged by the route hop indicated by hopIndex. func (r *Route) HopFee(hopIndex int) lnwire.MilliSatoshi { var incomingAmt lnwire.MilliSatoshi @@ -320,6 +349,15 @@ func (r *Route) ReceiverAmt() lnwire.MilliSatoshi { return r.Hops[len(r.Hops)-1].AmtToForward } +// FinalHop returns the last hop of the route, or nil if the route is empty. +func (r *Route) FinalHop() *Hop { + if len(r.Hops) == 0 { + return nil + } + + return r.Hops[len(r.Hops)-1] +} + // NewRouteFromHops creates a new Route structure from the minimally required // information to perform the payment. It infers fee amounts and populates the // node, chan and prev/next hop maps. From 95c5a123c8661918723566740e50de573b9d0942 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:27 +0200 Subject: [PATCH 32/34] routing/router_test: add TestSendToRouteMultiShardSend --- routing/mock_test.go | 16 +++-- routing/router_test.go | 133 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 4 deletions(-) diff --git a/routing/mock_test.go b/routing/mock_test.go index 36523a973..f47a94204 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -15,6 +15,8 @@ import ( type mockPaymentAttemptDispatcher struct { onPayment func(firstHop lnwire.ShortChannelID) ([32]byte, error) results map[uint64]*htlcswitch.PaymentResult + + sync.Mutex } var _ PaymentAttemptDispatcher = (*mockPaymentAttemptDispatcher)(nil) @@ -27,10 +29,6 @@ func (m *mockPaymentAttemptDispatcher) SendHTLC(firstHop lnwire.ShortChannelID, return nil } - if m.results == nil { - m.results = make(map[uint64]*htlcswitch.PaymentResult) - } - var result *htlcswitch.PaymentResult preimage, err := m.onPayment(firstHop) if err != nil { @@ -45,7 +43,13 @@ func (m *mockPaymentAttemptDispatcher) SendHTLC(firstHop lnwire.ShortChannelID, result = &htlcswitch.PaymentResult{Preimage: preimage} } + m.Lock() + if m.results == nil { + m.results = make(map[uint64]*htlcswitch.PaymentResult) + } + m.results[pid] = result + m.Unlock() return nil } @@ -55,7 +59,11 @@ func (m *mockPaymentAttemptDispatcher) GetPaymentResult(paymentID uint64, <-chan *htlcswitch.PaymentResult, error) { c := make(chan *htlcswitch.PaymentResult, 1) + + m.Lock() res, ok := m.results[paymentID] + m.Unlock() + if !ok { return nil, htlcswitch.ErrPaymentIDNotFound } diff --git a/routing/router_test.go b/routing/router_test.go index 0bb90d715..7ac7527c5 100644 --- a/routing/router_test.go +++ b/routing/router_test.go @@ -21,6 +21,7 @@ import ( "github.com/lightningnetwork/lnd/htlcswitch" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/record" "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/zpay32" ) @@ -2725,6 +2726,138 @@ func TestSendToRouteStructuredError(t *testing.T) { } } +// TestSendToRouteMultiShardSend checks that a 3-shard payment can be executed +// using SendToRoute. +func TestSendToRouteMultiShardSend(t *testing.T) { + t.Parallel() + + ctx, cleanup, err := createTestCtxSingleNode(0) + if err != nil { + t.Fatal(err) + } + defer cleanup() + + const numShards = 3 + const payAmt = lnwire.MilliSatoshi(numShards * 10000) + node, err := createTestNode() + if err != nil { + t.Fatal(err) + } + + // Create a simple 1-hop route that we will use for all three shards. + hops := []*route.Hop{ + { + ChannelID: 1, + PubKeyBytes: node.PubKeyBytes, + AmtToForward: payAmt / numShards, + MPP: record.NewMPP(payAmt, [32]byte{}), + }, + } + + sourceNode, err := ctx.graph.SourceNode() + if err != nil { + t.Fatal(err) + } + + rt, err := route.NewRouteFromHops( + payAmt, 100, sourceNode.PubKeyBytes, hops, + ) + if err != nil { + t.Fatalf("unable to create route: %v", err) + } + + // The first shard we send we'll fail immediately, to check that we are + // still allowed to retry with other shards after a failed one. + ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcher).setPaymentResult( + func(firstHop lnwire.ShortChannelID) ([32]byte, error) { + return [32]byte{}, htlcswitch.NewForwardingError( + &lnwire.FailFeeInsufficient{ + Update: lnwire.ChannelUpdate{}, + }, 1, + ) + }) + + // The payment parameter is mostly redundant in SendToRoute. Can be left + // empty for this test. + var payment lntypes.Hash + + // Send the shard using the created route, and expect an error to be + // returned. + _, err = ctx.router.SendToRoute(payment, rt) + if err == nil { + t.Fatalf("expected forwarding error") + } + + // Now we'll modify the SendToSwitch method again to wait until all + // three shards are initiated before returning a result. We do this by + // signalling when the method has been called, and then stop to wait + // for the test to deliver the final result on the channel below. + waitForResultSignal := make(chan struct{}, numShards) + results := make(chan lntypes.Preimage, numShards) + + ctx.router.cfg.Payer.(*mockPaymentAttemptDispatcher).setPaymentResult( + func(firstHop lnwire.ShortChannelID) ([32]byte, error) { + + // Signal that the shard has been initiated and is + // waiting for a result. + waitForResultSignal <- struct{}{} + + // Wait for a result before returning it. + res, ok := <-results + if !ok { + return [32]byte{}, fmt.Errorf("failure") + } + return res, nil + }) + + // Launch three shards by calling SendToRoute in three goroutines, + // returning their final error on the channel. + errChan := make(chan error) + successes := make(chan lntypes.Preimage) + + for i := 0; i < numShards; i++ { + go func() { + preimg, err := ctx.router.SendToRoute(payment, rt) + if err != nil { + errChan <- err + return + } + + successes <- preimg + }() + } + + // Wait for all shards to signal they have been initiated. + for i := 0; i < numShards; i++ { + select { + case <-waitForResultSignal: + case <-time.After(5 * time.Second): + t.Fatalf("not waiting for results") + } + } + + // Deliver a dummy preimage to all the shard handlers. + preimage := lntypes.Preimage{} + preimage[4] = 42 + for i := 0; i < numShards; i++ { + results <- preimage + } + + // Finally expect all shards to return with the above preimage. + for i := 0; i < numShards; i++ { + select { + case p := <-successes: + if p != preimage { + t.Fatalf("preimage mismatch") + } + case err := <-errChan: + t.Fatalf("unexpected error from SendToRoute: %v", err) + case <-time.After(5 * time.Second): + t.Fatalf("result not received") + } + } +} + // TestSendToRouteMaxHops asserts that SendToRoute fails when using a route that // exceeds the maximum number of hops. func TestSendToRouteMaxHops(t *testing.T) { From fee5fd0093b578e2a515969d49a99ddd7dd3ddf1 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:27 +0200 Subject: [PATCH 33/34] itest: add testSendToRouteMultiPath testSendToRouteMultiPath tests that we are able to successfully route a payment using multiple shards across different paths, by using SendToRoute. Co-authored-by: Joost Jager --- lntest/itest/lnd_mpp_test.go | 357 +++++++++++++++++++++++++++++++++++ lntest/itest/lnd_test.go | 4 + 2 files changed, 361 insertions(+) create mode 100644 lntest/itest/lnd_mpp_test.go diff --git a/lntest/itest/lnd_mpp_test.go b/lntest/itest/lnd_mpp_test.go new file mode 100644 index 000000000..9b338baf0 --- /dev/null +++ b/lntest/itest/lnd_mpp_test.go @@ -0,0 +1,357 @@ +// +build rpctest + +package itest + +import ( + "bytes" + "context" + "fmt" + "time" + + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" + "github.com/lightningnetwork/lnd" + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntest" + "github.com/lightningnetwork/lnd/routing/route" +) + +// testSendToRouteMultiPath tests that we are able to successfully route a +// payment using multiple shards across different paths, by using SendToRoute. +func testSendToRouteMultiPath(net *lntest.NetworkHarness, t *harnessTest) { + ctxb := context.Background() + + // To ensure the payment goes through seperate paths, we'll set a + // channel size that can only carry one shard at a time. We'll divide + // the payment into 3 shards. + const ( + paymentAmt = btcutil.Amount(300000) + shardAmt = paymentAmt / 3 + chanAmt = shardAmt * 3 / 2 + ) + + // Set up a network with three different paths Alice <-> Bob. + // _ Eve _ + // / \ + // Alice -- Carol ---- Bob + // \ / + // \__ Dave ____/ + // + // + // Create the three nodes in addition to Alice and Bob. + alice := net.Alice + bob := net.Bob + carol, err := net.NewNode("carol", nil) + if err != nil { + t.Fatalf("unable to create carol: %v", err) + } + defer shutdownAndAssert(net, t, carol) + + dave, err := net.NewNode("dave", nil) + if err != nil { + t.Fatalf("unable to create dave: %v", err) + } + defer shutdownAndAssert(net, t, dave) + + eve, err := net.NewNode("eve", nil) + if err != nil { + t.Fatalf("unable to create eve: %v", err) + } + defer shutdownAndAssert(net, t, eve) + + nodes := []*lntest.HarnessNode{alice, bob, carol, dave, eve} + + // Connect nodes to ensure propagation of channels. + for i := 0; i < len(nodes); i++ { + for j := i + 1; j < len(nodes); j++ { + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + if err := net.EnsureConnected(ctxt, nodes[i], nodes[j]); err != nil { + t.Fatalf("unable to connect nodes: %v", err) + } + } + } + + // We'll send shards along three routes from Alice. + sendRoutes := [][]*lntest.HarnessNode{ + {carol, bob}, + {dave, bob}, + {carol, eve, bob}, + } + + // Keep a list of all our active channels. + var networkChans []*lnrpc.ChannelPoint + var closeChannelFuncs []func() + + // openChannel is a helper to open a channel from->to. + openChannel := func(from, to *lntest.HarnessNode, chanSize btcutil.Amount) { + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + err := net.SendCoins(ctxt, btcutil.SatoshiPerBitcoin, from) + if err != nil { + t.Fatalf("unable to send coins : %v", err) + } + + ctxt, _ = context.WithTimeout(ctxb, channelOpenTimeout) + chanPoint := openChannelAndAssert( + ctxt, t, net, from, to, + lntest.OpenChannelParams{ + Amt: chanSize, + }, + ) + + closeChannelFuncs = append(closeChannelFuncs, func() { + ctxt, _ := context.WithTimeout(ctxb, channelCloseTimeout) + closeChannelAndAssert( + ctxt, t, net, from, chanPoint, false, + ) + }) + + networkChans = append(networkChans, chanPoint) + } + + // Open channels between the nodes. + openChannel(carol, bob, chanAmt) + openChannel(dave, bob, chanAmt) + openChannel(alice, dave, chanAmt) + openChannel(eve, bob, chanAmt) + openChannel(carol, eve, chanAmt) + + // Since the channel Alice-> Carol will have to carry two + // shards, we make it larger. + openChannel(alice, carol, chanAmt+shardAmt) + + for _, f := range closeChannelFuncs { + defer f() + } + + // Wait for all nodes to have seen all channels. + for _, chanPoint := range networkChans { + for _, node := range nodes { + txid, err := lnd.GetChanPointFundingTxid(chanPoint) + if err != nil { + t.Fatalf("unable to get txid: %v", err) + } + point := wire.OutPoint{ + Hash: *txid, + Index: chanPoint.OutputIndex, + } + + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + err = node.WaitForNetworkChannelOpen(ctxt, chanPoint) + if err != nil { + t.Fatalf("(%d): timeout waiting for "+ + "channel(%s) open: %v", + node.NodeID, point, err) + } + } + } + + // Make Bob create an invoice for Alice to pay. + payReqs, rHashes, invoices, err := createPayReqs( + net.Bob, paymentAmt, 1, + ) + if err != nil { + t.Fatalf("unable to create pay reqs: %v", err) + } + + rHash := rHashes[0] + payReq := payReqs[0] + + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + decodeResp, err := net.Bob.DecodePayReq( + ctxt, &lnrpc.PayReqString{PayReq: payReq}, + ) + if err != nil { + t.Fatalf("decode pay req: %v", err) + } + + payAddr := decodeResp.PaymentAddr + + // Helper function for Alice to build a route from pubkeys. + buildRoute := func(amt btcutil.Amount, hops []*lntest.HarnessNode) ( + *lnrpc.Route, error) { + + rpcHops := make([][]byte, 0, len(hops)) + for _, hop := range hops { + k := hop.PubKeyStr + pubkey, err := route.NewVertexFromStr(k) + if err != nil { + return nil, fmt.Errorf("error parsing %v: %v", + k, err) + } + rpcHops = append(rpcHops, pubkey[:]) + } + + req := &routerrpc.BuildRouteRequest{ + AmtMsat: int64(amt * 1000), + FinalCltvDelta: lnd.DefaultBitcoinTimeLockDelta, + HopPubkeys: rpcHops, + } + + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + routeResp, err := net.Alice.RouterClient.BuildRoute(ctxt, req) + if err != nil { + return nil, err + } + + return routeResp.Route, nil + } + + responses := make(chan *routerrpc.SendToRouteResponse, len(sendRoutes)) + for _, hops := range sendRoutes { + // Build a route for the specified hops. + r, err := buildRoute(shardAmt, hops) + if err != nil { + t.Fatalf("unable to build route: %v", err) + } + + // Set the MPP records to indicate this is a payment shard. + hop := r.Hops[len(r.Hops)-1] + hop.TlvPayload = true + hop.MppRecord = &lnrpc.MPPRecord{ + PaymentAddr: payAddr, + TotalAmtMsat: int64(paymentAmt * 1000), + } + + // Send the shard. + sendReq := &routerrpc.SendToRouteRequest{ + PaymentHash: rHash, + Route: r, + } + + // We'll send all shards in their own goroutine, since SendToRoute will + // block as long as the payment is in flight. + go func() { + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + resp, err := net.Alice.RouterClient.SendToRoute(ctxt, sendReq) + if err != nil { + t.Fatalf("unable to send payment: %v", err) + } + + responses <- resp + }() + } + + // Wait for all responses to be back, and check that they all + // succeeded. + for range sendRoutes { + var resp *routerrpc.SendToRouteResponse + select { + case resp = <-responses: + case <-time.After(defaultTimeout): + t.Fatalf("response not received") + } + + if resp.Failure != nil { + t.Fatalf("received payment failure : %v", resp.Failure) + } + + // All shards should come back with the preimage. + if !bytes.Equal(resp.Preimage, invoices[0].RPreimage) { + t.Fatalf("preimage doesn't match") + } + } + + // assertNumHtlcs is a helper that checks the node's latest payment, + // and asserts it was split into num shards. + assertNumHtlcs := func(node *lntest.HarnessNode, num int) { + req := &lnrpc.ListPaymentsRequest{ + IncludeIncomplete: true, + } + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + paymentsResp, err := node.ListPayments(ctxt, req) + if err != nil { + t.Fatalf("error when obtaining payments: %v", + err) + } + + payments := paymentsResp.Payments + if len(payments) == 0 { + t.Fatalf("no payments found") + } + + payment := payments[len(payments)-1] + htlcs := payment.Htlcs + if len(htlcs) == 0 { + t.Fatalf("no htlcs") + } + + succeeded := 0 + for _, htlc := range htlcs { + if htlc.Status == lnrpc.HTLCAttempt_SUCCEEDED { + succeeded++ + } + } + + if succeeded != num { + t.Fatalf("expected %v succussful HTLCs, got %v", num, + succeeded) + } + } + + // assertSettledInvoice checks that the invoice for the given payment + // hash is settled, and has been paid using num HTLCs. + assertSettledInvoice := func(node *lntest.HarnessNode, rhash []byte, + num int) { + + found := false + offset := uint64(0) + for !found { + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + invoicesResp, err := node.ListInvoices( + ctxt, &lnrpc.ListInvoiceRequest{ + IndexOffset: offset, + }, + ) + if err != nil { + t.Fatalf("error when obtaining payments: %v", + err) + } + + if len(invoicesResp.Invoices) == 0 { + break + } + + for _, inv := range invoicesResp.Invoices { + if !bytes.Equal(inv.RHash, rhash) { + continue + } + + // Assert that the amount paid to the invoice is + // correct. + if inv.AmtPaidSat != int64(paymentAmt) { + t.Fatalf("incorrect payment amt for "+ + "invoicewant: %d, got %d", + paymentAmt, inv.AmtPaidSat) + } + + if inv.State != lnrpc.Invoice_SETTLED { + t.Fatalf("Invoice not settled: %v", + inv.State) + } + + if len(inv.Htlcs) != num { + t.Fatalf("expected invoice to be "+ + "settled with %v HTLCs, had %v", + num, len(inv.Htlcs)) + } + + found = true + break + } + + offset = invoicesResp.LastIndexOffset + } + + if !found { + t.Fatalf("invoice not found") + } + } + + // Finally check that the payment shows up with three settled HTLCs in + // Alice's list of payments... + assertNumHtlcs(net.Alice, 3) + + // ...and in Bob's list of paid invoices. + assertSettledInvoice(net.Bob, rHash, 3) +} diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index 70febd9fa..d35fca729 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -14908,6 +14908,10 @@ var testsCases = []*testCase{ name: "psbt channel funding", test: testPsbtChanFunding, }, + { + name: "sendtoroute multi path payment", + test: testSendToRouteMultiPath, + }, } // TestLightningNetworkDaemon performs a series of integration tests amongst a From 5e72a4b77c7019b6fa1987d99f9212e9beaeb41f Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Wed, 1 Apr 2020 00:13:27 +0200 Subject: [PATCH 34/34] routing: exit on unexpected RequestRoute error We whitelist a set of "expected" errors that can be returned from RequestRoute, by converting them into a new type noRouteError. For any other error returned by RequestRoute, we'll now exit immediately. --- routing/mock_test.go | 2 +- routing/pathfind.go | 18 ---------- routing/payment_lifecycle.go | 26 ++++---------- routing/payment_session.go | 70 +++++++++++++++++++++++++++++++++--- 4 files changed, 73 insertions(+), 43 deletions(-) diff --git a/routing/mock_test.go b/routing/mock_test.go index f47a94204..9645f73bf 100644 --- a/routing/mock_test.go +++ b/routing/mock_test.go @@ -141,7 +141,7 @@ func (m *mockPaymentSession) RequestRoute(_, _ lnwire.MilliSatoshi, _, height uint32) (*route.Route, error) { if len(m.routes) == 0 { - return nil, fmt.Errorf("no routes") + return nil, errNoPathFound } r := m.routes[0] diff --git a/routing/pathfind.go b/routing/pathfind.go index 549d34166..1365958c6 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -58,24 +58,6 @@ var ( // DefaultAprioriHopProbability is the default a priori probability for // a hop. DefaultAprioriHopProbability = float64(0.6) - - // errNoTlvPayload is returned when the destination hop does not support - // a tlv payload. - errNoTlvPayload = errors.New("destination hop doesn't " + - "understand new TLV payloads") - - // errNoPaymentAddr is returned when the destination hop does not - // support payment addresses. - errNoPaymentAddr = errors.New("destination hop doesn't " + - "understand payment addresses") - - // errNoPathFound is returned when a path to the target destination does - // not exist in the graph. - errNoPathFound = errors.New("unable to find a path to destination") - - // errInsufficientLocalBalance is returned when none of the local - // channels have enough balance for the payment. - errInsufficientBalance = errors.New("insufficient local balance") ) // edgePolicyWithSource is a helper struct to keep track of the source node diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 3a0eb75b8..6ac79400a 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -210,11 +210,16 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) { log.Warnf("Failed to find route for payment %x: %v", p.paymentHash, err) + routeErr, ok := err.(noRouteError) + if !ok { + return [32]byte{}, nil, err + } + // There is no route to try, and we have no active // shards. This means that there is no way for us to // send the payment, so mark it failed with no route. if state.numShardsInFlight == 0 { - failureCode := errorToPaymentFailure(err) + failureCode := routeErr.FailureReason() log.Debugf("Marking payment %v permanently "+ "failed with no route: %v", p.paymentHash, failureCode) @@ -570,25 +575,6 @@ func (p *shardHandler) collectResult(attempt *channeldb.HTLCAttemptInfo) ( }, nil } -// errorToPaymentFailure takes a path finding error and converts it into a -// payment-level failure. -func errorToPaymentFailure(err error) channeldb.FailureReason { - switch err { - case - errNoTlvPayload, - errNoPaymentAddr, - errNoPathFound, - errEmptyPaySession: - - return channeldb.FailureReasonNoRoute - - case errInsufficientBalance: - return channeldb.FailureReasonInsufficientBalance - } - - return channeldb.FailureReasonError -} - // createNewPaymentAttempt creates a new payment attempt from the given route. func (p *shardHandler) createNewPaymentAttempt(rt *route.Route) ( lnwire.ShortChannelID, *lnwire.UpdateAddHTLC, diff --git a/routing/payment_session.go b/routing/payment_session.go index f8c297552..6a597ff01 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -1,8 +1,6 @@ package routing import ( - "errors" - "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" @@ -12,12 +10,73 @@ import ( // to prevent an HTLC being failed if some blocks are mined while it's in-flight. const BlockPadding uint16 = 3 -var ( +// noRouteError encodes a non-critical error encountered during path finding. +type noRouteError uint8 + +const ( + // errNoTlvPayload is returned when the destination hop does not support + // a tlv payload. + errNoTlvPayload noRouteError = iota + + // errNoPaymentAddr is returned when the destination hop does not + // support payment addresses. + errNoPaymentAddr + + // errNoPathFound is returned when a path to the target destination does + // not exist in the graph. + errNoPathFound + + // errInsufficientLocalBalance is returned when none of the local + // channels have enough balance for the payment. + errInsufficientBalance + // errEmptyPaySession is returned when the empty payment session is // queried for a route. - errEmptyPaySession = errors.New("empty payment session") + errEmptyPaySession ) +// Error returns the string representation of the noRouteError +func (e noRouteError) Error() string { + switch e { + case errNoTlvPayload: + return "destination hop doesn't understand new TLV payloads" + + case errNoPaymentAddr: + return "destination hop doesn't understand payment addresses" + + case errNoPathFound: + return "unable to find a path to destination" + + case errEmptyPaySession: + return "empty payment session" + + case errInsufficientBalance: + return "insufficient local balance" + + default: + return "unknown no-route error" + } +} + +// FailureReason converts a path finding error into a payment-level failure. +func (e noRouteError) FailureReason() channeldb.FailureReason { + switch e { + case + errNoTlvPayload, + errNoPaymentAddr, + errNoPathFound, + errEmptyPaySession: + + return channeldb.FailureReasonNoRoute + + case errInsufficientBalance: + return channeldb.FailureReasonInsufficientBalance + + default: + return channeldb.FailureReasonError + } +} + // PaymentSession is used during SendPayment attempts to provide routes to // attempt. It also defines methods to give the PaymentSession additional // information learned during the previous attempts. @@ -29,6 +88,9 @@ type PaymentSession interface { // argument should be set to instruct the payment session about the // number of in flight HTLCS for the payment, such that it can choose // splitting strategy accordingly. + // + // A noRouteError is returned if a non-critical error is encountered + // during path finding. RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, activeShards, height uint32) (*route.Route, error) }