Repository: lightningnetwork/lnd
Pull request #4199 (2020-04-16 13:34:34)

routing: do not split payment if destination does not support mpp
Problems can occur for legacy payments or keysend payments (which are also non-mpp).


Patch:
diff --git a/routing/payment_session.go b/routing/payment_session.go
index c5a20f7a1..f5a49eff0 100644
--- a/routing/payment_session.go
+++ b/routing/payment_session.go
@@ -1,6 +1,10 @@
 package routing
 
 import (
+	"fmt"
+
+	"github.com/btcsuite/btclog"
+	"github.com/lightningnetwork/lnd/build"
 	"github.com/lightningnetwork/lnd/channeldb"
 	"github.com/lightningnetwork/lnd/lnwire"
 	"github.com/lightningnetwork/lnd/routing/route"
@@ -124,6 +128,36 @@ type paymentSession struct {
 	// specified in the payment is one, under no circumstances splitting
 	// will happen and this value remains unused.
 	minShardAmt lnwire.MilliSatoshi
+
+	// log is a payment session-specific logger.
+	log btclog.Logger
+}
+
+// newPaymentSession instantiates a new payment session.
+func newPaymentSession(p *LightningPayment,
+	getBandwidthHints func() (map[uint64]lnwire.MilliSatoshi, error),
+	getRoutingGraph func() (routingGraph, func(), error),
+	missionControl MissionController, pathFindingConfig PathFindingConfig) (
+	*paymentSession, error) {
+
+	edges, err := RouteHintsToEdges(p.RouteHints, p.Target)
+	if err != nil {
+		return nil, err
+	}
+
+	logPrefix := fmt.Sprintf("PaymentSession(%x):", p.PaymentHash)
+
+	return &paymentSession{
+		additionalEdges:   edges,
+		getBandwidthHints: getBandwidthHints,
+		payment:           p,
+		pathFinder:        findPath,
+		getRoutingGraph:   getRoutingGraph,
+		pathFindingConfig: pathFindingConfig,
+		missionControl:    missionControl,
+		minShardAmt:       DefaultShardMinAmt,
+		log:               build.NewPrefixLog(logPrefix, log),
+	}, nil
 }
 
 // RequestRoute returns a route which is likely to be capable for successfully
@@ -182,8 +216,7 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi,
 			return nil, err
 		}
 
-		log.Debugf("PaymentSession for %x: trying pathfinding with %v",
-			p.payment.PaymentHash, maxAmt)
+		p.log.Debugf("pathfinding for amt=%v", maxAmt)
 
 		// Get a routing graph.
 		routingGraph, cleanup, err := p.getRoutingGraph()
@@ -210,9 +243,22 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi,
 
 		switch {
 		case err == errNoPathFound:
+			// Don't split if this is a legacy payment without mpp
+			// record.
+			if p.payment.PaymentAddr == nil {
+				p.log.Debugf("not splitting because payment " +
+					"address is unspecified")
+
+				return nil, errNoPathFound
+			}
+
 			// No splitting if this is the last shard.
 			isLastShard := activeShards+1 >= p.payment.MaxShards
 			if isLastShard {
+				p.log.Debugf("not splitting because shard "+
+					"limit %v has been reached",
+					p.payment.MaxShards)
+
 				return nil, errNoPathFound
 			}
 
@@ -222,6 +268,10 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi,
 
 			// Put a lower bound on the minimum shard size.
 			if maxAmt < p.minShardAmt {
+				p.log.Debugf("not splitting because minimum "+
+					"shard amount %v has been reached",
+					p.minShardAmt)
+
 				return nil, errNoPathFound
 			}
 
diff --git a/routing/payment_session_source.go b/routing/payment_session_source.go
index a6016ab85..8122ff711 100644
--- a/routing/payment_session_source.go
+++ b/routing/payment_session_source.go
@@ -62,11 +62,6 @@ func (m *SessionSource) getRoutingGraph() (routingGraph, func(), error) {
 func (m *SessionSource) NewPaymentSession(p *LightningPayment) (
 	PaymentSession, error) {
 
-	edges, err := RouteHintsToEdges(p.RouteHints, p.Target)
-	if err != nil {
-		return nil, err
-	}
-
 	sourceNode, err := m.Graph.SourceNode()
 	if err != nil {
 		return nil, err
@@ -78,16 +73,15 @@ func (m *SessionSource) NewPaymentSession(p *LightningPayment) (
 		return generateBandwidthHints(sourceNode, m.QueryBandwidth)
 	}
 
-	return &paymentSession{
-		additionalEdges:   edges,
-		getBandwidthHints: getBandwidthHints,
-		payment:           p,
-		pathFinder:        findPath,
-		getRoutingGraph:   m.getRoutingGraph,
-		pathFindingConfig: m.PathFindingConfig,
-		missionControl:    m.MissionControl,
-		minShardAmt:       DefaultShardMinAmt,
-	}, nil
+	session, err := newPaymentSession(
+		p, getBandwidthHints, m.getRoutingGraph,
+		m.MissionControl, m.PathFindingConfig,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	return session, nil
 }
 
 // NewPaymentSessionEmpty creates a new paymentSession instance that is empty,


Test patch:
diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go
index 457ed4f46..cc71560d8 100644
--- a/routing/integrated_routing_context_test.go
+++ b/routing/integrated_routing_context_test.go
@@ -140,18 +140,20 @@ func (c *integratedRoutingContext) testPayment(maxShards uint32) ([]htlcAttempt,
 		MaxShards:      maxShards,
 	}
 
-	session := &paymentSession{
-		getBandwidthHints: getBandwidthHints,
-		payment:           &payment,
-		pathFinder:        findPath,
-		getRoutingGraph: func() (routingGraph, func(), error) {
+	session, err := newPaymentSession(
+		&payment, getBandwidthHints,
+		func() (routingGraph, func(), error) {
 			return c.graph, func() {}, nil
 		},
-		pathFindingConfig: c.pathFindingCfg,
-		missionControl:    mc,
-		minShardAmt:       lnwire.NewMSatFromSatoshis(5000),
+		mc, c.pathFindingCfg,
+	)
+	if err != nil {
+		c.t.Fatal(err)
 	}
 
+	// Override default minimum shard amount.
+	session.minShardAmt = lnwire.NewMSatFromSatoshis(5000)
+
 	// Now the payment control loop starts. It will keep trying routes until
 	// the payment succeeds.
 	var (
diff --git a/routing/payment_session_test.go b/routing/payment_session_test.go
index 60d960b87..ba55fcea6 100644
--- a/routing/payment_session_test.go
+++ b/routing/payment_session_test.go
@@ -13,9 +13,36 @@ func TestRequestRoute(t *testing.T) {
 		height = 10
 	)
 
-	findPath := func(
-		g *graphParams,
-		r *RestrictParams, cfg *PathFindingConfig,
+	cltvLimit := uint32(30)
+	finalCltvDelta := uint16(8)
+
+	payment := &LightningPayment{
+		CltvLimit:      cltvLimit,
+		FinalCLTVDelta: finalCltvDelta,
+		Amount:         1000,
+		FeeLimit:       1000,
+	}
+
+	session, err := newPaymentSession(
+		payment,
+		func() (map[uint64]lnwire.MilliSatoshi,
+			error) {
+
+			return nil, nil
+		},
+		func() (routingGraph, func(), error) {
+			return &sessionGraph{}, func() {}, nil
+		},
+		&MissionControl{cfg: &MissionControlConfig{}},
+		PathFindingConfig{},
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Override pathfinder with a mock.
+	session.pathFinder = func(
+		g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
 		source, target route.Vertex, amt lnwire.MilliSatoshi,
 		finalHtlcExpiry int32) ([]*channeldb.ChannelEdgePolicy, error) {
 
@@ -38,32 +65,6 @@ func TestRequestRoute(t *testing.T) {
 		return path, nil
 	}
 
-	cltvLimit := uint32(30)
-	finalCltvDelta := uint16(8)
-
-	payment := &LightningPayment{
-		CltvLimit:      cltvLimit,
-		FinalCLTVDelta: finalCltvDelta,
-		Amount:         1000,
-		FeeLimit:       1000,
-	}
-
-	session := &paymentSession{
-		getBandwidthHints: func() (map[uint64]lnwire.MilliSatoshi,
-			error) {
-
-			return nil, nil
-		},
-		payment:    payment,
-		pathFinder: findPath,
-		missionControl: &MissionControl{
-			cfg: &MissionControlConfig{},
-		},
-		getRoutingGraph: func() (routingGraph, func(), error) {
-			return &sessionGraph{}, func() {}, nil
-		},
-	}
-
 	route, err := session.RequestRoute(
 		payment.Amount, payment.FeeLimit, 0, height,
 	)