lnwire+htlcswitch+routing: report and interpret incorrect_or_unknown_payment_details height

channeldb/db.go

@@ -49,6 +49,12 @@ var (
 			number:    11,
 			migration: migrateInvoices,
 		},
+		{
+			// The DB version where final cltv deltas are stored in
+			// mission control.
+			number:    12,
+			migration: migrateMcFinalCltv,
+		},
 	}
 
 	// Big endian is the preferred byte order, due to cursor scans over

channeldb/migration_12_mc_final_cltv.go

@@ -0,0 +1,20 @@
+package channeldb
+
+import "github.com/coreos/bbolt"
+
+// migrateMcFinalCltv clears mission control to start with a clean slate on
+// which final cltv delta values are stored too. Because mission control data is
+// not critical we opt for simply clearing the existing data rather than a
+// read/update/write migration.
+func migrateMcFinalCltv(tx *bbolt.Tx) error {
+	log.Infof("Migrating to new mission control store by clearing " +
+		"existing data")
+
+	resultsKey := []byte("missioncontrol-results")
+	if err := tx.DeleteBucket(resultsKey); err != nil {
+		return err
+	}
+
+	log.Infof("Migration to new mission control completed!")
+	return nil
+}

lnrpc/routerrpc/router.proto

@@ -161,6 +161,17 @@ message SendToRouteRequest {
 
     /// Route that should be used to attempt to complete the payment.
     lnrpc.Route route = 2;
+
+    /**
+    The final cltv delta requirement for the payment request. This field is only
+    required to properly update mission control in case the payment fails with
+    incorrect_or_unknown_payment_details. Matching the final cltv delta with
+    the expiry height of the final htlc and the returned acceptance height in
+    the failure message, allows discerning an incorrect final expiry delta from
+    an unexpected delay along the route. Only in the latter case, mission
+    control will apply a penalty to the intermediary nodes.
+    **/
+    int32 final_cltv_delta = 3;
 }
 
 message SendToRouteResponse {

lnrpc/routerrpc/router_server.go

@@ -280,7 +280,9 @@ func (s *Server) SendToRoute(ctx context.Context,
 		return nil, err
 	}
 
-	preimage, err := s.cfg.Router.SendToRoute(hash, route)
+	preimage, err := s.cfg.Router.SendToRoute(
+		hash, route, req.FinalCltvDelta,
+	)
 
 	// In the success case, return the preimage.
 	if err == nil {

lnrpc/rpc.proto

@@ -910,6 +910,17 @@ message SendToRouteRequest {
 
     /// Route that should be used to attempt to complete the payment.
     Route route = 4;
+
+    /**
+    The final cltv delta requirement for the payment request. This field is only
+    required to properly update mission control in case the payment fails with
+    incorrect_or_unknown_payment_details. Matching the final cltv delta with
+    the expiry height of the final htlc and the returned acceptance height in
+    the failure message, allows discerning an incorrect final expiry delta from
+    an unexpected delay along the route. Only in the latter case, mission
+    control will apply a penalty to the intermediary nodes.
+    **/
+    int32 final_cltv_delta = 5;
 }
 
 message ChannelPoint {

lnrpc/rpc.swagger.json

@@ -3411,6 +3411,11 @@
         "route": {
           "$ref": "#/definitions/lnrpcRoute",
           "description": "/ Route that should be used to attempt to complete the payment."
+        },
+        "final_cltv_delta": {
+          "type": "integer",
+          "format": "int32",
+          "description": "*\nThe final cltv delta requirement for the payment request. This field is only\nrequired to properly update mission control in case the payment fails with\nincorrect_or_unknown_payment_details. Matching the final cltv delta with\nthe expiry height of the final htlc and the returned acceptance height in\nthe failure message, allows discerning an incorrect final expiry delta from\nan unexpected delay along the route. Only in the latter case, mission\ncontrol will apply a penalty to the intermediary nodes."
         }
       }
     },

routing/missioncontrol.go

@@ -164,6 +164,7 @@ type paymentResult struct {
 	success            bool
 	failureSourceIdx   *int
 	failure            lnwire.FailureMessage
+	finalCltvDelta     uint32
 }
 
 // NewMissionControl returns a new instance of missionControl.
@@ -385,8 +386,8 @@ func (m *MissionControl) GetHistorySnapshot() *MissionControlSnapshot {
 // returns a reason if this failure is a final failure. In that case no further
 // payment attempts need to be made.
 func (m *MissionControl) ReportPaymentFail(paymentID uint64, rt *route.Route,
-	failureSourceIdx *int, failure lnwire.FailureMessage) (
-	*channeldb.FailureReason, error) {
+	finalCltvDelta uint32, failureSourceIdx *int,
+	failure lnwire.FailureMessage) (*channeldb.FailureReason, error) {
 
 	timestamp := m.now()
 
@@ -398,6 +399,7 @@ func (m *MissionControl) ReportPaymentFail(paymentID uint64, rt *route.Route,
 		failureSourceIdx: failureSourceIdx,
 		failure:          failure,
 		route:            rt,
+		finalCltvDelta:   finalCltvDelta,
 	}
 
 	return m.processPaymentResult(result)
@@ -446,8 +448,8 @@ func (m *MissionControl) applyPaymentResult(
 
 	// Interpret result.
 	i := interpretResult(
-		result.route, result.success, result.failureSourceIdx,
-		result.failure,
+		result.route, result.finalCltvDelta, result.success,
+		result.failureSourceIdx, result.failure,
 	)
 
 	// Update mission control state using the interpretation.

routing/missioncontrol_store.go

@@ -128,7 +128,7 @@ func serializeResult(rp *paymentResult) ([]byte, []byte, error) {
 		&b,
 		uint64(rp.timeFwd.UnixNano()),
 		uint64(rp.timeReply.UnixNano()),
-		rp.success, dbFailureSourceIdx,
+		rp.success, rp.finalCltvDelta, dbFailureSourceIdx,
 	)
 	if err != nil {
 		return nil, nil, err
@@ -174,7 +174,8 @@ func deserializeResult(k, v []byte) (*paymentResult, error) {
 	)
 
 	err := channeldb.ReadElements(
-		r, &timeFwd, &timeReply, &result.success, &dbFailureSourceIdx,
+		r, &timeFwd, &timeReply, &result.success, &result.finalCltvDelta,
+		&dbFailureSourceIdx,
 	)
 	if err != nil {
 		return nil, err

routing/missioncontrol_store_test.go

@@ -66,6 +66,7 @@ func TestMissionControlStore(t *testing.T) {
 		id:               99,
 		timeReply:        testTime,
 		timeFwd:          testTime.Add(-time.Minute),
+		finalCltvDelta:   40,
 	}
 
 	result2 := result1

routing/missioncontrol_test.go

@@ -29,9 +29,10 @@ var (
 		},
 	}
 
-	mcTestTime  = time.Date(2018, time.January, 9, 14, 00, 00, 0, time.UTC)
-	mcTestNode1 = mcTestRoute.Hops[0].PubKeyBytes
-	mcTestNode2 = mcTestRoute.Hops[1].PubKeyBytes
+	mcTestTime       = time.Date(2018, time.January, 9, 14, 00, 00, 0, time.UTC)
+	mcTestNode1      = mcTestRoute.Hops[0].PubKeyBytes
+	mcTestNode2      = mcTestRoute.Hops[1].PubKeyBytes
+	mcFinalCltvDelta = uint32(40)
 )
 
 type mcTestContext struct {
@@ -110,7 +111,8 @@ func (ctx *mcTestContext) reportFailure(amt lnwire.MilliSatoshi,
 
 	errorSourceIdx := 1
 	ctx.mc.ReportPaymentFail(
-		ctx.pid, mcTestRoute, &errorSourceIdx, failure,
+		ctx.pid, mcTestRoute, mcFinalCltvDelta,
+		&errorSourceIdx, failure,
 	)
 }
 

routing/mock_test.go

@@ -99,8 +99,8 @@ type mockMissionControl struct {
 var _ MissionController = (*mockMissionControl)(nil)
 
 func (m *mockMissionControl) ReportPaymentFail(paymentID uint64, rt *route.Route,
-	failureSourceIdx *int, failure lnwire.FailureMessage) (
-	*channeldb.FailureReason, error) {
+	finalCltvDelta uint32, failureSourceIdx *int,
+	failure lnwire.FailureMessage) (*channeldb.FailureReason, error) {
 
 	return nil, nil
 }

routing/payment_lifecycle.go

@@ -352,7 +352,8 @@ func (p *paymentLifecycle) sendPaymentAttempt(firstHop lnwire.ShortChannelID,
 func (p *paymentLifecycle) handleSendError(sendErr error) error {
 
 	reason := p.router.processSendError(
-		p.attempt.PaymentID, &p.attempt.Route, sendErr,
+		p.attempt.PaymentID, &p.attempt.Route, uint32(p.finalCLTVDelta),
+		sendErr,
 	)
 	if reason == nil {
 		// Save the forwarding error so it can be returned if

routing/result_interpretation.go

@@ -58,8 +58,8 @@ type interpretedResult struct {
 
 // interpretResult interprets a payment outcome and returns an object that
 // contains information required to update mission control.
-func interpretResult(rt *route.Route, success bool, failureSrcIdx *int,
-	failure lnwire.FailureMessage) *interpretedResult {
+func interpretResult(rt *route.Route, finalCltvDelta uint32, success bool,
+	failureSrcIdx *int, failure lnwire.FailureMessage) *interpretedResult {
 
 	i := &interpretedResult{
 		pairResults: make(map[DirectedNodePair]pairResult),
@@ -68,7 +68,7 @@ func interpretResult(rt *route.Route, success bool, failureSrcIdx *int,
 	if success {
 		i.processSuccess(rt)
 	} else {
-		i.processFail(rt, failureSrcIdx, failure)
+		i.processFail(rt, finalCltvDelta, failureSrcIdx, failure)
 	}
 	return i
 }
@@ -82,7 +82,7 @@ func (i *interpretedResult) processSuccess(route *route.Route) {
 
 // processFail processes a failed payment attempt.
 func (i *interpretedResult) processFail(
-	rt *route.Route, errSourceIdx *int,
+	rt *route.Route, finalCltvDelta uint32, errSourceIdx *int,
 	failure lnwire.FailureMessage) {
 
 	if errSourceIdx == nil {
@@ -99,7 +99,7 @@ func (i *interpretedResult) processFail(
 	// A failure from the final hop was received.
 	case len(rt.Hops):
 		i.processPaymentOutcomeFinal(
-			rt, failure,
+			rt, finalCltvDelta, failure,
 		)
 
 	// An intermediate hop failed. Interpret the outcome, update reputation
@@ -142,7 +142,8 @@ func (i *interpretedResult) processPaymentOutcomeSelf(
 
 // processPaymentOutcomeFinal handles failures sent by the final hop.
 func (i *interpretedResult) processPaymentOutcomeFinal(
-	route *route.Route, failure lnwire.FailureMessage) {
+	route *route.Route, finalCltvDelta uint32,
+	failure lnwire.FailureMessage) {
 
 	n := len(route.Hops)
 
@@ -151,7 +152,7 @@ func (i *interpretedResult) processPaymentOutcomeFinal(
 	// incorrect htlc, we want to retry via another route. Invalid onion
 	// failures are not expected, because the final node wouldn't be able to
 	// encrypt that failure.
-	switch failure.(type) {
+	switch msg := failure.(type) {
 
 	// Expiry or amount of the HTLC doesn't match the onion, try another
 	// route.
@@ -178,14 +179,29 @@ func (i *interpretedResult) processPaymentOutcomeFinal(
 			i.successPairRange(route, 0, n-2)
 		}
 
+	// We used the wrong amount, fail the payment. This is a legacy failure
+	// that we still need to handle.
+	case *lnwire.FailIncorrectPaymentAmount:
+		// Assign all pairs a success result, as the payment reached the
+		// destination correctly.
+		i.successPairRange(route, 0, n-1)
+		i.finalFailureReason = &reasonIncorrectDetails
+
 	// We are using wrong payment hash or amount, fail the payment.
-	case *lnwire.FailIncorrectPaymentAmount,
-		*lnwire.FailIncorrectDetails:
+	case *lnwire.FailIncorrectDetails:
+		// If the htlc reached the receiver with not enough blocks left,
+		// there must have been a delay on the path.
+		finalExpiry := route.Hops[len(route.Hops)-1].OutgoingTimeLock
+		if msg.Height()+finalCltvDelta > finalExpiry {
+			// As we don't know which node is to blame, we penalize
+			// all pairs along the route and retry.
+			i.failPairRange(route, 0, n-1)
+			return
+		}
 
 		// Assign all pairs a success result, as the payment reached the
 		// destination correctly.
 		i.successPairRange(route, 0, n-1)
-
 		i.finalFailureReason = &reasonIncorrectDetails
 
 	// The HTLC that was extended to the final hop expires too soon. Fail