Repository: lightningnetwork/lnd
Pull request #4198 (2020-04-16 08:47:03)

routing: payment splitting pre-check
Splitting pre-check to see if the total balance is sufficient for the payment. Otherwise splitting won't ever result in a successful payment, but small shards may be held by the recipient until mpp timeout.

joostjager: Hold off re-review, found a problem
joostjager: Problem fixed. In `findPath` still only the max was checked and not the total.

Patch:
diff --git a/channeldb/payments.go b/channeldb/payments.go
index 16497dcdc..7c5f49a2a 100644
--- a/channeldb/payments.go
+++ b/channeldb/payments.go
@@ -116,9 +116,6 @@ const (
 
 	// FailureReasonInsufficientBalance indicates that we didn't have enough
 	// balance to complete the payment.
-	//
-	// This reason isn't assigned anymore, but may still exist for older
-	// payments.
 	FailureReasonInsufficientBalance FailureReason = 4
 
 	// TODO(halseth): cancel state.
diff --git a/routing/pathfind.go b/routing/pathfind.go
index b6879cc6d..e5def09b5 100644
--- a/routing/pathfind.go
+++ b/routing/pathfind.go
@@ -326,13 +326,14 @@ type PathFindingConfig struct {
 	MinProbability float64
 }
 
-// getMaxOutgoingAmt returns the maximum available balance in any of the
-// channels of the given node.
-func getMaxOutgoingAmt(node route.Vertex, outgoingChan *uint64,
+// getOutgoingBalance returns the maximum available balance in any of the
+// channels of the given node. The second return parameters is the total
+// available balance.
+func getOutgoingBalance(node route.Vertex, outgoingChan *uint64,
 	bandwidthHints map[uint64]lnwire.MilliSatoshi,
-	g routingGraph) (lnwire.MilliSatoshi, error) {
+	g routingGraph) (lnwire.MilliSatoshi, lnwire.MilliSatoshi, error) {
 
-	var max lnwire.MilliSatoshi
+	var max, total lnwire.MilliSatoshi
 	cb := func(edgeInfo *channeldb.ChannelEdgeInfo, outEdge,
 		_ *channeldb.ChannelEdgePolicy) error {
 
@@ -349,26 +350,30 @@ func getMaxOutgoingAmt(node route.Vertex, outgoingChan *uint64,
 
 		bandwidth, ok := bandwidthHints[chanID]
 
-		// If the bandwidth is not available for whatever reason, don't
-		// fail the pathfinding early.
+		// If the bandwidth is not available, use the channel capacity.
+		// This can happen when a channel is added to the graph after
+		// we've already queried the bandwidth hints.
 		if !ok {
-			max = lnwire.MaxMilliSatoshi
-			return nil
+			bandwidth = lnwire.NewMSatFromSatoshis(
+				edgeInfo.Capacity,
+			)
 		}
 
 		if bandwidth > max {
 			max = bandwidth
 		}
 
+		total += bandwidth
+
 		return nil
 	}
 
 	// Iterate over all channels of the to node.
 	err := g.forEachNodeChannel(node, cb)
 	if err != nil {
-		return 0, err
+		return 0, 0, err
 	}
-	return max, err
+	return max, total, err
 }
 
 // findPath attempts to find a path from the source node within the ChannelGraph
@@ -447,12 +452,21 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 	self := g.graph.sourceNode()
 
 	if source == self {
-		max, err := getMaxOutgoingAmt(
+		max, total, err := getOutgoingBalance(
 			self, r.OutgoingChannelID, g.bandwidthHints, g.graph,
 		)
 		if err != nil {
 			return nil, err
 		}
+
+		// If the total outgoing balance isn't sufficient, it will be
+		// impossible to complete the payment.
+		if total < amt {
+			return nil, errInsufficientBalance
+		}
+
+		// If there is only not enough capacity on a single route, it
+		// may still be possible to complete the payment by splitting.
 		if max < amt {
 			return nil, errNoPathFound
 		}
diff --git a/routing/payment_session.go b/routing/payment_session.go
index f5a49eff0..5cd8d9f7d 100644
--- a/routing/payment_session.go
+++ b/routing/payment_session.go
@@ -30,6 +30,10 @@ const (
 	// 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
@@ -57,6 +61,9 @@ func (e noRouteError) Error() string {
 	case errEmptyPaySession:
 		return "empty payment session"
 
+	case errInsufficientBalance:
+		return "insufficient local balance"
+
 	default:
 		return "unknown no-route error"
 	}
@@ -73,6 +80,9 @@ func (e noRouteError) FailureReason() channeldb.FailureReason {
 
 		return channeldb.FailureReasonNoRoute
 
+	case errInsufficientBalance:
+		return channeldb.FailureReasonInsufficientBalance
+
 	default:
 		return channeldb.FailureReasonError
 	}
@@ -278,6 +288,16 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi,
 			// Go pathfinding.
 			continue
 
+		// If there isn't enough local bandwidth, there is no point in
+		// splitting. It won't be possible to create a complete set in
+		// any case, but the sent out partial payments would be held by
+		// the receiver until the mpp timeout.
+		case err == errInsufficientBalance:
+			p.log.Debug("not splitting because local balance " +
+				"is insufficient")
+
+			return nil, err
+
 		case err != nil:
 			return nil, err
 		}


Test patch:
diff --git a/routing/integrated_routing_test.go b/routing/integrated_routing_test.go
index 2fdf7136e..1d897d8a8 100644
--- a/routing/integrated_routing_test.go
+++ b/routing/integrated_routing_test.go
@@ -107,7 +107,7 @@ func onePathGraph(g *mockGraph) {
 	g.addChannel(chanIm1Target, targetNodeID, im1NodeID, 100000)
 }
 
-func twoPathGraph(g *mockGraph) {
+func twoPathGraph(g *mockGraph, capacityOut, capacityIn btcutil.Amount) {
 	// Create the following network of nodes:
 	// source -> intermediate1 -> target
 	// source -> intermediate2 -> target
@@ -120,10 +120,10 @@ func twoPathGraph(g *mockGraph) {
 	intermediate2 := newMockNode(im2NodeID)
 	g.addNode(intermediate2)
 
-	g.addChannel(chanSourceIm1, sourceNodeID, im1NodeID, 200000)
-	g.addChannel(chanSourceIm2, sourceNodeID, im2NodeID, 200000)
-	g.addChannel(chanIm1Target, targetNodeID, im1NodeID, 100000)
-	g.addChannel(chanIm2Target, targetNodeID, im2NodeID, 100000)
+	g.addChannel(chanSourceIm1, sourceNodeID, im1NodeID, capacityOut)
+	g.addChannel(chanSourceIm2, sourceNodeID, im2NodeID, capacityOut)
+	g.addChannel(chanIm1Target, targetNodeID, im1NodeID, capacityIn)
+	g.addChannel(chanIm2Target, targetNodeID, im2NodeID, capacityIn)
 }
 
 var mppTestCases = []mppSendTestCase{
@@ -136,8 +136,10 @@ var mppTestCases = []mppSendTestCase{
 	// too. Mpp payment complete.
 	{
 
-		name:             "sufficient inbound",
-		graph:            twoPathGraph,
+		name: "sufficient inbound",
+		graph: func(g *mockGraph) {
+			twoPathGraph(g, 200000, 100000)
+		},
 		amt:              70000,
 		expectedAttempts: 5,
 		expectedSuccesses: []expectedHtlcSuccess{
@@ -155,8 +157,10 @@ var mppTestCases = []mppSendTestCase{
 
 	// Test that a cap on the max htlcs makes it impossible to pay.
 	{
-		name:              "no splitting",
-		graph:             twoPathGraph,
+		name: "no splitting",
+		graph: func(g *mockGraph) {
+			twoPathGraph(g, 200000, 100000)
+		},
 		amt:               70000,
 		expectedAttempts:  2,
 		expectedSuccesses: []expectedHtlcSuccess{},
@@ -188,6 +192,19 @@ var mppTestCases = []mppSendTestCase{
 		expectedFailure: true,
 		maxShards:       1000,
 	},
+
+	// Test that no attempts are made if the total local balance is
+	// insufficient.
+	{
+		name: "insufficient total balance",
+		graph: func(g *mockGraph) {
+			twoPathGraph(g, 100000, 500000)
+		},
+		amt:              300000,
+		expectedAttempts: 0,
+		expectedFailure:  true,
+		maxShards:        10,
+	},
 }
 
 // TestMppSend tests that a payment can be completed using multiple shards.
diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go
index 0683f36ee..deaccb289 100644
--- a/routing/pathfind_test.go
+++ b/routing/pathfind_test.go
@@ -1768,7 +1768,7 @@ func TestPathInsufficientCapacity(t *testing.T) {
 		noRestrictions, testPathFindingConfig,
 		sourceNode.PubKeyBytes, target, payAmt, 0,
 	)
-	if err != errNoPathFound {
+	if err != errInsufficientBalance {
 		t.Fatalf("graph shouldn't be able to support payment: %v", err)
 	}
 }
@@ -2790,6 +2790,7 @@ type pathFindingTestContext struct {
 	t                 *testing.T
 	graph             *channeldb.ChannelGraph
 	restrictParams    RestrictParams
+	bandwidthHints    map[uint64]lnwire.MilliSatoshi
 	pathFindingConfig PathFindingConfig
 	testGraphInstance *testGraphInstance
 	source            route.Vertex
@@ -2844,8 +2845,8 @@ func (c *pathFindingTestContext) findPath(target route.Vertex,
 	error) {
 
 	return dbFindPath(
-		c.graph, nil, nil, &c.restrictParams, &c.pathFindingConfig,
-		c.source, target, amt, 0,
+		c.graph, nil, c.bandwidthHints, &c.restrictParams,
+		&c.pathFindingConfig, c.source, target, amt, 0,
 	)
 }