diff --git a/cmd/lncli/cmd_invoice.go b/cmd/lncli/cmd_invoice.go index cdb177780..4053a8d7d 100644 --- a/cmd/lncli/cmd_invoice.go +++ b/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", diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 07056586a..a5ecb0f4e 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/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 diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index c37ed001d..860290c97 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/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 -// hints is bigger of equal than our target (currently twice the invoice -// amount) +// - Total incoming capacity (for non-zero invoice amounts): the sum of the +// remote balance amount in the hints is bigger of equal than our target +// (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 diff --git a/lnrpc/invoicesrpc/addinvoice_test.go b/lnrpc/invoicesrpc/addinvoice_test.go index d7ee620eb..2dcb516a6 100644 --- a/lnrpc/invoicesrpc/addinvoice_test.go +++ b/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 := 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) + chanID1, chanID2 := setupMockTwoChannels(h) // 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 diff --git a/lnrpc/invoicesrpc/invoices.swagger.json b/lnrpc/invoicesrpc/invoices.swagger.json index f220d5988..ff3294e70 100644 --- a/lnrpc/invoicesrpc/invoices.swagger.json +++ b/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", diff --git a/lnrpc/lightning.pb.go b/lnrpc/lightning.pb.go index 767198040..fe97d362c 100644 --- a/lnrpc/lightning.pb.go +++ b/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 diff --git a/lnrpc/lightning.proto b/lnrpc/lightning.proto index aa3138690..73cb4f72b 100644 --- a/lnrpc/lightning.proto +++ b/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; /* diff --git a/lnrpc/lightning.swagger.json b/lnrpc/lightning.swagger.json index 0b0f86adc..24223fad1 100644 --- a/lnrpc/lightning.swagger.json +++ b/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", diff --git a/lntest/itest/lnd_routing_test.go b/lntest/itest/lnd_routing_test.go index eca7744b7..caf5320af 100644 --- a/lntest/itest/lnd_routing_test.go +++ b/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 { - t.Fatalf("unable to retrieve alice's channels: %v", err) - } + require.NoError(t.t, err, "unable to retrieve alice's channels") var aliceBobChanID uint64 for _, channel := range listResp.Channels { @@ -1216,14 +1165,82 @@ func testInvoiceRoutingHints(net *lntest.NetworkHarness, t *harnessTest) { } } - if aliceBobChanID == 0 { - t.Fatalf("channel between alice and bob not found") + require.NotZero(t.t, aliceBobChanID, + "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 { - t.Fatalf("expected channel ID %d, got %d", aliceBobChanID, - chanID) + // Create an invoice for Alice that will populate the routing hints. + invoice := &lnrpc.Invoice{ + 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.