From 56e83de9eff7f1a21a2b764a00759d6b2e5e3c18 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 27 Jan 2022 15:09:18 +0200 Subject: [PATCH 1/5] invoicesrpc: add HopHintInfo decoupling hop hint from channel internals --- lnrpc/invoicesrpc/addinvoice.go | 88 ++++++++++++++++++++++++--------- 1 file changed, 66 insertions(+), 22 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index fda49f21c..27671e5ec 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -9,6 +9,7 @@ import ( "math" "time" + "github.com/btcsuite/btcd/btcec" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" @@ -379,12 +380,21 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, if len(openChannels) > 0 { // We filter the channels by excluding the ones that were specified by // the caller and were already added. - var filteredChannels []*channeldb.OpenChannel + var filteredChannels []*HopHintInfo for _, c := range openChannels { if _, ok := forcedHints[c.ShortChanID().ToUint64()]; ok { continue } - filteredChannels = append(filteredChannels, c) + + chanID := lnwire.NewChanIDFromOutPoint( + &c.FundingOutpoint, + ) + isActive := cfg.IsChannelActive(chanID) + + hopHintInfo := newHopHintInfo(c, isActive) + filteredChannels = append( + filteredChannels, hopHintInfo, + ) } // We'll restrict the number of individual route hints @@ -466,24 +476,20 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, // chanCanBeHopHint returns true if the target channel is eligible to be a hop // hint. -func chanCanBeHopHint(channel *channeldb.OpenChannel, cfg *AddInvoiceConfig) ( +func chanCanBeHopHint(channel *HopHintInfo, 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 { + if channel.IsPublic { return nil, false } // Make sure the channel is active. - chanPoint := lnwire.NewChanIDFromOutPoint( - &channel.FundingOutpoint, - ) - if !cfg.IsChannelActive(chanPoint) { + if !channel.IsActive { log.Debugf("Skipping channel %v due to not "+ "being eligible to forward payments", - chanPoint) + channel.ShortChannelID) return nil, false } @@ -493,7 +499,7 @@ func chanCanBeHopHint(channel *channeldb.OpenChannel, cfg *AddInvoiceConfig) ( // unadvertised, like in the case of a node only having private // channels. var remotePub [33]byte - copy(remotePub[:], channel.IdentityPub.SerializeCompressed()) + copy(remotePub[:], channel.RemotePubkey.SerializeCompressed()) isRemoteNodePublic, err := cfg.Graph.IsPublicNode(remotePub) if err != nil { log.Errorf("Unable to determine if node %x "+ @@ -504,17 +510,18 @@ func chanCanBeHopHint(channel *channeldb.OpenChannel, cfg *AddInvoiceConfig) ( if !isRemoteNodePublic { log.Debugf("Skipping channel %v due to "+ "counterparty %x being unadvertised", - chanPoint, remotePub) + channel.ShortChannelID, remotePub) return nil, false } // Fetch the policies for each end of the channel. - chanID := channel.ShortChanID().ToUint64() - info, p1, p2, err := cfg.Graph.FetchChannelEdgesByID(chanID) + info, p1, p2, err := cfg.Graph.FetchChannelEdgesByID( + channel.ShortChannelID, + ) if err != nil { log.Errorf("Unable to fetch the routing "+ "policies for the edges of the channel "+ - "%v: %v", chanPoint, err) + "%v: %v", channel.ShortChannelID, err) return nil, false } @@ -533,11 +540,11 @@ func chanCanBeHopHint(channel *channeldb.OpenChannel, cfg *AddInvoiceConfig) ( // 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) { + channel *HopHintInfo, chanPolicy *channeldb.ChannelEdgePolicy) { hopHint := zpay32.HopHint{ - NodeID: channel.IdentityPub, - ChannelID: channel.ShortChanID().ToUint64(), + NodeID: channel.RemotePubkey, + ChannelID: channel.ShortChannelID, FeeBaseMSat: uint32(chanPolicy.FeeBaseMSat), FeeProportionalMillionths: uint32( chanPolicy.FeeProportionalMillionths, @@ -549,13 +556,50 @@ func addHopHint(hopHints *[]func(*zpay32.Invoice), ) } +// HopHintInfo contains the channel information required to create a hop hint. +type HopHintInfo struct { + // IsPublic indicates whether a channel is advertised to the network. + IsPublic bool + + // IsActive indicates whether the channel is online and available for + // use. + IsActive bool + + // FundingOutpoint is the funding txid:index for the channel. + FundingOutpoint wire.OutPoint + + // RemotePubkey is the public key of the remote party that this channel + // is in. + RemotePubkey *btcec.PublicKey + + // RemoteBalance is the remote party's balance (our current incoming + // capacity). + RemoteBalance lnwire.MilliSatoshi + + // ShortChannelID is the short channel ID of the channel. + ShortChannelID uint64 +} + +func newHopHintInfo(c *channeldb.OpenChannel, isActive bool) *HopHintInfo { + isPublic := c.ChannelFlags&lnwire.FFAnnounceChannel != 0 + + return &HopHintInfo{ + IsPublic: isPublic, + IsActive: isActive, + FundingOutpoint: c.FundingOutpoint, + RemotePubkey: c.IdentityPub, + RemoteBalance: c.LocalCommitment.RemoteBalance, + ShortChannelID: c.ShortChannelID.ToUint64(), + } +} + // 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, + openChannels []*HopHintInfo, numMaxHophints int) []func(*zpay32.Invoice) { // We'll add our hop hints in two passes, first we'll add all channels @@ -573,7 +617,7 @@ func SelectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, // Similarly, in this first pass, we'll ignore all channels in // isolation can't satisfy this payment. - if channel.LocalCommitment.RemoteBalance < amtMSat { + if channel.RemoteBalance < amtMSat { continue } @@ -582,7 +626,7 @@ func SelectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, addHopHint(&hopHints, channel, edgePolicy) hopHintChans[channel.FundingOutpoint] = struct{}{} - totalHintBandwidth += channel.LocalCommitment.RemoteBalance + totalHintBandwidth += channel.RemoteBalance } // If we have enough hop hints at this point, then we'll exit early. @@ -629,7 +673,7 @@ func SelectHopHints(amtMSat lnwire.MilliSatoshi, cfg *AddInvoiceConfig, // available balance now to update our tally. // // TODO(roasbeef): have a cut off based on min bandwidth? - totalHintBandwidth += channel.LocalCommitment.RemoteBalance + totalHintBandwidth += channel.RemoteBalance } return hopHints From 00a47740c8fa67d6ea4188631bcdfa1f99c3b7a0 Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 27 Jan 2022 15:09:19 +0200 Subject: [PATCH 2/5] invoicesrpc: pass in specialized config for select hop hints --- lnrpc/invoicesrpc/addinvoice.go | 37 ++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index 27671e5ec..a2c7ffd07 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -400,8 +400,11 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, // We'll restrict the number of individual route hints // to 20 to avoid creating overly large invoices. numMaxHophints := 20 - len(forcedHints) + + hopHintsCfg := newSelectHopHintsCfg(cfg) hopHints := SelectHopHints( - amtMSat, cfg, filteredChannels, numMaxHophints, + amtMSat, hopHintsCfg, filteredChannels, + numMaxHophints, ) options = append(options, hopHints...) @@ -476,7 +479,7 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, // chanCanBeHopHint returns true if the target channel is eligible to be a hop // hint. -func chanCanBeHopHint(channel *HopHintInfo, cfg *AddInvoiceConfig) ( +func chanCanBeHopHint(channel *HopHintInfo, cfg *SelectHopHintsCfg) ( *channeldb.ChannelEdgePolicy, bool) { // Since we're only interested in our private channels, we'll skip @@ -500,7 +503,7 @@ func chanCanBeHopHint(channel *HopHintInfo, cfg *AddInvoiceConfig) ( // channels. var remotePub [33]byte copy(remotePub[:], channel.RemotePubkey.SerializeCompressed()) - isRemoteNodePublic, err := cfg.Graph.IsPublicNode(remotePub) + isRemoteNodePublic, err := cfg.IsPublicNode(remotePub) if err != nil { log.Errorf("Unable to determine if node %x "+ "is advertised: %v", remotePub, err) @@ -515,9 +518,7 @@ func chanCanBeHopHint(channel *HopHintInfo, cfg *AddInvoiceConfig) ( } // Fetch the policies for each end of the channel. - info, p1, p2, err := cfg.Graph.FetchChannelEdgesByID( - channel.ShortChannelID, - ) + info, p1, p2, err := cfg.FetchChannelEdgesByID(channel.ShortChannelID) if err != nil { log.Errorf("Unable to fetch the routing "+ "policies for the edges of the channel "+ @@ -593,12 +594,34 @@ func newHopHintInfo(c *channeldb.OpenChannel, isActive bool) *HopHintInfo { } } +// SelectHopHintsCfg contains the dependencies required to obtain hop hints +// for an invoice. +type SelectHopHintsCfg struct { + // IsPublicNode is returns a bool indicating whether the node with the + // given public key is seen as a public node in the graph from the + // graph's source node's point of view. + IsPublicNode func(pubKey [33]byte) (bool, error) + + // FetchChannelEdgesByID attempts to lookup the two directed edges for + // the channel identified by the channel ID. + FetchChannelEdgesByID func(chanID uint64) (*channeldb.ChannelEdgeInfo, + *channeldb.ChannelEdgePolicy, *channeldb.ChannelEdgePolicy, + error) +} + +func newSelectHopHintsCfg(invoicesCfg *AddInvoiceConfig) *SelectHopHintsCfg { + return &SelectHopHintsCfg{ + IsPublicNode: invoicesCfg.Graph.IsPublicNode, + FetchChannelEdgesByID: invoicesCfg.Graph.FetchChannelEdgesByID, + } +} + // 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, +func SelectHopHints(amtMSat lnwire.MilliSatoshi, cfg *SelectHopHintsCfg, openChannels []*HopHintInfo, numMaxHophints int) []func(*zpay32.Invoice) { From 80bf4bf0143ef62e70e5096e5d4f2e092e33447f Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 27 Jan 2022 15:09:20 +0200 Subject: [PATCH 3/5] invoicerpc: return hop hints from select hop hint rather than functions Update SelectHopHints to return a set of hop hints that can be converted to route hints / functional options by the caller. This change allows external code to call SelectHopHints to get a set of route hints and use them as it likes, rather than limiting the return value to functional options for invoice creation. --- lnrpc/invoicesrpc/addinvoice.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index a2c7ffd07..b9a4712f3 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -407,7 +407,15 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig, numMaxHophints, ) - options = append(options, hopHints...) + // Convert our set of selected hop hints into route + // hints and add to our invoice options. + for _, hopHint := range hopHints { + routeHint := zpay32.RouteHint(hopHint) + + options = append( + options, routeHint, + ) + } } } @@ -540,7 +548,7 @@ func chanCanBeHopHint(channel *HopHintInfo, cfg *SelectHopHintsCfg) ( // 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 *[][]zpay32.HopHint, channel *HopHintInfo, chanPolicy *channeldb.ChannelEdgePolicy) { hopHint := zpay32.HopHint{ @@ -552,9 +560,8 @@ func addHopHint(hopHints *[]func(*zpay32.Invoice), ), CLTVExpiryDelta: chanPolicy.TimeLockDelta, } - *hopHints = append( - *hopHints, zpay32.RouteHint([]zpay32.HopHint{hopHint}), - ) + + *hopHints = append(*hopHints, []zpay32.HopHint{hopHint}) } // HopHintInfo contains the channel information required to create a hop hint. @@ -623,14 +630,14 @@ func newSelectHopHintsCfg(invoicesCfg *AddInvoiceConfig) *SelectHopHintsCfg { // TODO(roasbeef): do proper sub-set sum max hints usually << numChans func SelectHopHints(amtMSat lnwire.MilliSatoshi, cfg *SelectHopHintsCfg, openChannels []*HopHintInfo, - numMaxHophints int) []func(*zpay32.Invoice) { + numMaxHophints int) [][]zpay32.HopHint { // 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) + hopHints := make([][]zpay32.HopHint, 0, numMaxHophints) for _, channel := range openChannels { // If this channel can't be a hop hint, then skip it. edgePolicy, canBeHopHint := chanCanBeHopHint(channel, cfg) From 714a1fb05c2c32912c8732ebf55b83761244284a Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 27 Jan 2022 15:09:21 +0200 Subject: [PATCH 4/5] invoicerpc: move hop hint to constant and add tests to select hop hint --- lnrpc/invoicesrpc/addinvoice.go | 6 +- lnrpc/invoicesrpc/addinvoice_test.go | 549 +++++++++++++++++++++++++++ 2 files changed, 554 insertions(+), 1 deletion(-) create mode 100644 lnrpc/invoicesrpc/addinvoice_test.go diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index b9a4712f3..17f58b9cc 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/lnrpc/invoicesrpc/addinvoice.go @@ -31,6 +31,11 @@ const ( // DefaultAMPInvoiceExpiry is the default invoice expiry for new AMP // invoices. DefaultAMPInvoiceExpiry = 30 * 24 * time.Hour + + // hopHintFactor is factor by which we scale the total amount of + // inbound capacity we want our hop hints to represent, allowing us to + // have some leeway if peers go offline. + hopHintFactor = 2 ) // AddInvoiceConfig contains dependencies for invoice creation. @@ -670,7 +675,6 @@ func SelectHopHints(amtMSat lnwire.MilliSatoshi, cfg *SelectHopHintsCfg, // 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. diff --git a/lnrpc/invoicesrpc/addinvoice_test.go b/lnrpc/invoicesrpc/addinvoice_test.go new file mode 100644 index 000000000..fd2aa60b7 --- /dev/null +++ b/lnrpc/invoicesrpc/addinvoice_test.go @@ -0,0 +1,549 @@ +package invoicesrpc + +import ( + "encoding/hex" + "errors" + "math/big" + "testing" + + "github.com/btcsuite/btcd/btcec" + "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/lnwire" + "github.com/lightningnetwork/lnd/zpay32" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type hopHintsConfigMock struct { + mock.Mock +} + +// IsPublicNode mocks node public state lookup. +func (h *hopHintsConfigMock) IsPublicNode(pubKey [33]byte) (bool, error) { + args := h.Mock.Called(pubKey) + return args.Bool(0), args.Error(1) +} + +// FetchChannelEdgesByID mocks channel edge lookup. +func (h *hopHintsConfigMock) FetchChannelEdgesByID(chanID uint64) ( + *channeldb.ChannelEdgeInfo, *channeldb.ChannelEdgePolicy, + *channeldb.ChannelEdgePolicy, error) { + + args := h.Mock.Called(chanID) + + // If our error is non-nil, we expect nil responses otherwise. Our + // casts below will fail with nil values, so we check our error and + // return early on failure first. + err := args.Error(3) + if err != nil { + return nil, nil, nil, err + } + + edgeInfo := args.Get(0).(*channeldb.ChannelEdgeInfo) + policy1 := args.Get(1).(*channeldb.ChannelEdgePolicy) + policy2 := args.Get(2).(*channeldb.ChannelEdgePolicy) + + return edgeInfo, policy1, policy2, err +} + +// TestSelectHopHints tests selection of hop hints for a node with private +// channels. +func TestSelectHopHints(t *testing.T) { + var ( + // We need to serialize our pubkey in SelectHopHints so it + // needs to be valid. + pubkeyBytes, _ = hex.DecodeString( + "598ec453728e0ffe0ae2f5e174243cf58f2" + + "a3f2c83d2457b43036db568b11093", + ) + pubkey = &btcec.PublicKey{ + X: big.NewInt(4), + Y: new(big.Int).SetBytes(pubkeyBytes), + Curve: btcec.S256(), + } + compressed = pubkey.SerializeCompressed() + + publicChannel = &HopHintInfo{ + IsPublic: true, + IsActive: true, + FundingOutpoint: wire.OutPoint{ + Index: 0, + }, + RemoteBalance: 10, + ShortChannelID: 0, + } + + inactiveChannel = &HopHintInfo{ + IsPublic: false, + IsActive: false, + } + + // Create a private channel that we'll generate hints from. + private1ShortID uint64 = 1 + privateChannel1 = &HopHintInfo{ + IsPublic: false, + IsActive: true, + FundingOutpoint: wire.OutPoint{ + Index: 1, + }, + RemotePubkey: pubkey, + RemoteBalance: 100, + ShortChannelID: private1ShortID, + } + + // Create a edge policy for private channel 1. + privateChan1Policy = &channeldb.ChannelEdgePolicy{ + FeeBaseMSat: 10, + FeeProportionalMillionths: 100, + TimeLockDelta: 1000, + } + + // Create an edge policy different to ours which we'll use for + // the other direction + otherChanPolicy = &channeldb.ChannelEdgePolicy{ + FeeBaseMSat: 90, + FeeProportionalMillionths: 900, + TimeLockDelta: 9000, + } + + // Create a hop hint based on privateChan1Policy. + privateChannel1Hint = zpay32.HopHint{ + NodeID: privateChannel1.RemotePubkey, + ChannelID: private1ShortID, + FeeBaseMSat: uint32(privateChan1Policy.FeeBaseMSat), + FeeProportionalMillionths: uint32( + privateChan1Policy.FeeProportionalMillionths, + ), + CLTVExpiryDelta: privateChan1Policy.TimeLockDelta, + } + + // Create a second private channel that we'll use for hints. + private2ShortID uint64 = 2 + privateChannel2 = &HopHintInfo{ + IsPublic: false, + IsActive: true, + FundingOutpoint: wire.OutPoint{ + Index: 2, + }, + RemotePubkey: pubkey, + RemoteBalance: 100, + ShortChannelID: private2ShortID, + } + + // Create a edge policy for private channel 1. + privateChan2Policy = &channeldb.ChannelEdgePolicy{ + FeeBaseMSat: 20, + FeeProportionalMillionths: 200, + TimeLockDelta: 2000, + } + + // Create a hop hint based on privateChan2Policy. + privateChannel2Hint = zpay32.HopHint{ + NodeID: privateChannel2.RemotePubkey, + ChannelID: private2ShortID, + FeeBaseMSat: uint32(privateChan2Policy.FeeBaseMSat), + FeeProportionalMillionths: uint32( + privateChan2Policy.FeeProportionalMillionths, + ), + CLTVExpiryDelta: privateChan2Policy.TimeLockDelta, + } + + // Create a third private channel that we'll use for hints. + private3ShortID uint64 = 3 + privateChannel3 = &HopHintInfo{ + IsPublic: false, + IsActive: true, + FundingOutpoint: wire.OutPoint{ + Index: 3, + }, + RemotePubkey: pubkey, + RemoteBalance: 100, + ShortChannelID: private3ShortID, + } + + // Create a edge policy for private channel 1. + privateChan3Policy = &channeldb.ChannelEdgePolicy{ + FeeBaseMSat: 30, + FeeProportionalMillionths: 300, + TimeLockDelta: 3000, + } + + // Create a hop hint based on privateChan2Policy. + privateChannel3Hint = zpay32.HopHint{ + NodeID: privateChannel3.RemotePubkey, + ChannelID: private3ShortID, + FeeBaseMSat: uint32(privateChan3Policy.FeeBaseMSat), + FeeProportionalMillionths: uint32( + privateChan3Policy.FeeProportionalMillionths, + ), + CLTVExpiryDelta: privateChan3Policy.TimeLockDelta, + } + ) + + // We can't copy in the above var decls, so we copy in our pubkey here. + var peer [33]byte + copy(peer[:], compressed) + + var ( + // We pick our policy based on which node (1 or 2) the remote + // peer is. Here we create two different sets of edge + // information. One where our peer is node 1, the other where + // our peer is edge 2. This ensures that we always pick the + // right edge policy for our hint. + infoNode1 = &channeldb.ChannelEdgeInfo{ + NodeKey1Bytes: peer, + } + + infoNode2 = &channeldb.ChannelEdgeInfo{ + NodeKey1Bytes: [33]byte{9, 9, 9}, + NodeKey2Bytes: peer, + } + + // setMockChannelUsed preps our mock for the case where we + // want our private channel to be used for a hop hint. + setMockChannelUsed = func(h *hopHintsConfigMock, + shortID uint64, + policy *channeldb.ChannelEdgePolicy) { + + // Return public node = true so that we'll consider + // this node for our hop hints. + h.Mock.On( + "IsPublicNode", peer, + ).Once().Return(true, nil) + + // When it gets time to find an edge policy for this + // node, fail it. We won't use it as a hop hint. + h.Mock.On( + "FetchChannelEdgesByID", + shortID, + ).Once().Return( + infoNode1, policy, otherChanPolicy, nil, + ) + } + ) + + tests := []struct { + name string + setupMock func(*hopHintsConfigMock) + amount lnwire.MilliSatoshi + channels []*HopHintInfo + numHints int + + // expectedHints is the set of hop hints that we expect. We + // initialize this slice with our max hop hints length, so this + // value won't be nil even if its empty. + expectedHints [][]zpay32.HopHint + }{ + { + // We don't need hop hints for public channels. + name: "channel is public", + // When a channel is public, we exit before we make any + // calls. + setupMock: func(h *hopHintsConfigMock) { + }, + amount: 100, + channels: []*HopHintInfo{ + publicChannel, + }, + numHints: 2, + expectedHints: nil, + }, + { + name: "channel is inactive", + setupMock: func(h *hopHintsConfigMock) {}, + amount: 100, + channels: []*HopHintInfo{ + inactiveChannel, + }, + numHints: 2, + expectedHints: nil, + }, + { + // If we can't lookup an edge policy, we skip channels. + name: "no edge policy", + setupMock: func(h *hopHintsConfigMock) { + // Return public node = true so that we'll + // consider this node for our hop hints. + h.Mock.On( + "IsPublicNode", peer, + ).Return(true, nil) + + // When it gets time to find an edge policy for + // this node, fail it. We won't use it as a + // hop hint. + h.Mock.On( + "FetchChannelEdgesByID", + private1ShortID, + ).Return( + nil, nil, nil, + errors.New("no edge"), + ) + }, + amount: 100, + channels: []*HopHintInfo{ + privateChannel1, + }, + numHints: 3, + expectedHints: nil, + }, + { + // If one of our private channels belongs to a node + // that is otherwise not announced to the network, we're + // polite and don't include them (they can't be routed + // through anyway). + name: "node is private", + setupMock: func(h *hopHintsConfigMock) { + // Return public node = false so that we'll + // give up on this node. + h.Mock.On( + "IsPublicNode", peer, + ).Return(false, nil) + }, + amount: 100, + channels: []*HopHintInfo{ + privateChannel1, + }, + numHints: 1, + expectedHints: nil, + }, + { + // If a channel has more balance than the amount we're + // looking for, it'll be added in our first pass. We + // can be sure we're adding it in our first pass because + // we assert that there are no additional calls to our + // mock (which would happen if we ran a second pass). + // + // We set our peer to be node 1 in our policy ordering. + name: "balance > total amount, node 1", + setupMock: func(h *hopHintsConfigMock) { + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + }, + // Our channel has balance of 100 (> 50). + amount: 50, + channels: []*HopHintInfo{ + privateChannel1, + }, + numHints: 2, + expectedHints: [][]zpay32.HopHint{ + { + privateChannel1Hint, + }, + }, + }, + { + // As above, but we set our peer to be node 2 in our + // policy ordering. + name: "balance > total amount, node 2", + setupMock: func(h *hopHintsConfigMock) { + // Return public node = true so that we'll + // consider this node for our hop hints. + h.Mock.On( + "IsPublicNode", peer, + ).Return(true, nil) + + // When it gets time to find an edge policy for + // this node, fail it. We won't use it as a + // hop hint. + h.Mock.On( + "FetchChannelEdgesByID", + private1ShortID, + ).Return( + infoNode2, otherChanPolicy, + privateChan1Policy, nil, + ) + }, + // Our channel has balance of 100 (> 50). + amount: 50, + channels: []*HopHintInfo{ + privateChannel1, + }, + numHints: 2, + expectedHints: [][]zpay32.HopHint{ + { + privateChannel1Hint, + }, + }, + }, + { + // Since our balance is less than the amount we're + // looking to route, we expect this hint to be picked + // up in our second pass on the channel set. + name: "balance < total amount", + setupMock: func(h *hopHintsConfigMock) { + // We expect to call all our checks twice + // because we pick up this channel in the + // second round. + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + }, + // Our channel has balance of 100 (< 150). + amount: 150, + channels: []*HopHintInfo{ + privateChannel1, + }, + numHints: 2, + expectedHints: [][]zpay32.HopHint{ + { + privateChannel1Hint, + }, + }, + }, + { + // Test the case where we hit our total amount of + // required liquidity in our first pass. + name: "first pass sufficient balance", + setupMock: func(h *hopHintsConfigMock) { + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + }, + // Divide our balance by hop hint factor so that the + // channel balance will always reach our factored up + // amount, even if we change this value. + amount: privateChannel1.RemoteBalance / hopHintFactor, + channels: []*HopHintInfo{ + privateChannel1, + }, + numHints: 2, + expectedHints: [][]zpay32.HopHint{ + { + privateChannel1Hint, + }, + }, + }, + { + // Setup our amount so that we don't have enough + // inbound total for our amount, but we hit our + // desired hint limit. + name: "second pass sufficient hint count", + setupMock: func(h *hopHintsConfigMock) { + // We expect all of our channels to be passed + // on in the first pass. + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + + setMockChannelUsed( + h, private2ShortID, privateChan2Policy, + ) + + // In the second pass, our first two channels + // should be added before we hit our hint count. + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + + }, + // Add two channels that we'd want to use, but the + // second one will be cut off due to our hop hint count + // limit. + channels: []*HopHintInfo{ + privateChannel1, privateChannel2, + }, + // Set the amount we need to more than our two channels + // can provide us. + amount: privateChannel1.RemoteBalance + + privateChannel2.RemoteBalance, + numHints: 1, + expectedHints: [][]zpay32.HopHint{ + { + privateChannel1Hint, + }, + }, + }, + { + // Add three channels that are all less than the amount + // we wish to receive, but collectively will reach the + // total amount that we need. + name: "second pass reaches bandwidth requirement", + setupMock: func(h *hopHintsConfigMock) { + // In the first round, all channels should be + // passed on. + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + + setMockChannelUsed( + h, private2ShortID, privateChan2Policy, + ) + + setMockChannelUsed( + h, private3ShortID, privateChan3Policy, + ) + + // In the second round, we'll pick up all of + // our hop hints. + setMockChannelUsed( + h, private1ShortID, privateChan1Policy, + ) + + setMockChannelUsed( + h, private2ShortID, privateChan2Policy, + ) + + setMockChannelUsed( + h, private3ShortID, privateChan3Policy, + ) + }, + channels: []*HopHintInfo{ + privateChannel1, privateChannel2, + privateChannel3, + }, + + // All of our channels have 100 inbound, so none will + // be picked up in the first round. + amount: 110, + numHints: 5, + expectedHints: [][]zpay32.HopHint{ + { + privateChannel1Hint, + }, + { + privateChannel2Hint, + }, + { + privateChannel3Hint, + }, + }, + }, + } + + for _, test := range tests { + test := test + + t.Run(test.name, func(t *testing.T) { + // Create mock and prime it for the test case. + mock := &hopHintsConfigMock{} + test.setupMock(mock) + defer mock.AssertExpectations(t) + + cfg := &SelectHopHintsCfg{ + IsPublicNode: mock.IsPublicNode, + FetchChannelEdgesByID: mock.FetchChannelEdgesByID, + } + + hints := SelectHopHints( + test.amount, cfg, test.channels, test.numHints, + ) + + // SelectHopHints preallocates its hop hint slice, so + // we check that it is empty if we don't expect any + // hints, and otherwise assert that the two slices are + // equal. This allows tests to set their expected value + // to nil, rather than providing a preallocated empty + // slice. + if len(test.expectedHints) == 0 { + require.Zero(t, len(hints)) + } else { + require.Equal(t, test.expectedHints, hints) + } + }) + } +} From c15d010ba2033434638144cbc73e2a4b8e71c7ca Mon Sep 17 00:00:00 2001 From: carla Date: Thu, 27 Jan 2022 15:10:04 +0200 Subject: [PATCH 5/5] docs: add release notes --- docs/release-notes/release-notes-0.15.0.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/release-notes-0.15.0.md b/docs/release-notes/release-notes-0.15.0.md index 1b1dc2c60..d43ad51bc 100644 --- a/docs/release-notes/release-notes-0.15.0.md +++ b/docs/release-notes/release-notes-0.15.0.md @@ -76,12 +76,17 @@ * [Fix itest not picking up local config file or creating directories in home dir of the user](https://github.com/lightningnetwork/lnd/pull/6202). +* [A refactor of `SelectHopHints`](https://github.com/lightningnetwork/lnd/pull/6182) + allows code external to lnd to call the function, where previously it would + require access to lnd's internals. + # Contributors (Alphabetical Order) * 3nprob * Andreas Schjønhaug * asvdf * BTCparadigm +* Carla Kirk-Cohen * Carsten Otto * Dan Bolser * Daniel McNally