Merge pull request #7082 from alpeb-btc/addinvoice-private-hints

addinvoice: provide hop hints for no-amount invoice

cmd/lncli/cmd_invoice.go

@@ -64,7 +64,10 @@ var addInvoiceCommand = cli.Command{
 			Name: "private",
 			Usage: "encode routing hints in the invoice with " +
 				"private channels in order to assist the " +
-				"payer in reaching you",
+				"payer in reaching you. If amt and amt_msat " +
+				"are zero, a large number of hints with " +
+				"these channels can be included, which " +
+				"might not be desirable.",
 		},
 		cli.BoolFlag{
 			Name: "amp",

docs/release-notes/release-notes-0.16.0.md

@@ -61,6 +61,10 @@
   Final resolution data will only be available for htlcs that are resolved
   after upgrading lnd.
 
+* Zero-amount private invoices [now provide hop
+  hints](https://github.com/lightningnetwork/lnd/pull/7082), up to `maxHopHints`
+  (20 currently).
+
 ## Wallet
 
 * [Allows Taproot public keys and tap scripts to be imported as watch-only
@@ -227,6 +231,7 @@ to refactor the itest for code health and maintenance.
 
 # Contributors (Alphabetical Order)
 
+* Alejandro Pedraza
 * andreihod
 * Carla Kirk-Cohen
 * Conner Babinchak

lnrpc/invoicesrpc/addinvoice.go

@@ -655,9 +655,9 @@ func newSelectHopHintsCfg(invoicesCfg *AddInvoiceConfig,
 // sufficientHints checks whether we have sufficient hop hints, based on the
 // any of the following criteria:
 //   - Hop hint count: the number of hints have reach our max target.
-//   - Total incoming capacity: the sum of the remote balance amount in the
+//   - Total incoming capacity (for non-zero invoice amounts): the sum of the
-//     hints is bigger of equal than our target (currently twice the invoice
+//     remote balance amount in the hints is bigger of equal than our target
-//     amount)
+//     (currently twice the invoice amount)
 //
 // We limit our number of hop hints like this to keep our invoice size down,
 // and to avoid leaking all our private channels when we don't need to.
@@ -669,7 +669,7 @@ func sufficientHints(nHintsLeft int, currentAmount,
 		return true
 	}
 
-	if currentAmount >= targetAmount {
+	if targetAmount != 0 && currentAmount >= targetAmount {
 		log.Debugf("Total hint amount: %v has reached target hint "+
 			"bandwidth: %v", currentAmount, targetAmount)
 		return true

lnrpc/invoicesrpc/addinvoice_test.go

@@ -463,7 +463,7 @@ var sufficientHintsTestCases = []struct {
 	targetAmount  lnwire.MilliSatoshi
 	done          bool
 }{{
-	name:          "not enoguh hints neither bandwidth",
+	name:          "not enough hints neither bandwidth",
 	nHintsLeft:    3,
 	currentAmount: 100,
 	targetAmount:  200,
@@ -473,11 +473,17 @@ var sufficientHintsTestCases = []struct {
 	nHintsLeft: 0,
 	done:       true,
 }, {
-	name:          "enoguh bandwidth",
+	name:          "enough bandwidth",
 	nHintsLeft:    1,
 	currentAmount: 200,
 	targetAmount:  200,
 	done:          true,
+}, {
+	name:          "no amount provided",
+	nHintsLeft:    1,
+	currentAmount: 100,
+	targetAmount:  0,
+	done:          false,
 }}
 
 func TestSufficientHints(t *testing.T) {
@@ -668,38 +674,7 @@ var populateHopHintsTestCases = []struct {
 	name: "populate hop hints stops after having considered all the open " +
 		"channels",
 	setupMock: func(h *hopHintsConfigMock) {
-		fundingOutpoint1 := wire.OutPoint{Index: 9}
+		chanID1, chanID2 := setupMockTwoChannels(h)
-		chanID1 := lnwire.NewChanIDFromOutPoint(&fundingOutpoint1)
-		remoteBalance1 := lnwire.MilliSatoshi(10_000_000)
-
-		fundingOutpoint2 := wire.OutPoint{Index: 2}
-		chanID2 := lnwire.NewChanIDFromOutPoint(&fundingOutpoint2)
-		remoteBalance2 := lnwire.MilliSatoshi(1_000_000)
-
-		allChannels := []*channeldb.OpenChannel{
-			// After sorting we will first process chanID1 and then
-			// chanID2.
-			{
-				LocalCommitment: channeldb.ChannelCommitment{
-					RemoteBalance: remoteBalance2,
-				},
-				FundingOutpoint: fundingOutpoint2,
-				ShortChannelID:  lnwire.NewShortChanIDFromInt(2),
-				IdentityPub:     getTestPubKey(),
-			},
-			{
-				LocalCommitment: channeldb.ChannelCommitment{
-					RemoteBalance: remoteBalance1,
-				},
-				FundingOutpoint: fundingOutpoint1,
-				ShortChannelID:  lnwire.NewShortChanIDFromInt(9),
-				IdentityPub:     getTestPubKey(),
-			},
-		}
-
-		h.Mock.On(
-			"FetchAllChannels",
-		).Once().Return(allChannels, nil)
 
 		// Prepare the mock for the first channel.
 		h.Mock.On(
@@ -750,8 +725,134 @@ var populateHopHintsTestCases = []struct {
 			},
 		},
 	},
+}, {
+	name: "consider all the open channels when amount is zero",
+	setupMock: func(h *hopHintsConfigMock) {
+		chanID1, chanID2 := setupMockTwoChannels(h)
+
+		// Prepare the mock for the first channel.
+		h.Mock.On(
+			"IsChannelActive", chanID1,
+		).Once().Return(true)
+
+		h.Mock.On(
+			"IsPublicNode", mock.Anything,
+		).Once().Return(true, nil)
+
+		h.Mock.On(
+			"FetchChannelEdgesByID", mock.Anything,
+		).Once().Return(
+			&channeldb.ChannelEdgeInfo{},
+			&channeldb.ChannelEdgePolicy{},
+			&channeldb.ChannelEdgePolicy{}, nil,
+		)
+
+		// Prepare the mock for the second channel.
+		h.Mock.On(
+			"IsChannelActive", chanID2,
+		).Once().Return(true)
+
+		h.Mock.On(
+			"IsPublicNode", mock.Anything,
+		).Once().Return(true, nil)
+
+		h.Mock.On(
+			"FetchChannelEdgesByID", mock.Anything,
+		).Once().Return(
+			&channeldb.ChannelEdgeInfo{},
+			&channeldb.ChannelEdgePolicy{},
+			&channeldb.ChannelEdgePolicy{}, nil,
+		)
+	},
+	maxHopHints: 10,
+	amount:      0,
+	expectedHopHints: [][]zpay32.HopHint{
+		{
+			{
+				NodeID:    getTestPubKey(),
+				ChannelID: 9,
+			},
+		}, {
+			{
+				NodeID:    getTestPubKey(),
+				ChannelID: 2,
+			},
+		},
+	},
+}, {
+	name: "consider all the open channels when amount is zero" +
+		" up to maxHopHints",
+	setupMock: func(h *hopHintsConfigMock) {
+		chanID1, _ := setupMockTwoChannels(h)
+
+		// Prepare the mock for the first channel.
+		h.Mock.On(
+			"IsChannelActive", chanID1,
+		).Once().Return(true)
+
+		h.Mock.On(
+			"IsPublicNode", mock.Anything,
+		).Once().Return(true, nil)
+
+		h.Mock.On(
+			"FetchChannelEdgesByID", mock.Anything,
+		).Once().Return(
+			&channeldb.ChannelEdgeInfo{},
+			&channeldb.ChannelEdgePolicy{},
+			&channeldb.ChannelEdgePolicy{}, nil,
+		)
+	},
+	maxHopHints: 1,
+	amount:      0,
+	expectedHopHints: [][]zpay32.HopHint{
+		{
+			{
+				NodeID:    getTestPubKey(),
+				ChannelID: 9,
+			},
+		},
+	},
 }}
 
+func setupMockTwoChannels(h *hopHintsConfigMock) (lnwire.ChannelID,
+	lnwire.ChannelID) {
+
+	fundingOutpoint1 := wire.OutPoint{Index: 9}
+	chanID1 := lnwire.NewChanIDFromOutPoint(&fundingOutpoint1)
+	remoteBalance1 := lnwire.MilliSatoshi(10_000_000)
+
+	fundingOutpoint2 := wire.OutPoint{Index: 2}
+	chanID2 := lnwire.NewChanIDFromOutPoint(&fundingOutpoint2)
+	remoteBalance2 := lnwire.MilliSatoshi(1_000_000)
+
+	allChannels := []*channeldb.OpenChannel{
+		// After sorting we will first process chanID1 and then
+		// chanID2.
+		{
+			LocalCommitment: channeldb.ChannelCommitment{
+				RemoteBalance: remoteBalance2,
+			},
+			FundingOutpoint: fundingOutpoint2,
+			ShortChannelID:  lnwire.NewShortChanIDFromInt(2),
+			IdentityPub:     getTestPubKey(),
+		},
+		{
+			LocalCommitment: channeldb.ChannelCommitment{
+				RemoteBalance: remoteBalance1,
+			},
+			FundingOutpoint: fundingOutpoint1,
+			ShortChannelID:  lnwire.NewShortChanIDFromInt(9),
+			IdentityPub:     getTestPubKey(),
+		},
+	}
+
+	h.Mock.On(
+		"FetchAllChannels",
+	).Once().Return(allChannels, nil)
+
+	return chanID1, chanID2
+}
+
 func TestPopulateHopHints(t *testing.T) {
 	for _, tc := range populateHopHintsTestCases {
 		tc := tc

lnrpc/invoicesrpc/invoices.swagger.json

@@ -514,7 +514,7 @@
         },
         "private": {
           "type": "boolean",
-          "description": "Whether this invoice should include routing hints for private channels."
+          "description": "Whether this invoice should include routing hints for private channels.\nNote: When enabled, if value and value_msat are zero, a large number of\nhints with these channels can be included, which might not be desirable."
         },
         "add_index": {
           "type": "string",

lnrpc/lightning.pb.go

@@ -11640,6 +11640,8 @@ type Invoice struct {
 	// invoice's destination.
 	RouteHints []*RouteHint `protobuf:"bytes,14,rep,name=route_hints,json=routeHints,proto3" json:"route_hints,omitempty"`
 	// Whether this invoice should include routing hints for private channels.
+	// Note: When enabled, if value and value_msat are zero, a large number of
+	// hints with these channels can be included, which might not be desirable.
 	Private bool `protobuf:"varint,15,opt,name=private,proto3" json:"private,omitempty"`
 	// The "add" index of this invoice. Each newly created invoice will increment
 	// this index making it monotonically increasing. Callers to the

lnrpc/lightning.proto

@@ -3401,6 +3401,8 @@ message Invoice {
     repeated RouteHint route_hints = 14;
 
     // Whether this invoice should include routing hints for private channels.
+    // Note: When enabled, if value and value_msat are zero, a large number of
+    // hints with these channels can be included, which might not be desirable.
     bool private = 15;
 
     /*

lnrpc/lightning.swagger.json

@@ -5048,7 +5048,7 @@
         },
         "private": {
           "type": "boolean",
-          "description": "Whether this invoice should include routing hints for private channels."
+          "description": "Whether this invoice should include routing hints for private channels.\nNote: When enabled, if value and value_msat are zero, a large number of\nhints with these channels can be included, which might not be desirable."
         },
         "add_index": {
           "type": "string",

lntest/itest/lnd_routing_test.go

@@ -1151,63 +1151,12 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) {
 	// Now that the channels are open, we'll take down Eve's node.
 	shutdownAndAssert(net, t, eve)
 
-	// Create an invoice for Alice that will populate the routing hints.
-	invoice := &lnrpc.Invoice{
-		Memo:    "routing hints",
-		Value:   int64(chanAmt / 4),
-		Private: true,
-	}
-
-	// Due to the way the channels were set up above, the channel between
-	// Alice and Bob should be the only channel used as a routing hint.
-	var predErr error
-	var decoded *lnrpc.PayReq
-	err := wait.Predicate(func() bool {
-		ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
-		resp, err := net.Alice.AddInvoice(ctxt, invoice)
-		if err != nil {
-			predErr = fmt.Errorf("unable to add invoice: %v", err)
-			return false
-		}
-
-		// We'll decode the invoice's payment request to determine which
-		// channels were used as routing hints.
-		payReq := &lnrpc.PayReqString{
-			PayReq: resp.PaymentRequest,
-		}
-		ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
-		decoded, err = net.Alice.DecodePayReq(ctxt, payReq)
-		if err != nil {
-			predErr = fmt.Errorf("unable to decode payment "+
-				"request: %v", err)
-			return false
-		}
-
-		if len(decoded.RouteHints) != 1 {
-			predErr = fmt.Errorf("expected one route hint, got %d",
-				len(decoded.RouteHints))
-			return false
-		}
-		return true
-	}, defaultTimeout)
-	if err != nil {
-		t.Fatalf(predErr.Error())
-	}
-
-	hops := decoded.RouteHints[0].HopHints
-	if len(hops) != 1 {
-		t.Fatalf("expected one hop in route hint, got %d", len(hops))
-	}
-	chanID := hops[0].ChanId
-
 	// We'll need the short channel ID of the channel between Alice and Bob
 	// to make sure the routing hint is for this channel.
 	listReq := &lnrpc.ListChannelsRequest{}
 	ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
 	listResp, err := net.Alice.ListChannels(ctxt, listReq)
-	if err != nil {
+	require.NoError(t.t, err, "unable to retrieve alice's channels")
-		t.Fatalf("unable to retrieve alice's channels: %v", err)
-	}
 
 	var aliceBobChanID uint64
 	for _, channel := range listResp.Channels {
@@ -1216,14 +1165,82 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) {
 		}
 	}
 
-	if aliceBobChanID == 0 {
+	require.NotZero(t.t, aliceBobChanID,
-		t.Fatalf("channel between alice and bob not found")
+		"channel between alice and bob not found")
+
+	checkInvoiceHints := func(invoice *lnrpc.Invoice) {
+		// Due to the way the channels were set up above, the channel
+		// between Alice and Bob should be the only channel used as a
+		// routing hint.
+		var predErr error
+		var decoded *lnrpc.PayReq
+		err := wait.Predicate(func() bool {
+			ctxt, _ := context.WithTimeout(ctxb, defaultTimeout)
+			resp, err := net.Alice.AddInvoice(ctxt, invoice)
+			if err != nil {
+				predErr = fmt.Errorf(
+					"unable to add invoice: %w", err)
+				return false
+			}
+
+			// We'll decode the invoice's payment request to
+			// determine which channels were used as routing hints.
+			payReq := &lnrpc.PayReqString{
+				PayReq: resp.PaymentRequest,
+			}
+			ctxt, _ = context.WithTimeout(ctxb, defaultTimeout)
+			decoded, err = net.Alice.DecodePayReq(ctxt, payReq)
+			if err != nil {
+				predErr = fmt.Errorf(
+					"unable to decode payment "+
+						"request: %w", err)
+
+				return false
+			}
+
+			if len(decoded.RouteHints) != 1 {
+				predErr = fmt.Errorf(
+					"expected one route hint, got %d",
+					len(decoded.RouteHints))
+
+				return false
+			}
+
+			return true
+		}, defaultTimeout)
+		if err != nil {
+			t.t.Fatalf(predErr.Error())
+		}
+
+		hops := decoded.RouteHints[0].HopHints
+		if len(hops) != 1 {
+			t.t.Fatalf("expected one hop in route hint, got %d",
+				len(hops))
+		}
+		chanID := hops[0].ChanId
+
+		if chanID != aliceBobChanID {
+			t.t.Fatalf("expected channel ID %d, got %d",
+				aliceBobChanID, chanID)
+		}
 	}
 
-	if chanID != aliceBobChanID {
+	// Create an invoice for Alice that will populate the routing hints.
-		t.Fatalf("expected channel ID %d, got %d", aliceBobChanID,
+	invoice := &lnrpc.Invoice{
-			chanID)
+		Memo:    "routing hints",
+		Value:   int64(chanAmt / 4),
+		Private: true,
 	}
+	checkInvoiceHints(invoice)
+
+	// Create another invoice for Alice with no value and ensure it still
+	// populates routing hints.
+	invoice = &lnrpc.Invoice{
+		Memo:    "routing hints with no amount",
+		Value:   0,
+		Private: true,
+	}
+	checkInvoiceHints(invoice)
 
 	// Now that we've confirmed the routing hints were added correctly, we
 	// can close all the channels and shut down all the nodes created.