From 7a83731edbd92f3dc45811711ef4e338765cd38c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 10 Aug 2020 16:06:39 -0700 Subject: [PATCH 1/5] lnrpc/invoicesrpc: move hop hint selection into new method --- lnrpc/invoicesrpc/addinvoice.go | 229 +++++++++++++++++--------------- 1 file changed, 123 insertions(+), 106 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index 573d2f381..909195e10 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -254,113 +254,14 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, return nil, nil, fmt.Errorf("could not fetch all channels") } - graph := cfg.ChanDB.ChannelGraph() - - numHints := 0 - for _, channel := range openChannels { - // We'll restrict the number of individual route hints - // to 20 to avoid creating overly large invoices. - if numHints >= 20 { - break - } - - // Since we're only interested in our private channels, - // we'll skip public ones. - isPublic := channel.ChannelFlags&lnwire.FFAnnounceChannel != 0 - if isPublic { - continue - } - - // Make sure the counterparty has enough balance in the - // channel for our amount. We do this in order to reduce - // payment errors when attempting to use this channel - // as a hint. - chanPoint := lnwire.NewChanIDFromOutPoint( - &channel.FundingOutpoint, - ) - if amtMSat >= channel.LocalCommitment.RemoteBalance { - log.Debugf("Skipping channel %v due to "+ - "not having enough remote balance", - chanPoint) - continue - } - - // Make sure the channel is active. - if !cfg.IsChannelActive(chanPoint) { - log.Debugf("Skipping channel %v due to not "+ - "being eligible to forward payments", - chanPoint) - continue - } - - // To ensure we don't leak unadvertised nodes, we'll - // make sure our counterparty is publicly advertised - // within the network. Otherwise, we'll end up leaking - // information about nodes that intend to stay - // unadvertised, like in the case of a node only having - // private channels. - var remotePub [33]byte - copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) - isRemoteNodePublic, err := graph.IsPublicNode(remotePub) - if err != nil { - log.Errorf("Unable to determine if node %x "+ - "is advertised: %v", remotePub, err) - continue - } - - if !isRemoteNodePublic { - log.Debugf("Skipping channel %v due to "+ - "counterparty %x being unadvertised", - chanPoint, remotePub) - continue - } - - // Fetch the policies for each end of the channel. - chanID := channel.ShortChanID().ToUint64() - info, p1, p2, err := graph.FetchChannelEdgesByID(chanID) - if err != nil { - log.Errorf("Unable to fetch the routing "+ - "policies for the edges of the channel "+ - "%v: %v", chanPoint, err) - continue - } - - // Now, we'll need to determine which is the correct - // policy for HTLCs being sent from the remote node. - var remotePolicy *channeldb.ChannelEdgePolicy - if bytes.Equal(remotePub[:], info.NodeKey1Bytes[:]) { - remotePolicy = p1 - } else { - remotePolicy = p2 - } - - // If for some reason we don't yet have the edge for - // the remote party, then we'll just skip adding this - // channel as a routing hint. - if remotePolicy == nil { - continue - } - - // Finally, create the routing hint for this channel and - // add it to our list of route hints. - hint := zpay32.HopHint{ - NodeID: channel.IdentityPub, - ChannelID: chanID, - FeeBaseMSat: uint32(remotePolicy.FeeBaseMSat), - FeeProportionalMillionths: uint32( - remotePolicy.FeeProportionalMillionths, - ), - CLTVExpiryDelta: remotePolicy.TimeLockDelta, - } - - // Include the route hint in our set of options that - // will be used when creating the invoice. - routeHint := []zpay32.HopHint{hint} - options = append(options, zpay32.RouteHint(routeHint)) - - numHints++ - } + // We'll restrict the number of individual route hints + // to 20 to avoid creating overly large invoices. + const numMaxHophints = 20 + hopHints := selectHopHints( + amtMSat, cfg, openChannels, numMaxHophints, + ) + options = append(options, hopHints...) } // Set our desired invoice features and add them to our list of options. @@ -423,3 +324,119 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, return &paymentHash, newInvoice, nil } + +// selectHopHints will selects up to numMaxHophints from the set of passed open +// channels. The set of hop hints will be returned as a slice of functional +// options that'll append the route hint to the set of all route hints. +func selectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, + openChannels []*channeldb.OpenChannel, + numMaxHophints int) []func(*zpay32.Invoice) { + + graph := cfg.ChanDB.ChannelGraph() + + numHints := 0 + hopHints := make([]func(*zpay32.Invoice), 0, numMaxHophints) + for _, channel := range openChannels { + if numHints >= numMaxHophints { + break + } + + // Since we're only interested in our private channels, we'll + // skip public ones. + isPublic := channel.ChannelFlags&lnwire.FFAnnounceChannel != 0 + if isPublic { + continue + } + + // Make sure the counterparty has enough balance in the channel + // for our amount. We do this in order to reduce payment errors + // when attempting to use this channel as a hint. + chanPoint := lnwire.NewChanIDFromOutPoint( + &channel.FundingOutpoint, + ) + if amtMSat >= channel.LocalCommitment.RemoteBalance { + log.Debugf("Skipping channel %v due to "+ + "not having enough remote balance", + chanPoint) + continue + } + + // Make sure the channel is active. + if !cfg.IsChannelActive(chanPoint) { + log.Debugf("Skipping channel %v due to not "+ + "being eligible to forward payments", + chanPoint) + continue + } + + // To ensure we don't leak unadvertised nodes, we'll make sure + // our counterparty is publicly advertised within the network. + // Otherwise, we'll end up leaking information about nodes that + // intend to stay unadvertised, like in the case of a node only + // having private channels. + var remotePub [33]byte + copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) + isRemoteNodePublic, err := graph.IsPublicNode(remotePub) + if err != nil { + log.Errorf("Unable to determine if node %x "+ + "is advertised: %v", remotePub, err) + continue + } + + if !isRemoteNodePublic { + log.Debugf("Skipping channel %v due to "+ + "counterparty %x being unadvertised", + chanPoint, remotePub) + continue + } + + // Fetch the policies for each end of the channel. + chanID := channel.ShortChanID().ToUint64() + info, p1, p2, err := graph.FetchChannelEdgesByID(chanID) + if err != nil { + log.Errorf("Unable to fetch the routing "+ + "policies for the edges of the channel "+ + "%v: %v", chanPoint, err) + continue + } + + // Now, we'll need to determine which is the correct policy for + // HTLCs being sent from the remote node. + var remotePolicy *channeldb.ChannelEdgePolicy + if bytes.Equal(remotePub[:], info.NodeKey1Bytes[:]) { + remotePolicy = p1 + } else { + remotePolicy = p2 + } + + // If for some reason we don't yet have the edge for the remote + // party, then we'll just skip adding this channel as a routing + // hint. + if remotePolicy == nil { + continue + } + + // Finally, create the routing hint for this channel and add it + // to our list of route hints. + hint := zpay32.HopHint{ + NodeID: channel.IdentityPub, + ChannelID: chanID, + FeeBaseMSat: uint32(remotePolicy.FeeBaseMSat), + FeeProportionalMillionths: uint32( + remotePolicy.FeeProportionalMillionths, + ), + CLTVExpiryDelta: remotePolicy.TimeLockDelta, + } + + // Include the route hint in our set of options that will be + // used when creating the invoice. + hopHints = append( + hopHints, + zpay32.RouteHint([]zpay32.HopHint{hint}), + ) + + numHints++ + } + + return hopHints +} From ab055efa56b786230a37d2c03ead7de90b8448d4 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 11 Aug 2020 17:38:26 -0700 Subject: [PATCH 2/5] lnrpc/invoicesrpc: move hop hint eligibility for chan to new method --- lnrpc/invoicesrpc/addinvoice.go | 138 +++++++++++++++++--------------- 1 file changed, 74 insertions(+), 64 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index 909195e10..0e25baef3 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -325,6 +325,73 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, return &paymentHash, newInvoice, nil } +// chanCanBeHopHint returns true if the target channel is eligible to be a hop +// hint. +func chanCanBeHopHint(channel *channeldb.OpenChannel, + graph *channeldb.ChannelGraph, + cfg *AddInvoiceConfig) (*channeldb.ChannelEdgePolicy, bool) { + + // Since we're only interested in our private channels, we'll skip + // public ones. + isPublic := channel.ChannelFlags&lnwire.FFAnnounceChannel != 0 + if isPublic { + return nil, false + } + + // Make sure the channel is active. + chanPoint := lnwire.NewChanIDFromOutPoint( + &channel.FundingOutpoint, + ) + if !cfg.IsChannelActive(chanPoint) { + log.Debugf("Skipping channel %v due to not "+ + "being eligible to forward payments", + chanPoint) + return nil, false + } + + // To ensure we don't leak unadvertised nodes, we'll make sure our + // counterparty is publicly advertised within the network. Otherwise, + // we'll end up leaking information about nodes that intend to stay + // unadvertised, like in the case of a node only having private + // channels. + var remotePub [33]byte + copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) + isRemoteNodePublic, err := graph.IsPublicNode(remotePub) + if err != nil { + log.Errorf("Unable to determine if node %x "+ + "is advertised: %v", remotePub, err) + return nil, false + } + + if !isRemoteNodePublic { + log.Debugf("Skipping channel %v due to "+ + "counterparty %x being unadvertised", + chanPoint, remotePub) + return nil, false + } + + // Fetch the policies for each end of the channel. + chanID := channel.ShortChanID().ToUint64() + info, p1, p2, err := graph.FetchChannelEdgesByID(chanID) + if err != nil { + log.Errorf("Unable to fetch the routing "+ + "policies for the edges of the channel "+ + "%v: %v", chanPoint, err) + return nil, false + } + + // Now, we'll need to determine which is the correct policy for HTLCs + // being sent from the remote node. + var remotePolicy *channeldb.ChannelEdgePolicy + if bytes.Equal(remotePub[:], info.NodeKey1Bytes[:]) { + remotePolicy = p1 + } else { + remotePolicy = p2 + } + + return remotePolicy, true +} + // selectHopHints will selects up to numMaxHophints from the set of passed open // channels. The set of hop hints will be returned as a slice of functional // options that'll append the route hint to the set of all route hints. @@ -341,74 +408,16 @@ func selectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, break } - // Since we're only interested in our private channels, we'll - // skip public ones. - isPublic := channel.ChannelFlags&lnwire.FFAnnounceChannel != 0 - if isPublic { - continue - } - - // Make sure the counterparty has enough balance in the channel - // for our amount. We do this in order to reduce payment errors - // when attempting to use this channel as a hint. - chanPoint := lnwire.NewChanIDFromOutPoint( - &channel.FundingOutpoint, + // If the channel can'tbe a hop hint, then we'll skip it. + // Otherwise, we'll use the policy information to populate the + // hop hint. + remotePolicy, canBeHopHint := chanCanBeHopHint( + channel, graph, cfg, ) - if amtMSat >= channel.LocalCommitment.RemoteBalance { - log.Debugf("Skipping channel %v due to "+ - "not having enough remote balance", - chanPoint) + if !canBeHopHint { continue } - // Make sure the channel is active. - if !cfg.IsChannelActive(chanPoint) { - log.Debugf("Skipping channel %v due to not "+ - "being eligible to forward payments", - chanPoint) - continue - } - - // To ensure we don't leak unadvertised nodes, we'll make sure - // our counterparty is publicly advertised within the network. - // Otherwise, we'll end up leaking information about nodes that - // intend to stay unadvertised, like in the case of a node only - // having private channels. - var remotePub [33]byte - copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) - isRemoteNodePublic, err := graph.IsPublicNode(remotePub) - if err != nil { - log.Errorf("Unable to determine if node %x "+ - "is advertised: %v", remotePub, err) - continue - } - - if !isRemoteNodePublic { - log.Debugf("Skipping channel %v due to "+ - "counterparty %x being unadvertised", - chanPoint, remotePub) - continue - } - - // Fetch the policies for each end of the channel. - chanID := channel.ShortChanID().ToUint64() - info, p1, p2, err := graph.FetchChannelEdgesByID(chanID) - if err != nil { - log.Errorf("Unable to fetch the routing "+ - "policies for the edges of the channel "+ - "%v: %v", chanPoint, err) - continue - } - - // Now, we'll need to determine which is the correct policy for - // HTLCs being sent from the remote node. - var remotePolicy *channeldb.ChannelEdgePolicy - if bytes.Equal(remotePub[:], info.NodeKey1Bytes[:]) { - remotePolicy = p1 - } else { - remotePolicy = p2 - } - // If for some reason we don't yet have the edge for the remote // party, then we'll just skip adding this channel as a routing // hint. @@ -418,6 +427,7 @@ func selectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, // Finally, create the routing hint for this channel and add it // to our list of route hints. + chanID := channel.ShortChanID().ToUint64() hint := zpay32.HopHint{ NodeID: channel.IdentityPub, ChannelID: chanID, From f42d7780f1e38eb41593a7752e122d8410fbf66c Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Wed, 12 Aug 2020 17:43:55 -0700 Subject: [PATCH 3/5] lnrpc/invoicesrpc: refactor out hop hint accumulation into a new function --- lnrpc/invoicesrpc/addinvoice.go | 48 +++++++++++++++------------------ 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index 0e25baef3..eeb102c68 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -392,6 +392,25 @@ func chanCanBeHopHint(channel *channeldb.OpenChannel, return remotePolicy, true } +// addHopHint creates a hop hint out of the passed channel and channel policy. +// The new hop hint is appended to the passed slice. +func addHopHint(hopHints []func(*zpay32.Invoice), + channel *channeldb.OpenChannel, chanPolicy *channeldb.ChannelEdgePolicy) { + + hopHint := zpay32.HopHint{ + NodeID: channel.IdentityPub, + ChannelID: channel.ShortChanID().ToUint64(), + FeeBaseMSat: uint32(chanPolicy.FeeBaseMSat), + FeeProportionalMillionths: uint32( + chanPolicy.FeeProportionalMillionths, + ), + CLTVExpiryDelta: chanPolicy.TimeLockDelta, + } + hopHints = append( + hopHints, zpay32.RouteHint([]zpay32.HopHint{hopHint}), + ) +} + // selectHopHints will selects up to numMaxHophints from the set of passed open // channels. The set of hop hints will be returned as a slice of functional // options that'll append the route hint to the set of all route hints. @@ -408,42 +427,19 @@ func selectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, break } - // If the channel can'tbe a hop hint, then we'll skip it. + // If the channel can't a hop hint, then we'll skip it. // Otherwise, we'll use the policy information to populate the // hop hint. remotePolicy, canBeHopHint := chanCanBeHopHint( channel, graph, cfg, ) - if !canBeHopHint { + if !canBeHopHint || remotePolicy == nil { continue } - // If for some reason we don't yet have the edge for the remote - // party, then we'll just skip adding this channel as a routing - // hint. - if remotePolicy == nil { - continue - } - - // Finally, create the routing hint for this channel and add it - // to our list of route hints. - chanID := channel.ShortChanID().ToUint64() - hint := zpay32.HopHint{ - NodeID: channel.IdentityPub, - ChannelID: chanID, - FeeBaseMSat: uint32(remotePolicy.FeeBaseMSat), - FeeProportionalMillionths: uint32( - remotePolicy.FeeProportionalMillionths, - ), - CLTVExpiryDelta: remotePolicy.TimeLockDelta, - } - // Include the route hint in our set of options that will be // used when creating the invoice. - hopHints = append( - hopHints, - zpay32.RouteHint([]zpay32.HopHint{hint}), - ) + addHopHint(hopHints, channel, remotePolicy) numHints++ } From f6d6d6609f456b7fe9328500bdec7577174bd24a Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Mon, 10 Aug 2020 16:18:48 -0700 Subject: [PATCH 4/5] lnrpc/invoicesrpc: extend hop hint selection to account for MPP In this commit, we update the hop hint selection to account for the fact that with MPP, a single payment may consume multiple channels. As is, if a user only has two 0.5 BTC channels, and tries to make a 1 BTC channel, then the current logic won't include any hop hints. To solve this, we first add all the channels which in isolation can carry the payment in question. We then do another pass that accumulates channels until either we reach our hop-hint limit, or the total bandwidth that we've accumulate is greater than 2x the payment amount. --- lnrpc/invoicesrpc/addinvoice.go | 75 +++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 9 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index eeb102c68..d7172a5bb 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -10,6 +10,7 @@ import ( "time" "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/davecgh/go-spew/spew" @@ -394,7 +395,7 @@ func chanCanBeHopHint(channel *channeldb.OpenChannel, // addHopHint creates a hop hint out of the passed channel and channel policy. // The new hop hint is appended to the passed slice. -func addHopHint(hopHints []func(*zpay32.Invoice), +func addHopHint(hopHints *[]func(*zpay32.Invoice), channel *channeldb.OpenChannel, chanPolicy *channeldb.ChannelEdgePolicy) { hopHint := zpay32.HopHint{ @@ -406,28 +407,80 @@ func addHopHint(hopHints []func(*zpay32.Invoice), ), CLTVExpiryDelta: chanPolicy.TimeLockDelta, } - hopHints = append( - hopHints, zpay32.RouteHint([]zpay32.HopHint{hopHint}), + *hopHints = append( + *hopHints, zpay32.RouteHint([]zpay32.HopHint{hopHint}), ) } -// selectHopHints will selects up to numMaxHophints from the set of passed open +// selectHopHints will select up to numMaxHophints from the set of passed open // channels. The set of hop hints will be returned as a slice of functional // options that'll append the route hint to the set of all route hints. +// +// TODO(roasbeef): do proper sub-set sum max hints usually << numChans func selectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, openChannels []*channeldb.OpenChannel, numMaxHophints int) []func(*zpay32.Invoice) { graph := cfg.ChanDB.ChannelGraph() - numHints := 0 + // We'll add our hop hints in two passes, first we'll add all channels + // that are eligible to be hop hints, and also have a local balance + // above the payment amount. + var totalHintBandwidth lnwire.MilliSatoshi + hopHintChans := make(map[wire.OutPoint]struct{}) hopHints := make([]func(*zpay32.Invoice), 0, numMaxHophints) for _, channel := range openChannels { - if numHints >= numMaxHophints { + // If this channel can't be a hop hint, then skip it. + edgePolicy, canBeHopHint := chanCanBeHopHint( + channel, graph, cfg, + ) + if edgePolicy == nil || !canBeHopHint { + continue + } + + // Similarly, in this first pass, we'll ignore all channels in + // isolation can't satisfy this payment. + if channel.LocalCommitment.RemoteBalance < amtMSat { + continue + } + + // Now that we now this channel use usable, add it as a hop + // hint and the indexes we'll use later. + addHopHint(&hopHints, channel, edgePolicy) + + hopHintChans[channel.FundingOutpoint] = struct{}{} + totalHintBandwidth += channel.LocalCommitment.RemoteBalance + } + + // If we have enough hop hints at this point, then we'll exit early. + // Otherwise, we'll continue to add more that may help out mpp users. + if len(hopHints) >= numMaxHophints { + return hopHints + } + + // In this second pass we'll add channels, and we'll either stop when + // we have 20 hop hints, we've run through all the available channels, + // or if the sum of available bandwidth in the routing hints exceeds 2x + // the payment amount. We do 2x here to account for a margin of error + // if some of the selected channels no longer become operable. + hopHintFactor := lnwire.MilliSatoshi(2) + for i := 0; i < len(openChannels); i++ { + // If we hit either of our early termination conditions, then + // we'll break the loop here. + if totalHintBandwidth > amtMSat*hopHintFactor || + len(hopHints) >= numMaxHophints { + break } - // If the channel can't a hop hint, then we'll skip it. + channel := openChannels[i] + + // Skip the channel if we already selected it. + if _, ok := hopHintChans[channel.FundingOutpoint]; ok { + continue + } + + // If the channel can't be a hop hint, then we'll skip it. // Otherwise, we'll use the policy information to populate the // hop hint. remotePolicy, canBeHopHint := chanCanBeHopHint( @@ -439,9 +492,13 @@ func selectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, // Include the route hint in our set of options that will be // used when creating the invoice. - addHopHint(hopHints, channel, remotePolicy) + addHopHint(&hopHints, channel, remotePolicy) - numHints++ + // As we've just added a new hop hint, we'll accumulate it's + // available balance now to update our tally. + // + // TODO(roasbeef): have a cut off based on min bandwidth? + totalHintBandwidth += channel.LocalCommitment.RemoteBalance } return hopHints From 174faa795587a6bc614af68b71cca1688a0f22c7 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Tue, 11 Aug 2020 17:48:14 -0700 Subject: [PATCH 5/5] lnrpc/invoicesrpc: only try to add hop hints if we have channels --- lnrpc/invoicesrpc/addinvoice.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index d7172a5bb..afa7e7084 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -255,14 +255,16 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, return nil, nil, fmt.Errorf("could not fetch all channels") } - // We'll restrict the number of individual route hints - // to 20 to avoid creating overly large invoices. - const numMaxHophints = 20 - hopHints := selectHopHints( - amtMSat, cfg, openChannels, numMaxHophints, - ) + if len(openChannels) > 0 { + // We'll restrict the number of individual route hints + // to 20 to avoid creating overly large invoices. + const numMaxHophints = 20 + hopHints := selectHopHints( + amtMSat, cfg, openChannels, numMaxHophints, + ) - options = append(options, hopHints...) + options = append(options, hopHints...) + } } // Set our desired invoice features and add them to our list of options.