Skip to content
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

askrene: fixup fee computation #7639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion plugins/askrene/askrene.c
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ static const char *get_routes(const tal_t *ctx,
struct gossmap_node *far_end;
const struct half_chan *h = flow_edge(flows[i], j);

rh->amount = msat;
if (!amount_msat_add_fee(&msat, h->base_fee, h->proportional_fee))
plugin_err(plugin, "Adding fee to amount");
delay += h->delay;
Expand All @@ -447,7 +448,6 @@ static const char *get_routes(const tal_t *ctx,
rh->direction = flows[i]->dirs[j];
far_end = gossmap_nth_node(rq->gossmap, flows[i]->path[j], !flows[i]->dirs[j]);
gossmap_node_get_id(rq->gossmap, far_end, &rh->node_id);
rh->amount = msat;
rh->delay = delay;
}
(*amounts)[i] = flows[i]->delivers;
Expand Down
118 changes: 107 additions & 11 deletions tests/test_askrene.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
from fixtures import * # noqa: F401,F403
from fixtures import TEST_NETWORK
from pyln.client import RpcError
from utils import (
only_one, first_scid, GenChannel, generate_gossip_store,
sync_blockheight, wait_for
)
import pytest
import time
import os
import shutil


def test_layers(node_factory):
Expand Down Expand Up @@ -177,7 +180,7 @@ def test_getroutes(node_factory):
'path': [{'short_channel_id': '0x1x0',
'direction': 1,
'next_node_id': nodemap[1],
'amount_msat': 1010,
'amount_msat': 1000,
'delay': 99 + 6}]}]}
# Two hop, still easy.
assert l1.rpc.getroutes(source=nodemap[0],
Expand All @@ -192,12 +195,12 @@ def test_getroutes(node_factory):
'path': [{'short_channel_id': '0x1x0',
'direction': 1,
'next_node_id': nodemap[1],
'amount_msat': 103020,
'amount_msat': 102000,
'delay': 99 + 6 + 6},
{'short_channel_id': '1x3x2',
'direction': 1,
'next_node_id': nodemap[3],
'amount_msat': 102000,
'amount_msat': 100000,
'delay': 99 + 6}
]}]}

Expand Down Expand Up @@ -238,7 +241,7 @@ def test_getroutes(node_factory):
'path': [{'short_channel_id': '0x2x3',
'direction': 1,
'next_node_id': nodemap[2],
'amount_msat': 1000001,
'amount_msat': 1000000,
'delay': 99 + 6}]}]}

# For 10000 sats, we will split.
Expand All @@ -252,7 +255,7 @@ def test_getroutes(node_factory):
'delay': 99 + 6}],
[{'short_channel_id': '0x2x3',
'next_node_id': nodemap[2],
'amount_msat': 9500009,
'amount_msat': 9495000,
'delay': 99 + 6}]])


Expand Down Expand Up @@ -337,7 +340,7 @@ def test_getroutes_auto_sourcefree(node_factory):
{'short_channel_id': '1x3x2',
'direction': 1,
'next_node_id': nodemap[3],
'amount_msat': 102000,
'amount_msat': 100000,
'delay': 99 + 6}
]}]}

Expand Down Expand Up @@ -395,9 +398,9 @@ def test_getroutes_auto_localchans(node_factory):
100000,
maxfee_msat=100000,
layers=['auto.localchans'],
paths=[[{'short_channel_id': scid12, 'amount_msat': 102012, 'delay': 99 + 6 + 6 + 6},
{'short_channel_id': '0x1x0', 'amount_msat': 102010, 'delay': 99 + 6 + 6},
{'short_channel_id': '1x2x1', 'amount_msat': 101000, 'delay': 99 + 6}]])
paths=[[{'short_channel_id': scid12, 'amount_msat': 102010, 'delay': 99 + 6 + 6 + 6},
{'short_channel_id': '0x1x0', 'amount_msat': 101000, 'delay': 99 + 6 + 6},
{'short_channel_id': '1x2x1', 'amount_msat': 100000, 'delay': 99 + 6}]])

# This should get self-discount correct
check_getroute_paths(l2,
Expand All @@ -407,8 +410,8 @@ def test_getroutes_auto_localchans(node_factory):
maxfee_msat=100000,
layers=['auto.localchans', 'auto.sourcefree'],
paths=[[{'short_channel_id': scid12, 'amount_msat': 102010, 'delay': 99 + 6 + 6},
{'short_channel_id': '0x1x0', 'amount_msat': 102010, 'delay': 99 + 6 + 6},
{'short_channel_id': '1x2x1', 'amount_msat': 101000, 'delay': 99 + 6}]])
{'short_channel_id': '0x1x0', 'amount_msat': 101000, 'delay': 99 + 6 + 6},
{'short_channel_id': '1x2x1', 'amount_msat': 100000, 'delay': 99 + 6}]])


def test_fees_dont_exceed_constraints(node_factory):
Expand Down Expand Up @@ -677,3 +680,96 @@ def test_min_htlc_after_excess(node_factory, bitcoind):
layers=[],
maxfee_msat=20_000_000,
final_cltv=10)


def test_forwarding_fees(node_factory):
"""Tests if getroutes gets the fees right."""

def fees(amt, propfee, basefee):
return basefee + (amt * propfee) // 1000000

l1 = node_factory.get_node(start=False)

basefee = 7
propfee = 10000
msat = 123000
capacity = 1000000
# 0 has to use two paths (1 and 2) to reach 3. But we tell it 0->1 has limited capacity.
gsfile, nodemap = generate_gossip_store(
[
GenChannel(
0,
1,
capacity_sats=capacity,
forward=GenChannel.Half(propfee=propfee, basefee=basefee),
),
GenChannel(
1,
2,
capacity_sats=capacity,
forward=GenChannel.Half(propfee=propfee, basefee=basefee),
),
GenChannel(
2,
3,
capacity_sats=capacity,
forward=GenChannel.Half(propfee=propfee, basefee=basefee),
),
]
)

# Set up l1 with this as the gossip_store
shutil.copy(
gsfile.name, os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "gossip_store")
)
l1.start()

routes = l1.rpc.getroutes(
source=nodemap[0],
destination=nodemap[3],
amount_msat=msat,
layers=["test_layers"],
maxfee_msat=msat,
final_cltv=99,
)["routes"]
assert len(routes) == 1
assert len(routes[0]["path"]) == 3
assert routes[0]["path"][-1]["amount_msat"] >= msat
msat = routes[0]["path"][-1]["amount_msat"]
msat = msat + fees(msat, propfee, basefee)
assert routes[0]["path"][-2]["amount_msat"] >= msat
msat = routes[0]["path"][-2]["amount_msat"]
msat = msat + fees(msat, propfee, basefee)
assert routes[0]["path"][-3]["amount_msat"] >= msat


def test_forwarding_fees2(node_factory):
"""Make sure we use correct delay and fees for the direction we're going."""

def fees(amt, propfee, basefee):
return basefee + (amt * propfee) // 1000000

l1, l2, l3 = node_factory.line_graph(
3,
wait_for_announce=True,
opts=[
{},
{"fee-base": 2000, "fee-per-satoshi": 20, "cltv-delta": 20},
{"fee-base": 3000, "fee-per-satoshi": 30, "cltv-delta": 30},
],
)
msat = 123000
routes = l1.rpc.getroutes(
source=l1.info["id"],
destination=l3.info["id"],
layers=["auto.localchans"],
maxfee_msat=msat,
amount_msat=msat,
final_cltv=99,
)["routes"]
assert len(routes) == 1
assert len(routes[0]["path"]) == 2
assert routes[0]["path"][-1]["amount_msat"] >= msat
msat = routes[0]["path"][-1]["amount_msat"]
msat = msat + fees(msat, 20, 2000)
assert routes[0]["path"][-2]["amount_msat"] >= msat
Loading