Skip to content

Commit

Permalink
askrene: change getroutes amount and delay values
Browse files Browse the repository at this point in the history 
In the previous implementation getroutes would provide for every i-th
hop the value of the amount and delay which correspond to the (i-1)-th
HTLC. This commit changes that, instead the i-th hop contains the value
of the amount and delay of the i-th HTLC.

Changelog-Changed: askrene: getroutes outputs contains for each hop the correct amount and delay values.

Signed-off-by: Lagrang3 <[email protected]>
  • Loading branch information
@Lagrang3
Lagrang3 committed Feb 8, 2025
1 parent 3f2b490 commit 66ed456
Show file tree
Hide file tree
Showing 5 changed files with 133 additions and 47 deletions.
2 changes: 1 addition & 1 deletion contrib/msggen/msggen/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16190,7 +16190,7 @@
"delay": {
"type": "u32",
"description": [
"The total CLTV expected by the node at the start of this hop."
"The total CLTV on this hop's HTLC."
]
}
}
Expand Down
2 changes: 1 addition & 1 deletion doc/schemas/lightning-getroutes.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
"delay": {
"type": "u32",
"description": [
"The total CLTV expected by the node at the start of this hop."
"The total CLTV on this hop's HTLC."
]
}
}
Expand Down
4 changes: 2 additions & 2 deletions plugins/askrene/askrene.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,8 @@ 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;
rh->delay = delay;
if (!amount_msat_add_fee(&msat, h->base_fee, h->proportional_fee))
plugin_err(rq->plugin, "Adding fee to amount");
delay += h->delay;
Expand All @@ -534,8 +536,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;
rq_log(tmpctx, rq, LOG_INFORM, "Flow %zu/%zu: %s",
Expand Down
18 changes: 5 additions & 13 deletions plugins/xpay/xpay.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,12 +127,8 @@ struct hop {
struct pubkey next_node;
/* Via this channel */
struct short_channel_id_dir scidd;
/* This is amount the node needs (including fees) */
struct amount_msat amount_in;
/* ... to send this amount */
struct amount_msat amount_out;
/* This is the delay, including delay across node */
u32 cltv_value_in;
/* This is the delay, out from node. */
u32 cltv_value_out;
};
Expand Down Expand Up @@ -347,14 +343,14 @@ static struct amount_msat initial_sent(const struct attempt *attempt)
{
if (tal_count(attempt->hops) == 0)
return attempt->delivers;
return attempt->hops[0].amount_in;
return attempt->hops[0].amount_out;
}

static u32 initial_cltv_delta(const struct attempt *attempt)
{
if (tal_count(attempt->hops) == 0)
return attempt->payment->final_cltv;
return attempt->hops[0].cltv_value_in;
return attempt->hops[0].cltv_value_out;
}

/* The current attempt is the first to succeed: we assume all the ones
Expand Down Expand Up @@ -776,7 +772,7 @@ static struct amount_msat total_fees_being_sent(const struct payment *payment)
if (tal_count(attempt->hops) == 0)
continue;
if (!amount_msat_sub(&fee,
attempt->hops[0].amount_in,
attempt->hops[0].amount_out,
attempt->delivers))
abort();
if (!amount_msat_accumulate(&sum, fee))
Expand Down Expand Up @@ -1053,16 +1049,12 @@ static struct command_result *getroutes_done(struct command *aux_cmd,
",delay:%}",
JSON_SCAN(json_to_short_channel_id_dir,
&hop->scidd),
JSON_SCAN(json_to_msat, &hop->amount_in),
JSON_SCAN(json_to_msat, &hop->amount_out),
JSON_SCAN(json_to_pubkey, &hop->next_node),
JSON_SCAN(json_to_u32, &hop->cltv_value_in));
JSON_SCAN(json_to_u32, &hop->cltv_value_out));
if (err)
plugin_err(aux_cmd->plugin, "Malformed routes: %s",
err);
if (j > 0) {
hops[j-1].amount_out = hop->amount_in;
hops[j-1].cltv_value_out = hop->cltv_value_in;
}
}
hops[j-1].amount_out = delivers;
hops[j-1].cltv_value_out = payment->final_cltv;
Expand Down
154 changes: 124 additions & 30 deletions tests/test_askrene.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import subprocess
import time
import tempfile
import shutil


def direction(src, dst):
Expand Down Expand Up @@ -476,8 +477,8 @@ def test_getroutes(node_factory):
'amount_msat': 1000,
'path': [{'short_channel_id_dir': '0x1x0/1',
'next_node_id': nodemap[1],
'amount_msat': 1010,
'delay': 99 + 6}]}]}
'amount_msat': 1000,
'delay': 99}]}]}
# Two hop, still easy.
assert l1.rpc.getroutes(source=nodemap[0],
destination=nodemap[3],
Expand All @@ -490,12 +491,12 @@ def test_getroutes(node_factory):
'amount_msat': 100000,
'path': [{'short_channel_id_dir': '0x1x0/1',
'next_node_id': nodemap[1],
'amount_msat': 103020,
'delay': 99 + 6 + 6},
'amount_msat': 102000,
'delay': 99 + 6},
{'short_channel_id_dir': '1x3x2/1',
'next_node_id': nodemap[3],
'amount_msat': 102000,
'delay': 99 + 6}
'amount_msat': 100000,
'delay': 99}
]}]}

# Too expensive
Expand Down Expand Up @@ -534,8 +535,8 @@ def test_getroutes(node_factory):
'amount_msat': 1000000,
'path': [{'short_channel_id_dir': '0x2x3/1',
'next_node_id': nodemap[2],
'amount_msat': 1000001,
'delay': 99 + 6}]}]}
'amount_msat': 1000000,
'delay': 99}]}]}

# For 10000 sats, we will split.
check_getroute_paths(l1,
Expand All @@ -544,12 +545,12 @@ def test_getroutes(node_factory):
10000000,
[[{'short_channel_id_dir': '0x2x1/1',
'next_node_id': nodemap[2],
'amount_msat': 4500004,
'delay': 99 + 6}],
'amount_msat': 4500000,
'delay': 99}],
[{'short_channel_id_dir': '0x2x3/1',
'next_node_id': nodemap[2],
'amount_msat': 5500005,
'delay': 99 + 6}]])
'amount_msat': 5500000,
'delay': 99}]])


def test_getroutes_fee_fallback(node_factory):
Expand Down Expand Up @@ -617,8 +618,8 @@ def test_getroutes_auto_sourcefree(node_factory):
'amount_msat': 1000,
'path': [{'short_channel_id_dir': '0x1x0/1',
'next_node_id': nodemap[1],
'amount_msat': 1010,
'delay': 105}]}]}
'amount_msat': 1000,
'delay': 99}]}]}

# Start easy
assert l1.rpc.getroutes(source=nodemap[0],
Expand Down Expand Up @@ -650,8 +651,8 @@ def test_getroutes_auto_sourcefree(node_factory):
'delay': 99 + 6},
{'short_channel_id_dir': '1x3x2/1',
'next_node_id': nodemap[3],
'amount_msat': 102000,
'delay': 99 + 6}
'amount_msat': 100000,
'delay': 99}
]}]}

# Too expensive
Expand Down Expand Up @@ -698,8 +699,8 @@ def test_getroutes_maxdelay(node_factory):
'amount_msat': 1000,
'path': [{'short_channel_id_dir': '0x1x0/1',
'next_node_id': nodemap[1],
'amount_msat': 1010,
'delay': 179}]}]}
'amount_msat': 1000,
'delay': 99}]}]}

# But use the channel with lower delay when needed
assert l1.rpc.getroutes(source=nodemap[0],
Expand All @@ -714,8 +715,8 @@ def test_getroutes_maxdelay(node_factory):
'amount_msat': 1000,
'path': [{'short_channel_id_dir': '0x1x1/1',
'next_node_id': nodemap[1],
'amount_msat': 1010,
'delay': 139}]}]}
'amount_msat': 1000,
'delay': 99}]}]}

# Excessive maxdelay parameter
with pytest.raises(RpcError, match="maximum delay allowed is 2016"):
Expand Down Expand Up @@ -758,9 +759,9 @@ def test_getroutes_auto_localchans(node_factory):
100000,
maxfee_msat=100000,
layers=['auto.localchans'],
paths=[[{'short_channel_id_dir': scid21dir, 'amount_msat': 102012, 'delay': 99 + 6 + 6 + 6},
{'short_channel_id_dir': '0x1x0/0', 'amount_msat': 102010, 'delay': 99 + 6 + 6},
{'short_channel_id_dir': '1x2x1/1', 'amount_msat': 101000, 'delay': 99 + 6}]])
paths=[[{'short_channel_id_dir': scid21dir, 'amount_msat': 102010, 'delay': 99 + 6 + 6},
{'short_channel_id_dir': '0x1x0/0', 'amount_msat': 101000, 'delay': 99 + 6},
{'short_channel_id_dir': '1x2x1/1', 'amount_msat': 100000, 'delay': 99}]])

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


def test_fees_dont_exceed_constraints(node_factory):
Expand Down Expand Up @@ -847,7 +848,7 @@ def test_sourcefree_on_mods(node_factory, bitcoind):
final_cltv=99)['routes']
# Expect no fee.
check_route_as_expected(routes, [[{'short_channel_id_dir': '0x3x3/1',
'amount_msat': 1003000, 'delay': 117}]])
'amount_msat': 1000000, 'delay': 99}]])


def test_live_spendable(node_factory, bitcoind):
Expand Down Expand Up @@ -1013,8 +1014,8 @@ def test_max_htlc(node_factory, bitcoind):
final_cltv=10)

check_route_as_expected(routes['routes'],
[[{'short_channel_id_dir': '0x1x0/1', 'amount_msat': 1_000_001, 'delay': 10 + 6}],
[{'short_channel_id_dir': '0x1x1/1', 'amount_msat': 19_000_019, 'delay': 10 + 6}]])
[[{'short_channel_id_dir': '0x1x0/1', 'amount_msat': 1_000_000, 'delay': 10}],
[{'short_channel_id_dir': '0x1x1/1', 'amount_msat': 19_000_000, 'delay': 10}]])

# If we can't use channel 2, we fail.
l1.rpc.askrene_create_layer('removechan2')
Expand Down Expand Up @@ -1362,8 +1363,8 @@ def test_askrene_fake_channeld(node_factory, bitcoind):
# delay and amount_msat for sendpay are amounts at *end* of hop, not start!
with_end = r['path'] + [{'amount_msat': r['amount_msat'], 'delay': r['final_cltv']}]
for n, h in enumerate(paths[i]):
h['delay'] = with_end[n + 1]['delay']
h['amount_msat'] = with_end[n + 1]['amount_msat']
h['delay'] = with_end[n]['delay']
h['amount_msat'] = with_end[n]['amount_msat']

l1.rpc.sendpay(paths[i], hash_hex,
amount_msat=AMOUNT,
Expand Down Expand Up @@ -1398,3 +1399,96 @@ def test_askrene_fake_channeld(node_factory, bitcoind):
'constrained')
else:
raise err


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=[],
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

0 comments on commit 66ed456

Please sign in to comment.