From 76e711ead086c9877195265f09855b0eb8756031 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Wed, 9 Nov 2022 18:05:36 +0100 Subject: [PATCH] routing: refactor unified policies to edges This commit refactors the semantics of unified policies to unified edges. The main changes are the following renamings: * unifiedPolicies -> nodeEdgeUnifier * unifiedPolicy -> edgeUnifier * unifiedPolicyEdge -> unifiedEdge Comments and shortened variable names are changed to reflect the new semantics. --- routing/pathfind.go | 14 +-- routing/router.go | 39 ++++---- .../{unified_policies.go => unified_edges.go} | 96 +++++++++---------- ...policies_test.go => unified_edges_test.go} | 29 +++--- 4 files changed, 87 insertions(+), 91 deletions(-) rename routing/{unified_policies.go => unified_edges.go} (70%) rename routing/{unified_policies_test.go => unified_edges_test.go} (72%) diff --git a/routing/pathfind.go b/routing/pathfind.go index fb351bd88..fb11fa241 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -628,7 +628,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // satisfy our specific requirements. processEdge := func(fromVertex route.Vertex, fromFeatures *lnwire.FeatureVector, - edge *unifiedPolicyEdge, toNodeDist *nodeWithDist) { + edge *unifiedEdge, toNodeDist *nodeWithDist) { edgesExpanded++ @@ -849,8 +849,8 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, pivot := partialPath.node - // Create unified policies for all incoming connections. - u := newUnifiedPolicies(self, pivot, outgoingChanMap) + // Create unified edges for all incoming connections. + u := newNodeEdgeUnifier(self, pivot, outgoingChanMap) err := u.addGraphPolicies(g.graph) if err != nil { @@ -865,7 +865,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Expand all connections using the optimal policy for each // connection. - for fromNode, unifiedPolicy := range u.policies { + for fromNode, edgeUnifier := range u.edgeUnifiers { // The target node is not recorded in the distance map. // Therefore we need to have this check to prevent // creating a cycle. Only when we intend to route to @@ -882,11 +882,11 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, continue } - policy := unifiedPolicy.getPolicy( + edge := edgeUnifier.getEdge( amtToSend, g.bandwidthHints, ) - if policy == nil { + if edge == nil { continue } @@ -903,7 +903,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, // Check if this candidate node is better than what we // already have. - processEdge(fromNode, fromFeatures, policy, partialPath) + processEdge(fromNode, fromFeatures, edge, partialPath) } if nodeHeap.Len() == 0 { diff --git a/routing/router.go b/routing/router.go index 696d9cb9c..e8d2ab96a 100644 --- a/routing/router.go +++ b/routing/router.go @@ -2765,9 +2765,8 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, return nil, err } - // Allocate a list that will contain the unified policies for this - // route. - edges := make([]*unifiedPolicy, len(hops)) + // Allocate a list that will contain the edge unifiers for this route. + unifiers := make([]*edgeUnifier, len(hops)) var runningAmt lnwire.MilliSatoshi if useMinAmt { @@ -2796,9 +2795,9 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, localChan := i == 0 - // Build unified policies for this hop based on the channels - // known in the graph. - u := newUnifiedPolicies(source, toNode, outgoingChans) + // Build unified edges for this hop based on the channels known + // in the graph. + u := newNodeEdgeUnifier(source, toNode, outgoingChans) err := u.addGraphPolicies(r.cachedGraph) if err != nil { @@ -2806,7 +2805,7 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, } // Exit if there are no channels. - unifiedPolicy, ok := u.policies[fromNode] + edgeUnifier, ok := u.edgeUnifiers[fromNode] if !ok { log.Errorf("Cannot find policy for node %v", fromNode) return nil, ErrNoChannel{ @@ -2817,18 +2816,18 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, // If using min amt, increase amt if needed. if useMinAmt { - min := unifiedPolicy.minAmt() + min := edgeUnifier.minAmt() if min > runningAmt { runningAmt = min } } - // Get a forwarding policy for the specific amount that we want - // to forward. - policy := unifiedPolicy.getPolicy(runningAmt, bandwidthHints) - if policy == nil { + // Get an edge for the specific amount that we want to forward. + edge := edgeUnifier.getEdge(runningAmt, bandwidthHints) + if edge == nil { log.Errorf("Cannot find policy with amt=%v for node %v", runningAmt, fromNode) + return nil, ErrNoChannel{ fromNode: fromNode, position: i, @@ -2837,13 +2836,13 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, // Add fee for this hop. if !localChan { - runningAmt += policy.policy.ComputeFee(runningAmt) + runningAmt += edge.policy.ComputeFee(runningAmt) } log.Tracef("Select channel %v at position %v", - policy.policy.ChannelID, i) + edge.policy.ChannelID, i) - edges[i] = unifiedPolicy + unifiers[i] = edgeUnifier } // Now that we arrived at the start of the route and found out the route @@ -2852,9 +2851,9 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, // amount ranges re-checked. var pathEdges []*channeldb.CachedEdgePolicy receiverAmt := runningAmt - for i, edge := range edges { - policy := edge.getPolicy(receiverAmt, bandwidthHints) - if policy == nil { + for i, unifier := range unifiers { + edge := unifier.getEdge(receiverAmt, bandwidthHints) + if edge == nil { return nil, ErrNoChannel{ fromNode: hops[i-1], position: i, @@ -2863,12 +2862,12 @@ func (r *ChannelRouter) BuildRoute(amt *lnwire.MilliSatoshi, if i > 0 { // Decrease the amount to send while going forward. - receiverAmt -= policy.policy.ComputeFeeFromIncoming( + receiverAmt -= edge.policy.ComputeFeeFromIncoming( receiverAmt, ) } - pathEdges = append(pathEdges, policy.policy) + pathEdges = append(pathEdges, edge.policy) } // Build and return the final route. diff --git a/routing/unified_policies.go b/routing/unified_edges.go similarity index 70% rename from routing/unified_policies.go rename to routing/unified_edges.go index c2cc1b373..1c3574e8d 100644 --- a/routing/unified_policies.go +++ b/routing/unified_edges.go @@ -7,16 +7,16 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) -// unifiedPolicies holds all unified policies for connections towards a node. -type unifiedPolicies struct { - // policies contains a unified policy for every from node. - policies map[route.Vertex]*unifiedPolicy +// nodeEdgeUnifier holds all edge unifiers for connections towards a node. +type nodeEdgeUnifier struct { + // edgeUnifiers contains an edge unifier for every from node. + edgeUnifiers map[route.Vertex]*edgeUnifier // sourceNode is the sender of a payment. The rules to pick the final // policy are different for local channels. sourceNode route.Vertex - // toNode is the node for which the unified policies are instantiated. + // toNode is the node for which the edge unifiers are instantiated. toNode route.Vertex // outChanRestr is an optional outgoing channel restriction for the @@ -24,13 +24,13 @@ type unifiedPolicies struct { outChanRestr map[uint64]struct{} } -// newUnifiedPolicies instantiates a new unifiedPolicies object. Channel +// newNodeEdgeUnifier instantiates a new nodeEdgeUnifier object. Channel // policies can be added to this object. -func newUnifiedPolicies(sourceNode, toNode route.Vertex, - outChanRestr map[uint64]struct{}) *unifiedPolicies { +func newNodeEdgeUnifier(sourceNode, toNode route.Vertex, + outChanRestr map[uint64]struct{}) *nodeEdgeUnifier { - return &unifiedPolicies{ - policies: make(map[route.Vertex]*unifiedPolicy), + return &nodeEdgeUnifier{ + edgeUnifiers: make(map[route.Vertex]*edgeUnifier), toNode: toNode, sourceNode: sourceNode, outChanRestr: outChanRestr, @@ -39,7 +39,7 @@ func newUnifiedPolicies(sourceNode, toNode route.Vertex, // addPolicy adds a single channel policy. Capacity may be zero if unknown // (light clients). -func (u *unifiedPolicies) addPolicy(fromNode route.Vertex, +func (u *nodeEdgeUnifier) addPolicy(fromNode route.Vertex, edge *channeldb.CachedEdgePolicy, capacity btcutil.Amount) { localChan := fromNode == u.sourceNode @@ -51,16 +51,16 @@ func (u *unifiedPolicies) addPolicy(fromNode route.Vertex, } } - // Update the policies map. - policy, ok := u.policies[fromNode] + // Update the edgeUnifiers map. + unifier, ok := u.edgeUnifiers[fromNode] if !ok { - policy = &unifiedPolicy{ + unifier = &edgeUnifier{ localChan: localChan, } - u.policies[fromNode] = policy + u.edgeUnifiers[fromNode] = unifier } - policy.edges = append(policy.edges, &unifiedPolicyEdge{ + unifier.edges = append(unifier.edges, &unifiedEdge{ policy: edge, capacity: capacity, }) @@ -68,7 +68,7 @@ func (u *unifiedPolicies) addPolicy(fromNode route.Vertex, // addGraphPolicies adds all policies that are known for the toNode in the // graph. -func (u *unifiedPolicies) addGraphPolicies(g routingGraph) error { +func (u *nodeEdgeUnifier) addGraphPolicies(g routingGraph) error { cb := func(channel *channeldb.DirectedChannel) error { // If there is no edge policy for this candidate node, skip. // Note that we are searching backwards so this node would have @@ -77,7 +77,7 @@ func (u *unifiedPolicies) addGraphPolicies(g routingGraph) error { return nil } - // Add this policy to the unified policies map. + // Add this policy to the corresponding edgeUnifier. u.addPolicy( channel.OtherNode, channel.InPolicy, channel.Capacity, ) @@ -89,16 +89,16 @@ func (u *unifiedPolicies) addGraphPolicies(g routingGraph) error { return g.forEachNodeChannel(u.toNode, cb) } -// unifiedPolicyEdge is the individual channel data that is kept inside an -// unifiedPolicy object. -type unifiedPolicyEdge struct { +// unifiedEdge is the individual channel data that is kept inside an edgeUnifier +// object. +type unifiedEdge struct { policy *channeldb.CachedEdgePolicy capacity btcutil.Amount } // amtInRange checks whether an amount falls within the valid range for a // channel. -func (u *unifiedPolicyEdge) amtInRange(amt lnwire.MilliSatoshi) bool { +func (u *unifiedEdge) amtInRange(amt lnwire.MilliSatoshi) bool { // If the capacity is available (non-light clients), skip channels that // are too small. if u.capacity > 0 && @@ -122,33 +122,32 @@ func (u *unifiedPolicyEdge) amtInRange(amt lnwire.MilliSatoshi) bool { return true } -// unifiedPolicy is the unified policy that covers all channels between a pair -// of nodes. -type unifiedPolicy struct { - edges []*unifiedPolicyEdge +// edgeUnifier is an object that covers all channels between a pair of nodes. +type edgeUnifier struct { + edges []*unifiedEdge localChan bool } -// getPolicy returns the optimal policy to use for this connection given a +// getEdge returns the optimal unified edge to use for this connection given a // specific amount to send. It differentiates between local and network // channels. -func (u *unifiedPolicy) getPolicy(amt lnwire.MilliSatoshi, - bandwidthHints bandwidthHints) *unifiedPolicyEdge { +func (u *edgeUnifier) getEdge(amt lnwire.MilliSatoshi, + bandwidthHints bandwidthHints) *unifiedEdge { if u.localChan { - return u.getPolicyLocal(amt, bandwidthHints) + return u.getEdgeLocal(amt, bandwidthHints) } - return u.getPolicyNetwork(amt) + return u.getEdgeNetwork(amt) } -// getPolicyLocal returns the optimal policy to use for this local connection -// given a specific amount to send. -func (u *unifiedPolicy) getPolicyLocal(amt lnwire.MilliSatoshi, - bandwidthHints bandwidthHints) *unifiedPolicyEdge { +// getEdgeLocal returns the optimal unified edge to use for this local +// connection given a specific amount to send. +func (u *edgeUnifier) getEdgeLocal(amt lnwire.MilliSatoshi, + bandwidthHints bandwidthHints) *unifiedEdge { var ( - bestPolicy *unifiedPolicyEdge + bestEdge *unifiedEdge maxBandwidth lnwire.MilliSatoshi ) @@ -191,19 +190,18 @@ func (u *unifiedPolicy) getPolicyLocal(amt lnwire.MilliSatoshi, } maxBandwidth = bandwidth - // Update best policy. - bestPolicy = &unifiedPolicyEdge{policy: edge.policy} + // Update best edge. + bestEdge = &unifiedEdge{policy: edge.policy} } - return bestPolicy + return bestEdge } -// getPolicyNetwork returns the optimal policy to use for this connection given -// a specific amount to send. The goal is to return a policy that maximizes the -// probability of a successful forward in a non-strict forwarding context. -func (u *unifiedPolicy) getPolicyNetwork( - amt lnwire.MilliSatoshi) *unifiedPolicyEdge { - +// getEdgeNetwork returns the optimal unified edge to use for this connection +// given a specific amount to send. The goal is to return a unified edge with a +// policy that maximizes the probability of a successful forward in a non-strict +// forwarding context. +func (u *edgeUnifier) getEdgeNetwork(amt lnwire.MilliSatoshi) *unifiedEdge { var ( bestPolicy *channeldb.CachedEdgePolicy maxFee lnwire.MilliSatoshi @@ -256,14 +254,14 @@ func (u *unifiedPolicy) getPolicyNetwork( // chance for this node pair. But this is all only needed for nodes that // have distinct policies for channels to the same peer. policyCopy := *bestPolicy - modifiedPolicy := unifiedPolicyEdge{policy: &policyCopy} - modifiedPolicy.policy.TimeLockDelta = maxTimelock + modifiedEdge := unifiedEdge{policy: &policyCopy} + modifiedEdge.policy.TimeLockDelta = maxTimelock - return &modifiedPolicy + return &modifiedEdge } // minAmt returns the minimum amount that can be forwarded on this connection. -func (u *unifiedPolicy) minAmt() lnwire.MilliSatoshi { +func (u *edgeUnifier) minAmt() lnwire.MilliSatoshi { min := lnwire.MaxMilliSatoshi for _, edge := range u.edges { if edge.policy.MinHTLC < min { diff --git a/routing/unified_policies_test.go b/routing/unified_edges_test.go similarity index 72% rename from routing/unified_policies_test.go rename to routing/unified_edges_test.go index 8c8d9bbd8..f1c86abe9 100644 --- a/routing/unified_policies_test.go +++ b/routing/unified_edges_test.go @@ -8,16 +8,16 @@ import ( "github.com/lightningnetwork/lnd/routing/route" ) -// TestUnifiedPolicies tests the composition of unified policies for nodes that +// TestNodeEdgeUnifier tests the composition of unified edges for nodes that // have multiple channels between them. -func TestUnifiedPolicies(t *testing.T) { +func TestNodeEdgeUnifier(t *testing.T) { source := route.Vertex{1} toNode := route.Vertex{2} fromNode := route.Vertex{3} bandwidthHints := &mockBandwidthHints{} - u := newUnifiedPolicies(source, toNode, nil) + u := newNodeEdgeUnifier(source, toNode, nil) // Add two channels between the pair of nodes. p1 := channeldb.CachedEdgePolicy{ @@ -39,13 +39,12 @@ func TestUnifiedPolicies(t *testing.T) { u.addPolicy(fromNode, &p1, 7) u.addPolicy(fromNode, &p2, 7) - checkPolicy := func(unifiedPolicy *unifiedPolicyEdge, - feeBase lnwire.MilliSatoshi, feeRate lnwire.MilliSatoshi, - timeLockDelta uint16) { + checkPolicy := func(edge *unifiedEdge, feeBase lnwire.MilliSatoshi, + feeRate lnwire.MilliSatoshi, timeLockDelta uint16) { t.Helper() - policy := unifiedPolicy.policy + policy := edge.policy if policy.FeeBaseMSat != feeBase { t.Fatalf("expected fee base %v, got %v", @@ -63,31 +62,31 @@ func TestUnifiedPolicies(t *testing.T) { } } - policy := u.policies[fromNode].getPolicy(50, bandwidthHints) - if policy != nil { + edge := u.edgeUnifiers[fromNode].getEdge(50, bandwidthHints) + if edge != nil { t.Fatal("expected no policy for amt below min htlc") } - policy = u.policies[fromNode].getPolicy(550, bandwidthHints) - if policy != nil { + edge = u.edgeUnifiers[fromNode].getEdge(550, bandwidthHints) + if edge != nil { t.Fatal("expected no policy for amt above max htlc") } // For 200 sat, p1 yields the highest fee. Use that policy to forward, // because it will also match p2 in case p1 does not have enough // balance. - policy = u.policies[fromNode].getPolicy(200, bandwidthHints) + edge = u.edgeUnifiers[fromNode].getEdge(200, bandwidthHints) checkPolicy( - policy, p1.FeeBaseMSat, p1.FeeProportionalMillionths, + edge, p1.FeeBaseMSat, p1.FeeProportionalMillionths, p1.TimeLockDelta, ) // For 400 sat, p2 yields the highest fee. Use that policy to forward, // because it will also match p1 in case p2 does not have enough // balance. In order to match p1, it needs to have p1's time lock delta. - policy = u.policies[fromNode].getPolicy(400, bandwidthHints) + edge = u.edgeUnifiers[fromNode].getEdge(400, bandwidthHints) checkPolicy( - policy, p2.FeeBaseMSat, p2.FeeProportionalMillionths, + edge, p2.FeeBaseMSat, p2.FeeProportionalMillionths, p1.TimeLockDelta, ) }