routing+routerrpc: take max cltv limit into account within path finding

With the introduction of the max CLTV limit parameter, nodes are able to
reject HTLCs that exceed it. This should also be applied to path
finding, otherwise HTLCs crafted by the same node that exceed it never
left the switch. This wasn't a big deal since the previous max CLTV
limit was ~5000 blocks. Once it was lowered to 1008, the issue became
more apparent. Therefore, all of our path finding attempts now have a
restriction of said limit in in order to properly carry out HTLCs to the
network.

lnrpc/routerrpc/router.pb.go

@@ -212,8 +212,9 @@ type SendPaymentRequest struct {
 	//any channel may be used.
 	OutgoingChanId uint64 `protobuf:"varint,8,opt,name=outgoing_chan_id,json=outgoingChanId,proto3" json:"outgoing_chan_id,omitempty"`
 	//*
-	//An optional maximum total time lock for the route. If zero, there is no
-	//maximum enforced.
+	//An optional maximum total time lock for the route. This should not exceed
+	//lnd's `--max-cltv-expiry` setting. If zero, then the value of
+	//`--max-cltv-expiry` is enforced.
 	CltvLimit int32 `protobuf:"varint,9,opt,name=cltv_limit,json=cltvLimit,proto3" json:"cltv_limit,omitempty"`
 	//*
 	//Optional route hints to reach the destination through private channels.

lnrpc/routerrpc/router.proto

@@ -54,8 +54,9 @@ message SendPaymentRequest {
     uint64 outgoing_chan_id = 8;
 
     /** 
-    An optional maximum total time lock for the route. If zero, there is no
-    maximum enforced.
+    An optional maximum total time lock for the route. This should not exceed
+    lnd's `--max-cltv-expiry` setting. If zero, then the value of
+    `--max-cltv-expiry` is enforced.
     */
     int32 cltv_limit = 9;
 

lnrpc/routerrpc/router_backend.go

@@ -55,6 +55,10 @@ type RouterBackend struct {
 	// Tower is the ControlTower instance that is used to track pending
 	// payments.
 	Tower routing.ControlTower
+
+	// MaxTotalTimelock is the maximum total time lock a route is allowed to
+	// have.
+	MaxTotalTimelock uint32
 }
 
 // MissionControl defines the mission control dependencies of routerrpc.
@@ -471,11 +475,14 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 		payIntent.OutgoingChannelID = &rpcPayReq.OutgoingChanId
 	}
 
-	// Take cltv limit from request if set.
-	if rpcPayReq.CltvLimit != 0 {
-		cltvLimit := uint32(rpcPayReq.CltvLimit)
-		payIntent.CltvLimit = &cltvLimit
+	// Take the CLTV limit from the request if set, otherwise use the max.
+	cltvLimit, err := ValidateCLTVLimit(
+		uint32(rpcPayReq.CltvLimit), r.MaxTotalTimelock,
+	)
+	if err != nil {
+		return nil, err
 	}
+	payIntent.CltvLimit = cltvLimit
 
 	// Take fee limit from request.
 	payIntent.FeeLimit = lnwire.NewMSatFromSatoshis(
@@ -675,3 +682,18 @@ func ValidatePayReqExpiry(payReq *zpay32.Invoice) error {
 
 	return nil
 }
+
+// ValidateCLTVLimit returns a valid CLTV limit given a value and a maximum. If
+// the value exceeds the maximum, then an error is returned. If the value is 0,
+// then the maximum is used.
+func ValidateCLTVLimit(val, max uint32) (uint32, error) {
+	switch {
+	case val == 0:
+		return max, nil
+	case val > max:
+		return 0, fmt.Errorf("total time lock of %v exceeds max "+
+			"allowed %v", val, max)
+	default:
+		return val, nil
+	}
+}

lnrpc/routerrpc/router_server.go

@@ -248,11 +248,14 @@ func (s *Server) EstimateRouteFee(ctx context.Context,
 	feeLimit := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin)
 
 	// Finally, we'll query for a route to the destination that can carry
-	// that target amount, we'll only request a single route.
+	// that target amount, we'll only request a single route. Set a
+	// restriction for the default CLTV limit, otherwise we can find a route
+	// that exceeds it and is useless to us.
 	route, err := s.cfg.Router.FindRoute(
 		s.cfg.RouterBackend.SelfNode, destNode, amtMsat,
 		&routing.RestrictParams{
 			FeeLimit:  feeLimit,
+			CltvLimit: s.cfg.RouterBackend.MaxTotalTimelock,
 		}, nil,
 	)
 	if err != nil {

lnrpc/rpc.pb.go

@@ -1052,8 +1052,9 @@ type SendRequest struct {
 	//any channel may be used.
 	OutgoingChanId uint64 `protobuf:"varint,9,opt,name=outgoing_chan_id,json=outgoingChanId,proto3" json:"outgoing_chan_id,omitempty"`
 	//*
-	//An optional maximum total time lock for the route. If zero, there is no
-	//maximum enforced.
+	//An optional maximum total time lock for the route. This should not exceed
+	//lnd's `--max-cltv-expiry` setting. If zero, then the value of
+	//`--max-cltv-expiry` is enforced.
 	CltvLimit uint32 `protobuf:"varint,10,opt,name=cltv_limit,json=cltvLimit,proto3" json:"cltv_limit,omitempty"`
 	//*
 	//An optional field that can be used to pass an arbitrary set of TLV records

lnrpc/rpc.proto

@@ -888,8 +888,9 @@ message SendRequest {
     uint64 outgoing_chan_id = 9;
 
     /** 
-    An optional maximum total time lock for the route. If zero, there is no
-    maximum enforced.
+    An optional maximum total time lock for the route. This should not exceed
+    lnd's `--max-cltv-expiry` setting. If zero, then the value of
+    `--max-cltv-expiry` is enforced.
     */
     uint32 cltv_limit = 10;
 

lnrpc/rpc.swagger.json

@@ -3437,7 +3437,7 @@
         "cltv_limit": {
           "type": "integer",
           "format": "int64",
-          "description": "* \nAn optional maximum total time lock for the route. If zero, there is no\nmaximum enforced."
+          "description": "* \nAn optional maximum total time lock for the route. This should not exceed\nlnd's `--max-cltv-expiry` setting. If zero, then the value of\n`--max-cltv-expiry` is enforced."
         },
         "dest_tlv": {
           "type": "object",

routing/pathfind.go

@@ -260,7 +260,7 @@ type RestrictParams struct {
 	// CltvLimit is the maximum time lock of the route excluding the final
 	// ctlv. After path finding is complete, the caller needs to increase
 	// all cltv expiry heights with the required final cltv delta.
-	CltvLimit *uint32
+	CltvLimit uint32
 
 	// DestPayloadTLV should be set to true if we need to drop off a TLV
 	// payload at the final hop in order to properly complete this payment
@@ -495,8 +495,8 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		incomingCltv := toNodeDist.incomingCltv +
 			uint32(timeLockDelta)
 
-		// Check that we have cltv limit and that we are within it.
-		if r.CltvLimit != nil && incomingCltv > *r.CltvLimit {
+		// Check that we are within our CLTV limit.
+		if incomingCltv > r.CltvLimit {
 			return
 		}
 

routing/pathfind_test.go

@@ -54,6 +54,7 @@ var (
 	noRestrictions = &RestrictParams{
 		FeeLimit:          noFeeLimit,
 		ProbabilitySource: noProbabilitySource,
+		CltvLimit:         math.MaxUint32,
 	}
 
 	testPathFindingConfig = &PathFindingConfig{}
@@ -789,6 +790,7 @@ func testBasicGraphPathFindingCase(t *testing.T, graphInstance *testGraphInstanc
 		&RestrictParams{
 			FeeLimit:          test.feeLimit,
 			ProbabilitySource: noProbabilitySource,
+			CltvLimit:         math.MaxUint32,
 		},
 		testPathFindingConfig,
 		sourceNode.PubKeyBytes, target, paymentAmt,
@@ -1916,6 +1918,7 @@ func TestRestrictOutgoingChannel(t *testing.T) {
 			FeeLimit:          noFeeLimit,
 			OutgoingChannelID: &outgoingChannelID,
 			ProbabilitySource: noProbabilitySource,
+			CltvLimit:         math.MaxUint32,
 		},
 		testPathFindingConfig,
 		sourceVertex, target, paymentAmt,
@@ -1942,7 +1945,7 @@ func TestRestrictOutgoingChannel(t *testing.T) {
 // TestCltvLimit asserts that a cltv limit is obeyed by the path finding
 // algorithm.
 func TestCltvLimit(t *testing.T) {
-	t.Run("no limit", func(t *testing.T) { testCltvLimit(t, 0, 1) })
+	t.Run("no limit", func(t *testing.T) { testCltvLimit(t, 2016, 1) })
 	t.Run("no path", func(t *testing.T) { testCltvLimit(t, 50, 0) })
 	t.Run("force high cost", func(t *testing.T) { testCltvLimit(t, 80, 3) })
 }
@@ -1998,19 +2001,13 @@ func testCltvLimit(t *testing.T, limit uint32, expectedChannel uint64) {
 	paymentAmt := lnwire.NewMSatFromSatoshis(100)
 	target := testGraphInstance.aliasMap["target"]
 
-	// Find the best path given the cltv limit.
-	var cltvLimit *uint32
-	if limit != 0 {
-		cltvLimit = &limit
-	}
-
 	path, err := findPath(
 		&graphParams{
 			graph: testGraphInstance.graph,
 		},
 		&RestrictParams{
 			FeeLimit:          noFeeLimit,
-			CltvLimit:         cltvLimit,
+			CltvLimit:         limit,
 			ProbabilitySource: noProbabilitySource,
 		},
 		testPathFindingConfig,
@@ -2195,6 +2192,7 @@ func testProbabilityRouting(t *testing.T, p10, p11, p20, minProbability float64,
 		&RestrictParams{
 			FeeLimit:          noFeeLimit,
 			ProbabilitySource: probabilitySource,
+			CltvLimit:         math.MaxUint32,
 		},
 		&PathFindingConfig{
 			PaymentAttemptPenalty: lnwire.NewMSatFromSatoshis(10),

routing/payment_session.go

@@ -73,15 +73,10 @@ func (p *paymentSession) RequestRoute(payment *LightningPayment,
 	// does not reject the HTLC if some blocks are mined while it's in-flight.
 	finalCltvDelta += BlockPadding
 
-	// If a route cltv limit was specified, 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.
-	var cltvLimit *uint32
-	if payment.CltvLimit != nil {
-		limit := *payment.CltvLimit - uint32(finalCltvDelta)
-		cltvLimit = &limit
-	}
+	// 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)
 
 	// TODO(roasbeef): sync logic amongst dist sys
 

routing/payment_session_test.go

@@ -20,7 +20,7 @@ func TestRequestRoute(t *testing.T) {
 
 		// We expect find path to receive a cltv limit excluding the
 		// final cltv delta (including the block padding).
-		if *r.CltvLimit != 22-uint32(BlockPadding) {
+		if r.CltvLimit != 22-uint32(BlockPadding) {
 			t.Fatal("wrong cltv limit")
 		}
 
@@ -58,7 +58,7 @@ func TestRequestRoute(t *testing.T) {
 	finalCltvDelta := uint16(8)
 
 	payment := &LightningPayment{
-		CltvLimit:      &cltvLimit,
+		CltvLimit:      cltvLimit,
 		FinalCLTVDelta: finalCltvDelta,
 	}
 

routing/router.go

@@ -1560,7 +1560,7 @@ type LightningPayment struct {
 
 	// CltvLimit is the maximum time lock that is allowed for attempts to
 	// complete this payment.
-	CltvLimit *uint32
+	CltvLimit uint32
 
 	// PaymentHash is the r-hash value to use within the HTLC extended to
 	// the first hop.

routing/router_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"image/color"
+	"math"
 	"math/rand"
 	"strings"
 	"sync/atomic"
@@ -219,6 +220,7 @@ func TestFindRoutesWithFeeLimit(t *testing.T) {
 	restrictions := &RestrictParams{
 		FeeLimit:          lnwire.NewMSatFromSatoshis(10),
 		ProbabilitySource: noProbabilitySource,
+		CltvLimit:         math.MaxUint32,
 	}
 
 	route, err := ctx.router.FindRoute(

rpcserver.go

@@ -505,6 +505,7 @@ func newRPCServer(s *server, macService *macaroons.Service,
 		MissionControl:   s.missionControl,
 		ActiveNetParams:  activeNetParams.Params,
 		Tower:            s.controlTower,
+		MaxTotalTimelock: cfg.MaxOutgoingCltvExpiry,
 	}
 
 	var (
@@ -2940,7 +2941,7 @@ func (r *rpcServer) unmarshallSendToRouteRequest(
 type rpcPaymentIntent struct {
 	msat              lnwire.MilliSatoshi
 	feeLimit          lnwire.MilliSatoshi
-	cltvLimit         *uint32
+	cltvLimit         uint32
 	dest              route.Vertex
 	rHash             [32]byte
 	cltvDelta         uint16
@@ -2987,10 +2988,14 @@ func extractPaymentIntent(rpcPayReq *rpcPaymentRequest) (rpcPaymentIntent, error
 		payIntent.outgoingChannelID = &rpcPayReq.OutgoingChanId
 	}
 
-	// Take cltv limit from request if set.
-	if rpcPayReq.CltvLimit != 0 {
-		payIntent.cltvLimit = &rpcPayReq.CltvLimit
+	// Take the CLTV limit from the request if set, otherwise use the max.
+	cltvLimit, err := routerrpc.ValidateCLTVLimit(
+		rpcPayReq.CltvLimit, cfg.MaxOutgoingCltvExpiry,
+	)
+	if err != nil {
+		return payIntent, err
 	}
+	payIntent.cltvLimit = cltvLimit
 
 	if len(rpcPayReq.DestTlv) != 0 {
 		var err error