Skip to content

Consider opportunity cost of local channel fees in pathfinding #7361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions lnrpc/routerrpc/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,15 @@ type Config struct {
// DefaultConfig defines the config defaults.
func DefaultConfig() *Config {
defaultRoutingConfig := RoutingConfig{
AprioriHopProbability: routing.DefaultAprioriHopProbability,
AprioriWeight: routing.DefaultAprioriWeight,
MinRouteProbability: routing.DefaultMinRouteProbability,
PenaltyHalfLife: routing.DefaultPenaltyHalfLife,
AttemptCost: routing.DefaultAttemptCost.ToSatoshis(),
AttemptCostPPM: routing.DefaultAttemptCostPPM,
MaxMcHistory: routing.DefaultMaxMcHistory,
McFlushInterval: routing.DefaultMcFlushInterval,
AprioriHopProbability: routing.DefaultAprioriHopProbability,
AprioriWeight: routing.DefaultAprioriWeight,
MinRouteProbability: routing.DefaultMinRouteProbability,
LocalOpportunityCost: routing.DefaultLocalOpportunityCost,
PenaltyHalfLife: routing.DefaultPenaltyHalfLife,
AttemptCost: routing.DefaultAttemptCost.ToSatoshis(),
AttemptCostPPM: routing.DefaultAttemptCostPPM,
MaxMcHistory: routing.DefaultMaxMcHistory,
McFlushInterval: routing.DefaultMcFlushInterval,
}

return &Config{
Expand All @@ -60,13 +61,14 @@ func DefaultConfig() *Config {
// GetRoutingConfig returns the routing config based on this sub server config.
func GetRoutingConfig(cfg *Config) *RoutingConfig {
return &RoutingConfig{
AprioriHopProbability: cfg.AprioriHopProbability,
AprioriWeight: cfg.AprioriWeight,
MinRouteProbability: cfg.MinRouteProbability,
AttemptCost: cfg.AttemptCost,
AttemptCostPPM: cfg.AttemptCostPPM,
PenaltyHalfLife: cfg.PenaltyHalfLife,
MaxMcHistory: cfg.MaxMcHistory,
McFlushInterval: cfg.McFlushInterval,
AprioriHopProbability: cfg.AprioriHopProbability,
AprioriWeight: cfg.AprioriWeight,
MinRouteProbability: cfg.MinRouteProbability,
LocalOpportunityCost: cfg.LocalOpportunityCost,
AttemptCost: cfg.AttemptCost,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why did formatting break here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure, the formatting doesn't seem to work on my machine currently

AttemptCostPPM: cfg.AttemptCostPPM,
PenaltyHalfLife: cfg.PenaltyHalfLife,
MaxMcHistory: cfg.MaxMcHistory,
McFlushInterval: cfg.McFlushInterval,
}
}
6 changes: 6 additions & 0 deletions lnrpc/routerrpc/routing_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,10 @@ type RoutingConfig struct {
// McFlushInterval defines the timer interval to use to flush mission
// control state to the DB.
McFlushInterval time.Duration `long:"mcflushinterval" description:"the timer interval to use to flush mission control state to the DB"`

// LocalOpportunityCost defines whether to consider the local fee rate
// of the first hop channel when evaluating routes. While you do not
// pay this fee since it is your channel, you might want to consider
// it in order to preserve valuale liquidity.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo "valuale" -> "valuable"

LocalOpportunityCost bool `long:"localopportunitycost" description:"whether to consider the opportunity cost of using local channel liquidity when evaluating routes"`
}
21 changes: 19 additions & 2 deletions routing/pathfind.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ var (
// DefaultAprioriHopProbability is the default a priori probability for
// a hop.
DefaultAprioriHopProbability = float64(0.6)

// DefaultLocalOpportunityCost determines whether the pathfinder
// should consider the fee rates set on its local channels when selecting
// a path.
DefaultLocalOpportunityCost = bool(false)
)

// edgePolicyWithSource is a helper struct to keep track of the source node
Expand Down Expand Up @@ -362,6 +367,10 @@ type PathFindingConfig struct {
// MinProbability defines the minimum success probability of the
// returned route.
MinProbability float64

// Whether the fee rate on local channels is considered when calculating
// the total fee for a route.
LocalOpportunityCost bool
}

// getOutgoingBalance returns the maximum available balance in any of the
Expand Down Expand Up @@ -660,7 +669,7 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
//
// Source node has no predecessor to pay a fee. Therefore set
// fee to zero, because it should not be included in the fee
// limit check and edge weight.
// limit check and edge weight.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra space

//
// Also determine the time lock delta that will be added to the
// route if fromVertex is selected. If fromVertex is the source
Expand Down Expand Up @@ -705,11 +714,19 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig,
return
}

// If this is one of our own channels and
// LocalOpportunityCost is true, then we account for the
// fee on this channel.
var opportunityCostFee lnwire.MilliSatoshi
if cfg.LocalOpportunityCost && fromVertex == source {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of adding a new block of code you could merge this logic with line 679 like this

if fromVertex != source && cfg.LocalOpportunityCost {
	fee = edge.policy.ComputeFee(amountToSend)
	timeLockDelta = edge.policy.TimeLockDelta
}

not mandatory but imo it's cleaner and the fee is set only once there

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was looking at doing something like that, but couple issues:

  • i dont think we want to edit fee because that is used to calculate totalFee on line 700 which might affect the actual amount sent, which is not what we want (because we don't actually pay this new opportunityCostFee)
  • we don't want to add the timeLockDelta of this edge to our total timelock calculation for a similar reason

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha

opportunityCostFee = edge.policy.ComputeFee(amountToSend)
}

// By adding fromVertex in the route, there will be an extra
// weight composed of the fee that this node will charge and
// the amount that will be locked for timeLockDelta blocks in
// the HTLC that is handed out to fromVertex.
weight := edgeWeight(amountToReceive, fee, timeLockDelta)
weight := edgeWeight(amountToReceive, fee + opportunityCostFee, timeLockDelta)

// Compute the tentative weight to this new channel/edge
// which is the weight from our toNode to the target node
Expand Down
91 changes: 91 additions & 0 deletions routing/pathfind_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,9 @@ func TestPathFinding(t *testing.T) {
}, {
name: "with metadata",
fn: runFindPathWithMetadata,
}, {
name: "with opportunity cost",
fn: runFindPathWithOpportunityCost,
}}

// Run with graph cache enabled.
Expand Down Expand Up @@ -970,6 +973,94 @@ func runFindLowestFeePath(t *testing.T, useCache bool) {
}
}

func runFindPathWithOpportunityCost(t *testing.T, useCache bool) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add a short godoc describing what your test checks

// Set up a test graph with two paths from deezy to target. A normal
// pathfinder should choose the path through peer-b, but when the
// LocalOpportunityCost flag is set, it should select a path through
// peer-a in order to account for the opportunity cost of using the
// deezy -> peer-b channel liquidity.
testChannels := []*testChannel{
symmetricTestChannel("deezy", "peer-a", 100000, &testChannelPolicy{
Expiry: 144,
FeeRate: 0,
MinHTLC: 1,
MaxHTLC: 100000000,
}),
symmetricTestChannel("peer-a", "target", 100000, &testChannelPolicy{
Expiry: 144,
FeeRate: 50,
MinHTLC: 1,
MaxHTLC: 100000000,
}),
symmetricTestChannel("deezy", "peer-b", 100000, &testChannelPolicy{
Expiry: 144,
FeeRate: 100,
MinHTLC: 1,
MaxHTLC: 100000000,
}),

symmetricTestChannel("peer-b", "target", 100000, &testChannelPolicy{
Expiry: 144,
FeeRate: 0,
MinHTLC: 1,
MaxHTLC: 100000000,
}),
}

ctx := newPathFindingTestContext(t, useCache, testChannels, "deezy")
ctx.pathFindingConfig = PathFindingConfig{
LocalOpportunityCost: true,
}
const (
startingHeight = 100
finalHopCLTV = 1
)

paymentAmt := lnwire.NewMSatFromSatoshis(100)
target := ctx.keyFromAlias("target")
path, err := ctx.findPath(target, paymentAmt)
require.NoError(t, err, "unable to find path")
route, err := newRoute(
ctx.source, path, startingHeight,
finalHopParams{
amt: paymentAmt,
cltvDelta: finalHopCLTV,
records: nil,
},
)
require.NoError(t, err, "unable to create path")

if route.Hops[0].PubKeyBytes != ctx.keyFromAlias("peer-a") {
t.Fatalf("expected route to pass through peer-a, "+
"but got a route through %v",
ctx.aliasFromKey(route.Hops[0].PubKeyBytes))
}

// We can then set LocalOpportunityCost to false, and we'll see that
// the peer-b will be chosen for the first hop.
ctx.pathFindingConfig = PathFindingConfig{
LocalOpportunityCost: false,
}

path, err = ctx.findPath(target, paymentAmt)
require.NoError(t, err, "unable to find path")
route, err = newRoute(
ctx.source, path, startingHeight,
finalHopParams{
amt: paymentAmt,
cltvDelta: finalHopCLTV,
records: nil,
},
)
require.NoError(t, err, "unable to create path")

if route.Hops[0].PubKeyBytes != ctx.keyFromAlias("peer-b") {
t.Fatalf("expected route to pass through peer-b, "+
"but got a route through %v",
ctx.aliasFromKey(route.Hops[0].PubKeyBytes))
}
}

func getAliasFromPubKey(pubKey route.Vertex,
aliases map[string]route.Vertex) string {

Expand Down
5 changes: 3 additions & 2 deletions server.go
Original file line number Diff line number Diff line change
Expand Up @@ -893,8 +893,9 @@ func newServer(cfg *Config, listenAddrs []net.Addr,
AttemptCost: lnwire.NewMSatFromSatoshis(
routingConfig.AttemptCost,
),
AttemptCostPPM: routingConfig.AttemptCostPPM,
MinProbability: routingConfig.MinRouteProbability,
AttemptCostPPM: routingConfig.AttemptCostPPM,
MinProbability: routingConfig.MinRouteProbability,
LocalOpportunityCost: routingConfig.LocalOpportunityCost,
}

sourceNode, err := chanGraph.SourceNode()
Expand Down