Merge pull request #6914 from positiveblue/fix-6898

addinvoice: refactor hint hop selection algorithm

aliasmgr/aliasmgr.go

@@ -87,18 +87,22 @@ type Manager struct {
 	// negotiated option-scid-alias feature bit.
 	aliasToBase map[lnwire.ShortChannelID]lnwire.ShortChannelID
 
+	// peerAlias is a cache for the alias SCIDs that our peers send us in
+	// the funding_locked TLV. The keys are the ChannelID generated from
+	// the FundingOutpoint and the values are the remote peer's alias SCID.
+	// The values should match the ones stored in the "invoice-alias-bucket"
+	// bucket.
+	peerAlias map[lnwire.ChannelID]lnwire.ShortChannelID
+
 	sync.RWMutex
 }
 
 // NewManager initializes an alias Manager from the passed database backend.
 func NewManager(db kvdb.Backend) (*Manager, error) {
 	m := &Manager{backend: db}
-	m.baseToSet = make(
+	m.baseToSet = make(map[lnwire.ShortChannelID][]lnwire.ShortChannelID)
-		map[lnwire.ShortChannelID][]lnwire.ShortChannelID,
+	m.aliasToBase = make(map[lnwire.ShortChannelID]lnwire.ShortChannelID)
-	)
+	m.peerAlias = make(map[lnwire.ChannelID]lnwire.ShortChannelID)
-	m.aliasToBase = make(
-		map[lnwire.ShortChannelID]lnwire.ShortChannelID,
-	)
 
 	err := m.populateMaps()
 	return m, err
@@ -115,6 +119,10 @@ func (m *Manager) populateMaps() error {
 	// populate the Manager's actual maps.
 	aliasMap := make(map[lnwire.ShortChannelID]lnwire.ShortChannelID)
 
+	// This map caches the ChannelID/alias SCIDs stored in the database and
+	// is used to populate the Manager's cache.
+	peerAliasMap := make(map[lnwire.ChannelID]lnwire.ShortChannelID)
+
 	err := kvdb.Update(m.backend, func(tx kvdb.RwTx) error {
 		baseConfBucket, err := tx.CreateTopLevelBucket(confirmedBucket)
 		if err != nil {
@@ -152,12 +160,34 @@ func (m *Manager) populateMaps() error {
 			aliasMap[aliasScid] = baseScid
 			return nil
 		})
+		if err != nil {
+			return err
+		}
+
+		invAliasBucket, err := tx.CreateTopLevelBucket(
+			invoiceAliasBucket,
+		)
+		if err != nil {
+			return err
+		}
+
+		err = invAliasBucket.ForEach(func(k, v []byte) error {
+			var chanID lnwire.ChannelID
+			copy(chanID[:], k)
+			alias := lnwire.NewShortChanIDFromInt(
+				byteOrder.Uint64(v),
+			)
+
+			peerAliasMap[chanID] = alias
+
+			return nil
+		})
+
 		return err
 	}, func() {
 		baseConfMap = make(map[lnwire.ShortChannelID]struct{})
-		aliasMap = make(
+		aliasMap = make(map[lnwire.ShortChannelID]lnwire.ShortChannelID)
-			map[lnwire.ShortChannelID]lnwire.ShortChannelID,
+		peerAliasMap = make(map[lnwire.ChannelID]lnwire.ShortChannelID)
-		)
 	})
 	if err != nil {
 		return err
@@ -176,6 +206,9 @@ func (m *Manager) populateMaps() error {
 		m.aliasToBase[aliasSCID] = baseSCID
 	}
 
+	// Populate the peer alias cache.
+	m.peerAlias = peerAliasMap
+
 	return nil
 }
 
@@ -242,7 +275,9 @@ func (m *Manager) AddLocalAlias(alias, baseScid lnwire.ShortChannelID,
 
 // GetAliases fetches the set of aliases stored under a given base SCID from
 // write-through caches.
-func (m *Manager) GetAliases(base lnwire.ShortChannelID) []lnwire.ShortChannelID {
+func (m *Manager) GetAliases(
+	base lnwire.ShortChannelID) []lnwire.ShortChannelID {
+
 	m.RLock()
 	defer m.RUnlock()
 
@@ -310,7 +345,10 @@ func (m *Manager) DeleteSixConfs(baseScid lnwire.ShortChannelID) error {
 func (m *Manager) PutPeerAlias(chanID lnwire.ChannelID,
 	alias lnwire.ShortChannelID) error {
 
-	return kvdb.Update(m.backend, func(tx kvdb.RwTx) error {
+	m.Lock()
+	defer m.Unlock()
+
+	err := kvdb.Update(m.backend, func(tx kvdb.RwTx) error {
 		bucket, err := tx.CreateTopLevelBucket(invoiceAliasBucket)
 		if err != nil {
 			return err
@@ -320,36 +358,30 @@ func (m *Manager) PutPeerAlias(chanID lnwire.ChannelID,
 		byteOrder.PutUint64(scratch[:], alias.ToUint64())
 		return bucket.Put(chanID[:], scratch[:])
 	}, func() {})
+	if err != nil {
+		return err
+	}
+
+	// Now that the database state has been updated, we can update it in
+	// our cache.
+	m.peerAlias[chanID] = alias
+
+	return nil
 }
 
 // GetPeerAlias retrieves a peer's alias SCID by the channel's ChanID.
-func (m *Manager) GetPeerAlias(chanID lnwire.ChannelID) (
+func (m *Manager) GetPeerAlias(chanID lnwire.ChannelID) (lnwire.ShortChannelID,
-	lnwire.ShortChannelID, error) {
+	error) {
 
-	var alias lnwire.ShortChannelID
+	m.RLock()
+	defer m.RUnlock()
 
-	err := kvdb.Update(m.backend, func(tx kvdb.RwTx) error {
+	alias, ok := m.peerAlias[chanID]
-		bucket, err := tx.CreateTopLevelBucket(invoiceAliasBucket)
+	if !ok || alias == hop.Source {
-		if err != nil {
+		return lnwire.ShortChannelID{}, errNoPeerAlias
-			return err
-		}
-
-		aliasBytes := bucket.Get(chanID[:])
-		if aliasBytes == nil {
-			return nil
-		}
-
-		alias = lnwire.NewShortChanIDFromInt(
-			byteOrder.Uint64(aliasBytes),
-		)
-		return nil
-	}, func() {})
-
-	if alias == hop.Source {
-		return alias, errNoPeerAlias
 	}
 
-	return alias, err
+	return alias, nil
 }
 
 // RequestAlias returns a new ALIAS ShortChannelID to the caller by allocating

docs/release-notes/release-notes-0.15.2.md

@@ -16,5 +16,16 @@
 
 # Contributors (Alphabetical Order)
 
+
+## Performance improvements
+
+* [Refactor hop hint selection
+  algorithm](https://github.com/lightningnetwork/lnd/pull/6914)
+
+
+# Contributors (Alphabetical Order)
+
 * Eugene Siegel
+* Jordi Montes
 * Oliver Gugger
+

lnrpc/invoicesrpc/addinvoice.go

@@ -7,6 +7,8 @@ import (
 	"errors"
 	"fmt"
 	"math"
+	mathRand "math/rand"
+	"sort"
 	"time"
 
 	"github.com/btcsuite/btcd/btcec/v2"
@@ -35,6 +37,10 @@ const (
 	// inbound capacity we want our hop hints to represent, allowing us to
 	// have some leeway if peers go offline.
 	hopHintFactor = 2
+
+	// maxHopHints is the maximum number of hint paths that will be included
+	// in an invoice.
+	maxHopHints = 20
 )
 
 // AddInvoiceConfig contains dependencies for invoice creation.
@@ -126,8 +132,8 @@ type AddInvoiceData struct {
 	// NOTE: Preimage should always be set to nil when this value is true.
 	Amp bool
 
-	// RouteHints are optional route hints that can each be individually used
+	// RouteHints are optional route hints that can each be individually
-	// to assist in reaching the invoice's destination.
+	// used to assist in reaching the invoice's destination.
 	RouteHints [][]zpay32.HopHint
 }
 
@@ -159,7 +165,9 @@ func (d *AddInvoiceData) paymentHashAndPreimage() (
 
 // ampPaymentHashAndPreimage returns the payment hash to use for an AMP invoice.
 // The preimage will always be nil.
-func (d *AddInvoiceData) ampPaymentHashAndPreimage() (*lntypes.Preimage, lntypes.Hash, error) {
+func (d *AddInvoiceData) ampPaymentHashAndPreimage() (*lntypes.Preimage,
+	lntypes.Hash, error) {
+
 	switch {
 	// Preimages cannot be set on AMP invoice.
 	case d.Preimage != nil:
@@ -184,7 +192,9 @@ func (d *AddInvoiceData) ampPaymentHashAndPreimage() (*lntypes.Preimage, lntypes
 
 // mppPaymentHashAndPreimage returns the payment hash and preimage to use for an
 // MPP invoice.
-func (d *AddInvoiceData) mppPaymentHashAndPreimage() (*lntypes.Preimage, lntypes.Hash, error) {
+func (d *AddInvoiceData) mppPaymentHashAndPreimage() (*lntypes.Preimage,
+	lntypes.Hash, error) {
+
 	var (
 		paymentPreimage *lntypes.Preimage
 		paymentHash     lntypes.Hash
@@ -235,11 +245,14 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 	// exceed the maximum values for either of the fields.
 	if len(invoice.Memo) > channeldb.MaxMemoSize {
 		return nil, nil, fmt.Errorf("memo too large: %v bytes "+
-			"(maxsize=%v)", len(invoice.Memo), channeldb.MaxMemoSize)
+			"(maxsize=%v)", len(invoice.Memo),
+			channeldb.MaxMemoSize)
 	}
-	if len(invoice.DescriptionHash) > 0 && len(invoice.DescriptionHash) != 32 {
+	if len(invoice.DescriptionHash) > 0 &&
-		return nil, nil, fmt.Errorf("description hash is %v bytes, must be 32",
+		len(invoice.DescriptionHash) != 32 {
-			len(invoice.DescriptionHash))
+
+		return nil, nil, fmt.Errorf("description hash is %v bytes, "+
+			"must be 32", len(invoice.DescriptionHash))
 	}
 
 	// We set the max invoice amount to 100k BTC, which itself is several
@@ -281,8 +294,8 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 		addr, err := btcutil.DecodeAddress(invoice.FallbackAddr,
 			cfg.ChainParams)
 		if err != nil {
-			return nil, nil, fmt.Errorf("invalid fallback address: %v",
+			return nil, nil, fmt.Errorf("invalid fallback "+
-				err)
+				"address: %v", err)
 		}
 		options = append(options, zpay32.FallbackAddr(addr))
 	}
@@ -314,11 +327,13 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 
 	// Otherwise, use the default AMP expiry.
 	default:
-		options = append(options, zpay32.Expiry(DefaultAMPInvoiceExpiry))
+		defaultExpiry := zpay32.Expiry(DefaultAMPInvoiceExpiry)
+		options = append(options, defaultExpiry)
 	}
 
-	// If the description hash is set, then we add it do the list of options.
+	// If the description hash is set, then we add it do the list of
-	// If not, use the memo field as the payment request description.
+	// options. If not, use the memo field as the payment request
+	// description.
 	if len(invoice.DescriptionHash) > 0 {
 		var descHash [32]byte
 		copy(descHash[:], invoice.DescriptionHash[:])
@@ -333,8 +348,10 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 	// an option on the command line when creating an invoice.
 	switch {
 	case invoice.CltvExpiry > math.MaxUint16:
-		return nil, nil, fmt.Errorf("CLTV delta of %v is too large, max "+
+		return nil, nil, fmt.Errorf("CLTV delta of %v is too large, "+
-			"accepted is: %v", invoice.CltvExpiry, math.MaxUint16)
+			"max accepted is: %v", invoice.CltvExpiry,
+			math.MaxUint16)
+
 	case invoice.CltvExpiry != 0:
 		// Disallow user-chosen final CLTV deltas below the required
 		// minimum.
@@ -346,99 +363,52 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 
 		options = append(options,
 			zpay32.CLTVExpiry(invoice.CltvExpiry))
+
 	default:
 		// TODO(roasbeef): assumes set delta between versions
-		defaultDelta := cfg.DefaultCLTVExpiry
+		defaultCLTVExpiry := uint64(cfg.DefaultCLTVExpiry)
-		options = append(options, zpay32.CLTVExpiry(uint64(defaultDelta)))
+		options = append(options, zpay32.CLTVExpiry(defaultCLTVExpiry))
 	}
 
-	// We make sure that the given invoice routing hints number is within the
+	// We make sure that the given invoice routing hints number is within
-	// valid range
+	// the valid range
-	if len(invoice.RouteHints) > 20 {
+	if len(invoice.RouteHints) > maxHopHints {
-		return nil, nil, fmt.Errorf("number of routing hints must not exceed " +
+		return nil, nil, fmt.Errorf("number of routing hints must "+
-			"maximum of 20")
+			"not exceed maximum of %v", maxHopHints)
 	}
 
-	// We continue by populating the requested routing hints indexing their
+	// Include route hints if needed.
-	// corresponding channels so we won't duplicate them.
+	if len(invoice.RouteHints) > 0 || invoice.Private {
-	forcedHints := make(map[uint64]struct{})
+		// Validate provided hop hints.
-	for _, h := range invoice.RouteHints {
+		for _, hint := range invoice.RouteHints {
-		if len(h) == 0 {
+			if len(hint) == 0 {
-			return nil, nil, fmt.Errorf("number of hop hint within a route must " +
+				return nil, nil, fmt.Errorf("number of hop " +
-				"be positive")
+					"hint within a route must be positive")
+			}
 		}
-		options = append(options, zpay32.RouteHint(h))
 
-		// Only this first hop is our direct channel.
+		totalHopHints := len(invoice.RouteHints)
-		forcedHints[h[0].ChannelID] = struct{}{}
+		if invoice.Private {
-	}
+			totalHopHints = maxHopHints
+		}
 
-	// If we were requested to include routing hints in the invoice, then
+		hopHintsCfg := newSelectHopHintsCfg(cfg, totalHopHints)
-	// we'll fetch all of our available private channels and create routing
+		hopHints, err := PopulateHopHints(
-	// hints for them.
+			hopHintsCfg, amtMSat, invoice.RouteHints,
-	if invoice.Private {
+		)
-		openChannels, err := cfg.ChanDB.FetchAllChannels()
 		if err != nil {
-			return nil, nil, fmt.Errorf("could not fetch all channels")
+			return nil, nil, fmt.Errorf("unable to populate hop "+
+				"hints: %v", err)
 		}
 
-		if len(openChannels) > 0 {
+		// Convert our set of selected hop hints into route
-			// We filter the channels by excluding the ones that were specified by
+		// hints and add to our invoice options.
-			// the caller and were already added.
+		for _, hopHint := range hopHints {
-			var filteredChannels []*HopHintInfo
+			routeHint := zpay32.RouteHint(hopHint)
-			for _, c := range openChannels {
-				if _, ok := forcedHints[c.ShortChanID().ToUint64()]; ok {
-					continue
-				}
 
-				// If this is a zero-conf channel, check if the
+			options = append(
-				// confirmed SCID was used in forcedHints.
+				options, routeHint,
-				realScid := c.ZeroConfRealScid().ToUint64()
-				if c.IsZeroConf() {
-					if _, ok := forcedHints[realScid]; ok {
-						continue
-					}
-				}
-
-				chanID := lnwire.NewChanIDFromOutPoint(
-					&c.FundingOutpoint,
-				)
-
-				// Check whether the the peer's alias was
-				// provided in forcedHints.
-				peerAlias, _ := cfg.GetAlias(chanID)
-				peerScid := peerAlias.ToUint64()
-				if _, ok := forcedHints[peerScid]; ok {
-					continue
-				}
-
-				isActive := cfg.IsChannelActive(chanID)
-
-				hopHintInfo := newHopHintInfo(c, isActive)
-				filteredChannels = append(
-					filteredChannels, hopHintInfo,
-				)
-			}
-
-			// 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, hopHintsCfg, filteredChannels,
-				numMaxHophints,
 			)
-
-			// 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,
-				)
-			}
 		}
 	}
 
@@ -576,30 +546,6 @@ func chanCanBeHopHint(channel *HopHintInfo, cfg *SelectHopHintsCfg) (
 	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 *[][]zpay32.HopHint,
-	channel *HopHintInfo, chanPolicy *channeldb.ChannelEdgePolicy,
-	aliasScid lnwire.ShortChannelID) {
-
-	hopHint := zpay32.HopHint{
-		NodeID:      channel.RemotePubkey,
-		ChannelID:   channel.ShortChannelID,
-		FeeBaseMSat: uint32(chanPolicy.FeeBaseMSat),
-		FeeProportionalMillionths: uint32(
-			chanPolicy.FeeProportionalMillionths,
-		),
-		CLTVExpiryDelta: chanPolicy.TimeLockDelta,
-	}
-
-	var defaultScid lnwire.ShortChannelID
-	if aliasScid != defaultScid {
-		hopHint.ChannelID = aliasScid.ToUint64()
-	}
-
-	*hopHints = append(*hopHints, []zpay32.HopHint{hopHint})
-}
-
 // HopHintInfo contains the channel information required to create a hop hint.
 type HopHintInfo struct {
 	// IsPublic indicates whether a channel is advertised to the network.
@@ -647,6 +593,22 @@ func newHopHintInfo(c *channeldb.OpenChannel, isActive bool) *HopHintInfo {
 	}
 }
 
+// newHopHint returns a new hop hint using the relevant data from a hopHintInfo
+// and a ChannelEdgePolicy.
+func newHopHint(hopHintInfo *HopHintInfo,
+	chanPolicy *channeldb.ChannelEdgePolicy) zpay32.HopHint {
+
+	return zpay32.HopHint{
+		NodeID:      hopHintInfo.RemotePubkey,
+		ChannelID:   hopHintInfo.ShortChannelID,
+		FeeBaseMSat: uint32(chanPolicy.FeeBaseMSat),
+		FeeProportionalMillionths: uint32(
+			chanPolicy.FeeProportionalMillionths,
+		),
+		CLTVExpiryDelta: chanPolicy.TimeLockDelta,
+	}
+}
+
 // SelectHopHintsCfg contains the dependencies required to obtain hop hints
 // for an invoice.
 type SelectHopHintsCfg struct {
@@ -664,169 +626,208 @@ type SelectHopHintsCfg struct {
 	// GetAlias allows the peer's alias SCID to be retrieved for private
 	// option_scid_alias channels.
 	GetAlias func(lnwire.ChannelID) (lnwire.ShortChannelID, error)
+
+	// FetchAllChannels retrieves all open channels currently stored
+	// within the database.
+	FetchAllChannels func() ([]*channeldb.OpenChannel, error)
+
+	// IsChannelActive checks whether the channel identified by the provided
+	// ChannelID is considered active.
+	IsChannelActive func(chanID lnwire.ChannelID) bool
+
+	// MaxHopHints is the maximum number of hop hints we are interested in.
+	MaxHopHints int
 }
 
-func newSelectHopHintsCfg(invoicesCfg *AddInvoiceConfig) *SelectHopHintsCfg {
+func newSelectHopHintsCfg(invoicesCfg *AddInvoiceConfig,
+	maxHopHints int) *SelectHopHintsCfg {
+
 	return &SelectHopHintsCfg{
+		FetchAllChannels:      invoicesCfg.ChanDB.FetchAllChannels,
+		IsChannelActive:       invoicesCfg.IsChannelActive,
 		IsPublicNode:          invoicesCfg.Graph.IsPublicNode,
 		FetchChannelEdgesByID: invoicesCfg.Graph.FetchChannelEdgesByID,
 		GetAlias:              invoicesCfg.GetAlias,
+		MaxHopHints:           maxHopHints,
 	}
 }
 
 // sufficientHints checks whether we have sufficient hop hints, based on the
-// following criteria:
+// any of the following criteria:
-//   - Hop hint count: limit to a set number of hop hints, regardless of whether
+//   - Hop hint count: the number of hints have reach our max target.
-//     we've reached our invoice amount or not.
+//   - Total incoming capacity: the sum of the remote balance amount in the
-//   - Total incoming capacity: limit to our invoice amount * scaling factor to
+//     hints is bigger of equal than our target (currently twice the invoice
-//     allow for some of our links going offline.
+//     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.
-func sufficientHints(numHints, maxHints, scalingFactor int, amount,
+func sufficientHints(nHintsLeft int, currentAmount,
-	totalHintAmount lnwire.MilliSatoshi) bool {
+	targetAmount lnwire.MilliSatoshi) bool {
 
-	if numHints >= maxHints {
+	if nHintsLeft <= 0 {
-		log.Debug("Reached maximum number of hop hints")
+		log.Debugf("Reached targeted number of hop hints")
 		return true
 	}
 
-	requiredAmount := amount * lnwire.MilliSatoshi(scalingFactor)
+	if currentAmount >= targetAmount {
-	if totalHintAmount >= requiredAmount {
 		log.Debugf("Total hint amount: %v has reached target hint "+
-			"bandwidth: %v (invoice amount: %v * factor: %v)",
+			"bandwidth: %v", currentAmount, targetAmount)
-			totalHintAmount, requiredAmount, amount,
-			scalingFactor)
-
 		return true
 	}
 
 	return false
 }
 
-// SelectHopHints will select up to numMaxHophints from the set of passed open
+// getPotentialHints returns a slice of open channels that should be considered
+// for the hopHint list in an invoice. The slice is sorted in descending order
+// based on the remote balance.
+func getPotentialHints(cfg *SelectHopHintsCfg) ([]*channeldb.OpenChannel,
+	error) {
+
+	// TODO(positiveblue): get the channels slice already filtered by
+	// private == true and sorted by RemoteBalance?
+	openChannels, err := cfg.FetchAllChannels()
+	if err != nil {
+		return nil, err
+	}
+
+	privateChannels := make([]*channeldb.OpenChannel, 0, len(openChannels))
+	for _, oc := range openChannels {
+		isPublic := oc.ChannelFlags&lnwire.FFAnnounceChannel != 0
+		if !isPublic {
+			privateChannels = append(privateChannels, oc)
+		}
+	}
+
+	// Sort the channels in descending remote balance.
+	compareRemoteBalance := func(i, j int) bool {
+		iBalance := privateChannels[i].LocalCommitment.RemoteBalance
+		jBalance := privateChannels[j].LocalCommitment.RemoteBalance
+		return iBalance > jBalance
+	}
+	sort.Slice(privateChannels, compareRemoteBalance)
+
+	return privateChannels, nil
+}
+
+// shouldIncludeChannel returns true if the channel passes all the checks to
+// be a hopHint in a given invoice.
+func shouldIncludeChannel(cfg *SelectHopHintsCfg,
+	channel *channeldb.OpenChannel,
+	alreadyIncluded map[uint64]bool) (zpay32.HopHint, lnwire.MilliSatoshi,
+	bool) {
+
+	if _, ok := alreadyIncluded[channel.ShortChannelID.ToUint64()]; ok {
+		return zpay32.HopHint{}, 0, false
+	}
+
+	chanID := lnwire.NewChanIDFromOutPoint(
+		&channel.FundingOutpoint,
+	)
+
+	hopHintInfo := newHopHintInfo(channel, cfg.IsChannelActive(chanID))
+
+	// If this channel can't be a hop hint, then skip it.
+	edgePolicy, canBeHopHint := chanCanBeHopHint(hopHintInfo, cfg)
+	if edgePolicy == nil || !canBeHopHint {
+		return zpay32.HopHint{}, 0, false
+	}
+
+	if hopHintInfo.ScidAliasFeature {
+		alias, err := cfg.GetAlias(chanID)
+		if err != nil {
+			return zpay32.HopHint{}, 0, false
+		}
+
+		if alias.IsDefault() || alreadyIncluded[alias.ToUint64()] {
+			return zpay32.HopHint{}, 0, false
+		}
+
+		hopHintInfo.ShortChannelID = alias.ToUint64()
+	}
+
+	// Now that we know this channel use usable, add it as a hop hint and
+	// the indexes we'll use later.
+	hopHint := newHopHint(hopHintInfo, edgePolicy)
+	return hopHint, hopHintInfo.RemoteBalance, true
+}
+
+// selectHopHints iterates a list of potential hints selecting the valid hop
+// hints until we have enough hints or run out of channels.
+//
+// NOTE: selectHopHints expects potentialHints to be already sorted in
+// descending priority.
+func selectHopHints(cfg *SelectHopHintsCfg, nHintsLeft int,
+	targetBandwidth lnwire.MilliSatoshi,
+	potentialHints []*channeldb.OpenChannel,
+	alreadyIncluded map[uint64]bool) [][]zpay32.HopHint {
+
+	currentBandwidth := lnwire.MilliSatoshi(0)
+	hopHints := make([][]zpay32.HopHint, 0, nHintsLeft)
+	for _, channel := range potentialHints {
+		enoughHopHints := sufficientHints(
+			nHintsLeft, currentBandwidth, targetBandwidth,
+		)
+		if enoughHopHints {
+			return hopHints
+		}
+
+		hopHint, remoteBalance, include := shouldIncludeChannel(
+			cfg, channel, alreadyIncluded,
+		)
+
+		if include {
+			// Now that we now this channel use usable, add it as a hop
+			// hint and the indexes we'll use later.
+			hopHints = append(hopHints, []zpay32.HopHint{hopHint})
+			currentBandwidth += remoteBalance
+			nHintsLeft--
+		}
+	}
+
+	// We do not want to leak information about how our remote balance is
+	// distributed in our private channels. We shuffle the selected ones
+	// here so they do not appear in order in the invoice.
+	mathRand.Shuffle(
+		len(hopHints), func(i, j int) {
+			hopHints[i], hopHints[j] = hopHints[j], hopHints[i]
+		},
+	)
+	return hopHints
+}
+
+// PopulateHopHints will select up to cfg.MaxHophints from the current 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 *SelectHopHintsCfg,
+func PopulateHopHints(cfg *SelectHopHintsCfg, amtMSat lnwire.MilliSatoshi,
-	openChannels []*HopHintInfo,
+	forcedHints [][]zpay32.HopHint) ([][]zpay32.HopHint, error) {
-	numMaxHophints int) [][]zpay32.HopHint {
 
-	// We'll add our hop hints in two passes, first we'll add all channels
+	hopHints := forcedHints
-	// 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([][]zpay32.HopHint, 0, numMaxHophints)
-	for _, channel := range openChannels {
-		enoughHopHints := sufficientHints(
-			len(hopHints), numMaxHophints, hopHintFactor, amtMSat,
-			totalHintBandwidth,
-		)
-		if enoughHopHints {
-			log.Debugf("First pass of hop selection has " +
-				"sufficient hints")
 
-			return hopHints
+	// If we already have enough hints we don't need to add any more.
-		}
+	nHintsLeft := cfg.MaxHopHints - len(hopHints)
-
+	if nHintsLeft <= 0 {
-		// If this channel can't be a hop hint, then skip it.
+		return hopHints, nil
-		edgePolicy, canBeHopHint := chanCanBeHopHint(channel, 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.RemoteBalance < amtMSat {
-			continue
-		}
-
-		// Lookup and see if there is an alias SCID that exists.
-		chanID := lnwire.NewChanIDFromOutPoint(
-			&channel.FundingOutpoint,
-		)
-		alias, _ := cfg.GetAlias(chanID)
-
-		// If this is a channel where the option-scid-alias feature bit
-		// was negotiated and the alias is not yet assigned, we cannot
-		// issue an invoice. Doing so might expose the confirmed SCID
-		// of a private channel.
-		if channel.ScidAliasFeature {
-			var defaultScid lnwire.ShortChannelID
-			if alias == defaultScid {
-				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, alias)
-
-		hopHintChans[channel.FundingOutpoint] = struct{}{}
-		totalHintBandwidth += channel.RemoteBalance
 	}
 
-	// In this second pass we'll add channels, and we'll either stop when
+	alreadyIncluded := make(map[uint64]bool)
-	// we have 20 hop hints, we've run through all the available channels,
+	for _, hopHint := range hopHints {
-	// or if the sum of available bandwidth in the routing hints exceeds 2x
+		alreadyIncluded[hopHint[0].ChannelID] = true
-	// the payment amount. We do 2x here to account for a margin of error
-	// if some of the selected channels no longer become operable.
-	for i := 0; i < len(openChannels); i++ {
-		enoughHopHints := sufficientHints(
-			len(hopHints), numMaxHophints, hopHintFactor, amtMSat,
-			totalHintBandwidth,
-		)
-		if enoughHopHints {
-			log.Debugf("Second pass of hop selection has " +
-				"sufficient hints")
-
-			return hopHints
-		}
-
-		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(channel, cfg)
-		if !canBeHopHint || remotePolicy == nil {
-			continue
-		}
-
-		// Lookup and see if there's an alias SCID that exists.
-		chanID := lnwire.NewChanIDFromOutPoint(
-			&channel.FundingOutpoint,
-		)
-		alias, _ := cfg.GetAlias(chanID)
-
-		// If this is a channel where the option-scid-alias feature bit
-		// was negotiated and the alias is not yet assigned, we cannot
-		// issue an invoice. Doing so might expose the confirmed SCID
-		// of a private channel.
-		if channel.ScidAliasFeature {
-			var defaultScid lnwire.ShortChannelID
-			if alias == defaultScid {
-				continue
-			}
-		}
-
-		// Include the route hint in our set of options that will be
-		// used when creating the invoice.
-		addHopHint(&hopHints, channel, remotePolicy, alias)
-
-		// 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.RemoteBalance
 	}
 
-	return hopHints
+	potentialHints, err := getPotentialHints(cfg)
+	if err != nil {
+		return nil, err
+	}
+
+	targetBandwidth := amtMSat * hopHintFactor
+	selectedHints := selectHopHints(
+		cfg, nHintsLeft, targetBandwidth, potentialHints,
+		alreadyIncluded,
+	)
+
+	hopHints = append(hopHints, selectedHints...)
+	return hopHints, nil
 }

lnwire/short_channel_id.go

@@ -64,6 +64,12 @@ func (c *ShortChannelID) Record() tlv.Record {
 	)
 }
 
+// IsDefault returns true if the ShortChannelID represents the zero value for
+// its type.
+func (c ShortChannelID) IsDefault() bool {
+	return c == ShortChannelID{}
+}
+
 // EShortChannelID is an encoder for ShortChannelID. It is exported so other
 // packages can use the encoding scheme.
 func EShortChannelID(w io.Writer, val interface{}, buf *[8]byte) error {