From 2e920de292f73efc4aee7fc4bc3f080fd133d484 Mon Sep 17 00:00:00 2001
From: Joost Jager <joost.jager@gmail.com>
Date: Fri, 7 Jun 2019 11:27:55 +0200
Subject: [PATCH] routing+lnrpc: move default payment timeout out of router

This commit moves the default timeout out of router and thereby fixes a
bug that caused SendToRoute to not return the actual error, but a
timeout result instead. SendToRoute only tries a single route, so a
timeout should never happen.
---
 routing/payment_lifecycle.go |  8 ++++----
 routing/router.go            | 36 ++++++++++++++++++++----------------
 rpcserver.go                 |  1 +
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go
index 3710ff58..a9c9d4ce 100644
--- a/routing/payment_lifecycle.go
+++ b/routing/payment_lifecycle.go
@@ -182,10 +182,10 @@ func (p *paymentLifecycle) resumePayment() ([32]byte, *route.Route, error) {
 func (p *paymentLifecycle) createNewPaymentAttempt() (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.
+	// 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
diff --git a/routing/router.go b/routing/router.go
index 8bb50742..25e0b86f 100644
--- a/routing/router.go
+++ b/routing/router.go
@@ -29,10 +29,10 @@ import (
 )
 
 const (
-	// defaultPayAttemptTimeout is a duration that we'll use to determine
-	// if we should give up on a payment attempt. This will be used if a
-	// value isn't specified in the LightningNode struct.
-	defaultPayAttemptTimeout = time.Duration(time.Second * 60)
+	// DefaultPayAttemptTimeout is the default payment attempt timeout. The
+	// payment attempt timeout defines the duration after which we stop
+	// trying more routes for a payment.
+	DefaultPayAttemptTimeout = time.Duration(time.Second * 60)
 
 	// DefaultChannelPruneExpiry is the default duration used to determine
 	// if a channel should be pruned or not.
@@ -529,6 +529,9 @@ 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.MissionControl.NewPaymentSessionEmpty()
 
 			lPayment := &LightningPayment{
@@ -1572,7 +1575,8 @@ type LightningPayment struct {
 	// PayAttemptTimeout is a timeout value that we'll use to determine
 	// when we should should abandon the payment attempt after consecutive
 	// payment failure. This prevents us from attempting to send a payment
-	// indefinitely.
+	// indefinitely. A zero value means the payment will never time out.
+	//
 	// TODO(halseth): make wallclock time to allow resume after startup.
 	PayAttemptTimeout time.Duration
 
@@ -1701,8 +1705,11 @@ func (r *ChannelRouter) SendToRoute(hash lntypes.Hash, route *route.Route) (
 
 	// Create a (mostly) dummy payment, as the created payment session is
 	// not going to do path finding.
-	// TODO(halseth): sendPayment doesn't relly need LightningPayment, make
+	// 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,
 	}
@@ -1768,22 +1775,12 @@ func (r *ChannelRouter) sendPayment(
 		return [32]byte{}, nil, err
 	}
 
-	var payAttemptTimeout time.Duration
-	if payment.PayAttemptTimeout == time.Duration(0) {
-		payAttemptTimeout = defaultPayAttemptTimeout
-	} else {
-		payAttemptTimeout = payment.PayAttemptTimeout
-	}
-
-	timeoutChan := time.After(payAttemptTimeout)
-
 	// 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,
-		timeoutChan:    timeoutChan,
 		currentHeight:  currentHeight,
 		finalCLTVDelta: uint16(payment.FinalCLTVDelta),
 		attempt:        existingAttempt,
@@ -1791,6 +1788,13 @@ func (r *ChannelRouter) sendPayment(
 		lastError:      nil,
 	}
 
+	// 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)
+	}
+
 	return p.resumePayment()
 
 }
diff --git a/rpcserver.go b/rpcserver.go
index dca0a93a..683f8aa2 100644
--- a/rpcserver.go
+++ b/rpcserver.go
@@ -3070,6 +3070,7 @@ func (r *rpcServer) dispatchPaymentIntent(
 			RouteHints:        payIntent.routeHints,
 			OutgoingChannelID: payIntent.outgoingChannelID,
 			PaymentRequest:    payIntent.payReq,
+			PayAttemptTimeout: routing.DefaultPayAttemptTimeout,
 		}
 
 		preImage, route, routerErr = r.server.chanRouter.SendPayment(