Skip to content
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
11 changes: 9 additions & 2 deletions plugins/askrene/child/mcf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1110,6 +1110,14 @@ static void init_linear_network_single_path(
if (amount_msat_greater_eq(params->amount, maxcap))
continue;

const double probability =
pickhardt_richter_probability(mincap, maxcap,
params->amount);

/* too unlikely it is not worth considering */
if (probability < 1e-6)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The cutoff seems to conflict with how xpay probes liquidity? test_sendpay_grouping in test_pay.py documents that repeated attempts use attempted_amount - 1msat, specifically to land on the edge of the channel hint learned from the previous failure (as i see docstring at test_pay.py at line 5401-5406. Isn't this the situation where the new probability is smallest and most likely to fall under 1e-6?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If a payment failed at 1000'000 sat, yes you may try again with 999'999, but the probability
of that getting through is roughly 1 in a million.
In the real network we don't do that, we split the payment and maybe try again the same route
with a smaller amount eg. ~500'000, while the rest is routed through other paths.

The sendpay_grouping test is testing listpays for repeated pay calls by abusing xpay.
The important thing here is to try the same payment several times, the X-1 is not important to the
test.

continue;

const u32 chan_id = gossmap_chan_idx(gossmap, c);

const struct gossmap_node *next =
Expand All @@ -1129,8 +1137,7 @@ static void init_linear_network_single_path(

(*arc_capacity)[arc.idx] = 1;
(*arc_prob_cost)[arc.idx] =
(-1.0) * log(pickhardt_richter_probability(
mincap, maxcap, params->amount));
(-1.0) * log(probability);

struct amount_msat fee;
if (!amount_msat_fee(&fee, params->amount,
Expand Down
10 changes: 5 additions & 5 deletions tests/test_askrene.py
Original file line number Diff line number Diff line change
Expand Up @@ -958,13 +958,13 @@ def test_getroutes_single_path(node_factory):
l1,
nodemap[1],
nodemap[2],
10000000,
9100000,
[
[
{
"short_channel_id_dir": "3x2x2/1",
"node_id_out": nodemap[2],
"amount_in_msat": 10000010,
"amount_in_msat": 9100009,
"cltv_in": 99 + 6,
}
]
Expand All @@ -991,19 +991,19 @@ def test_getroutes_single_path(node_factory):
l1,
nodemap[0],
nodemap[2],
10000000,
9100000,
[
[
{
"short_channel_id_dir": "0x1x0/1",
"node_id_out": nodemap[1],
"amount_in_msat": 10000020,
"amount_in_msat": 9100018,
"cltv_in": 99 + 6 + 6,
},
{
"short_channel_id_dir": "3x2x2/1",
"node_id_out": nodemap[2],
"amount_in_msat": 10000010,
"amount_in_msat": 9100009,
"cltv_in": 99 + 6,
},
]
Expand Down
8 changes: 3 additions & 5 deletions tests/test_pay.py
Original file line number Diff line number Diff line change
Expand Up @@ -5400,10 +5400,7 @@ def test_sendpay_grouping(node_factory, bitcoind):

We always use slightly decreasing values for the payment, in order
to avoid having to adjust the channel_hints that are being
remembered across attempts. In case of a failure the
`channel_hint` will be `attempted amount - 1msat` so use that as
the next payment's amount.

remembered across attempts.
"""
l1, l2, l3 = node_factory.line_graph(
3,
Expand All @@ -5428,8 +5425,9 @@ def test_sendpay_grouping(node_factory, bitcoind):
# After this one invocation we have one entry in `listpays`
assert(len(l1.rpc.listpays()['pays']) == 1)

# try again with a smaller amount
with pytest.raises(RpcError, match=r'Failed after 1 attempts'):
l1.rpc.xpay(inv, amount_msat='100001msat')
l1.rpc.xpay(inv, amount_msat='90000msat')

# Surprise: we should have 2 entries after 2 invocations
assert(len(l1.rpc.listpays()['pays']) == 2)
Expand Down
Loading