Merge pull request #2565 from joostjager/sendpayment-refactor

routing: sendPayment broken down into multiple functions

routing/router.go

@@ -1622,11 +1622,6 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 		}),
 	)
 
-	var (
-		preImage  [32]byte
-		sendError error
-	)
-
 	// We'll also fetch the current block height so we can properly
 	// calculate the required HTLC time locks within the route.
 	_, currentHeight, err := r.cfg.Chain.GetBestBlock()
@@ -1652,6 +1647,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 
 	// We'll continue until either our payment succeeds, or we encounter a
 	// critical error during path finding.
+	var lastError error
 	for {
 		// Before we attempt this next payment, we'll check to see if
 		// either we've gone past the payment attempt timeout, or the
@@ -1662,12 +1658,12 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 			errStr := fmt.Sprintf("payment attempt not completed "+
 				"before timeout of %v", payAttemptTimeout)
 
-			return preImage, nil, newErr(
+			return [32]byte{}, nil, newErr(
 				ErrPaymentAttemptTimeout, errStr,
 			)
 
 		case <-r.quit:
-			return preImage, nil, fmt.Errorf("router shutting down")
+			return [32]byte{}, nil, fmt.Errorf("router shutting down")
 
 		default:
 			// Fall through if we haven't hit our time limit, or
@@ -1680,29 +1676,67 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 		if err != nil {
 			// If we're unable to successfully make a payment using
 			// any of the routes we've found, then return an error.
-			if sendError != nil {
+			if lastError != nil {
 				return [32]byte{}, nil, fmt.Errorf("unable to "+
 					"route payment to destination: %v",
-					sendError)
+					lastError)
 			}
 
-			return preImage, nil, err
+			return [32]byte{}, nil, err
 		}
 
+		// Send payment attempt. It will return a final boolean
+		// indicating if more attempts are needed.
+		preimage, final, err := r.sendPaymentAttempt(
+			paySession, route, payment.PaymentHash,
+		)
+		if final {
+			return preimage, route, err
+		}
+
+		lastError = err
+	}
+}
+
+// sendPaymentAttempt tries to send the payment via the specified route. If
+// successful, it returns the obtained preimage. If an error occurs, the last
+// bool parameter indicates whether this is a final outcome or more attempts
+// should be made.
+func (r *ChannelRouter) sendPaymentAttempt(paySession *paymentSession,
+	route *Route, paymentHash [32]byte) ([32]byte, bool, error) {
+
 	log.Tracef("Attempting to send payment %x, using route: %v",
-			payment.PaymentHash, newLogClosure(func() string {
+		paymentHash, newLogClosure(func() string {
 			return spew.Sdump(route)
 		}),
 	)
 
+	preimage, err := r.sendToSwitch(route, paymentHash)
+	if err == nil {
+		return preimage, true, nil
+	}
+
+	log.Errorf("Attempt to send payment %x failed: %v",
+		paymentHash, err)
+
+	finalOutcome := r.processSendError(paySession, route, err)
+
+	return [32]byte{}, finalOutcome, err
+}
+
+// sendToSwitch sends a payment along the specified route and returns the
+// obtained preimage.
+func (r *ChannelRouter) sendToSwitch(route *Route, paymentHash [32]byte) (
+	[32]byte, error) {
+
 	// Generate the raw encoded sphinx packet to be included along
 	// with the htlcAdd message that we send directly to the
 	// switch.
 	onionBlob, circuit, err := generateSphinxPacket(
-			route, payment.PaymentHash[:],
+		route, paymentHash[:],
 	)
 	if err != nil {
-			return preImage, nil, err
+		return [32]byte{}, err
 	}
 
 	// Craft an HTLC packet to send to the layer 2 switch. The
@@ -1711,7 +1745,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	htlcAdd := &lnwire.UpdateAddHTLC{
 		Amount:      route.TotalAmount,
 		Expiry:      route.TotalTimeLock,
-			PaymentHash: payment.PaymentHash,
+		PaymentHash: paymentHash,
 	}
 	copy(htlcAdd.OnionBlob[:], onionBlob)
 
@@ -1721,33 +1755,34 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	firstHop := lnwire.NewShortChanIDFromInt(
 		route.Hops[0].ChannelID,
 	)
-		preImage, sendError = r.cfg.SendToSwitch(
+	return r.cfg.SendToSwitch(
 		firstHop, htlcAdd, circuit,
 	)
-		if sendError != nil {
-			// An error occurred when attempting to send the
-			// payment, depending on the error type, we'll either
-			// continue to send using alternative routes, or simply
-			// terminate this attempt.
-			log.Errorf("Attempt to send payment %x failed: %v",
-				payment.PaymentHash, sendError)
+}
 
-			fErr, ok := sendError.(*htlcswitch.ForwardingError)
+// processSendError analyzes the error for the payment attempt received from the
+// switch and updates mission control and/or channel policies. Depending on the
+// error type, this error is either the final outcome of the payment or we need
+// to continue with an alternative route. This is indicated by the boolean
+// return value.
+func (r *ChannelRouter) processSendError(paySession *paymentSession,
+	route *Route, err error) bool {
+
+	fErr, ok := err.(*htlcswitch.ForwardingError)
 	if !ok {
-				return preImage, nil, sendError
+		return true
 	}
 
 	errSource := fErr.ErrorSource
 	errVertex := NewVertex(errSource)
 
-			log.Tracef("node=%x reported failure when sending "+
-				"htlc=%x", errVertex, payment.PaymentHash[:])
+	log.Tracef("node=%x reported failure when sending htlc", errVertex)
 
 	// Always determine chan id ourselves, because a channel
 	// update with id may not be available.
 	failedEdge, err := getFailedEdge(route, errVertex)
 	if err != nil {
-				return preImage, nil, err
+		return true
 	}
 
 	// processChannelUpdateAndRetry is a closure that
@@ -1787,26 +1822,27 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	}
 
 	switch onionErr := fErr.FailureMessage.(type) {
+
 	// If the end destination didn't know the payment
 	// hash or we sent the wrong payment amount to the
 	// destination, then we'll terminate immediately.
 	case *lnwire.FailUnknownPaymentHash:
-				return preImage, nil, sendError
+		return true
 
 	// If we sent the wrong amount to the destination, then
 	// we'll exit early.
 	case *lnwire.FailIncorrectPaymentAmount:
-				return preImage, nil, sendError
+		return true
 
 	// If the time-lock that was extended to the final node
 	// was incorrect, then we can't proceed.
 	case *lnwire.FailFinalIncorrectCltvExpiry:
-				return preImage, nil, sendError
+		return true
 
 	// If we crafted an invalid onion payload for the final
 	// node, then we'll exit early.
 	case *lnwire.FailFinalIncorrectHtlcAmount:
-				return preImage, nil, sendError
+		return true
 
 	// Similarly, if the HTLC expiry that we extended to
 	// the final hop expires too soon, then will fail the
@@ -1815,12 +1851,12 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	// TODO(roasbeef): can happen to to race condition, try
 	// again with recent block height
 	case *lnwire.FailFinalExpiryTooSoon:
-				return preImage, nil, sendError
+		return true
 
 	// If we erroneously attempted to cross a chain border,
 	// then we'll cancel the payment.
 	case *lnwire.FailInvalidRealm:
-				return preImage, nil, sendError
+		return true
 
 	// If we get a notice that the expiry was too soon for
 	// an intermediate node, then we'll prune out the node
@@ -1829,17 +1865,17 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	case *lnwire.FailExpiryTooSoon:
 		r.applyChannelUpdate(&onionErr.Update, errSource)
 		paySession.ReportVertexFailure(errVertex)
-				continue
+		return false
 
 	// If we hit an instance of onion payload corruption or
 	// an invalid version, then we'll exit early as this
 	// shouldn't happen in the typical case.
 	case *lnwire.FailInvalidOnionVersion:
-				return preImage, nil, sendError
+		return true
 	case *lnwire.FailInvalidOnionHmac:
-				return preImage, nil, sendError
+		return true
 	case *lnwire.FailInvalidOnionKey:
-				return preImage, nil, sendError
+		return true
 
 	// If we get a failure due to violating the minimum
 	// amount, we'll apply the new minimum amount and retry
@@ -1848,7 +1884,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 		processChannelUpdateAndRetry(
 			&onionErr.Update, errSource,
 		)
-				continue
+		return false
 
 	// If we get a failure due to a fee, we'll apply the
 	// new fee update, and retry our attempt using the
@@ -1857,7 +1893,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 		processChannelUpdateAndRetry(
 			&onionErr.Update, errSource,
 		)
-				continue
+		return false
 
 	// If we get the failure for an intermediate node that
 	// disagrees with our time lock values, then we'll
@@ -1866,7 +1902,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 		processChannelUpdateAndRetry(
 			&onionErr.Update, errSource,
 		)
-				continue
+		return false
 
 	// The outgoing channel that this node was meant to
 	// forward one is currently disabled, so we'll apply
@@ -1874,7 +1910,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	case *lnwire.FailChannelDisabled:
 		r.applyChannelUpdate(&onionErr.Update, errSource)
 		paySession.ReportEdgeFailure(failedEdge)
-				continue
+		return false
 
 	// It's likely that the outgoing channel didn't have
 	// sufficient capacity, so we'll prune this edge for
@@ -1882,21 +1918,21 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	case *lnwire.FailTemporaryChannelFailure:
 		r.applyChannelUpdate(onionErr.Update, errSource)
 		paySession.ReportEdgeFailure(failedEdge)
-				continue
+		return false
 
 	// If the send fail due to a node not having the
 	// required features, then we'll note this error and
 	// continue.
 	case *lnwire.FailRequiredNodeFeatureMissing:
 		paySession.ReportVertexFailure(errVertex)
-				continue
+		return false
 
 	// If the send fail due to a node not having the
 	// required features, then we'll note this error and
 	// continue.
 	case *lnwire.FailRequiredChannelFeatureMissing:
 		paySession.ReportVertexFailure(errVertex)
-				continue
+		return false
 
 	// If the next hop in the route wasn't known or
 	// offline, we'll only the channel which we attempted
@@ -1907,18 +1943,18 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	// another node.
 	case *lnwire.FailUnknownNextPeer:
 		paySession.ReportEdgeFailure(failedEdge)
-				continue
+		return false
 
 	// If the node wasn't able to forward for which ever
 	// reason, then we'll note this and continue with the
 	// routes.
 	case *lnwire.FailTemporaryNodeFailure:
 		paySession.ReportVertexFailure(errVertex)
-				continue
+		return false
 
 	case *lnwire.FailPermanentNodeFailure:
 		paySession.ReportVertexFailure(errVertex)
-				continue
+		return false
 
 	// If we crafted a route that contains a too long time
 	// lock for an intermediate node, we'll prune the node.
@@ -1931,7 +1967,7 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 	// that node.
 	case *lnwire.FailExpiryTooFar:
 		paySession.ReportVertexFailure(errVertex)
-				continue
+		return false
 
 	// If we get a permanent channel or node failure, then
 	// we'll prune the channel in both directions and
@@ -1945,14 +1981,10 @@ func (r *ChannelRouter) sendPayment(payment *LightningPayment,
 			channelID: failedEdge.channelID,
 			direction: 1,
 		})
-				continue
+		return false
 
 	default:
-				return preImage, nil, sendError
-			}
-		}
-
-		return preImage, route, nil
+		return true
 	}
 }