From 50949b7b9ca2742501870baebaba715d07892119 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 23 Aug 2024 09:03:43 +0930 Subject: [PATCH] askrene: hack in some padding so we don't overflow capacities. Of course, we still will, since spendable is for a single HTLC, but this also shows why we should treat *minimum* as the incorrect answer if they cross, too. Signed-off-by: Rusty Russell Fixes: https://github.com/ElementsProject/lightning/issues/7563 --- plugins/askrene/mcf.c | 12 ++++++++++-- tests/test_askrene.py | 21 ++++++++++----------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index ef96c01a654a..d0dd7333b22c 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -453,9 +453,17 @@ static void linearize_channel(const struct pay_parameters *params, /* This takes into account any payments in progress. */ get_constraints(params->rq, c, dir, &mincap, &maxcap); - /* Assume if min > max, max is wrong */ + + /* We seem to have some rounding error (perhaps due to our use + * of sats and fee interactions?). Since it's unusual to see + * a large unmber of flows, even if each overflows by 1 sat, + * 5 sats should be plenty. */ + if (!amount_msat_sub(&maxcap, maxcap, AMOUNT_MSAT(5000))) + maxcap = AMOUNT_MSAT(0); + + /* Assume if min > max, min is wrong */ if (amount_msat_greater(mincap, maxcap)) - maxcap = mincap; + mincap = maxcap; u64 a = mincap.millisatoshis/1000, /* Raw: linearize_channel */ b = 1 + maxcap.millisatoshis/1000; /* Raw: linearize_channel */ diff --git a/tests/test_askrene.py b/tests/test_askrene.py index e48eca229c79..e68fe1831339 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -169,8 +169,8 @@ def test_getroutes(node_factory): amount_msat=1000, layers=[], maxfee_msat=1000, - final_cltv=99) == {'probability_ppm': 999999, - 'routes': [{'probability_ppm': 999999, + final_cltv=99) == {'probability_ppm': 999998, + 'routes': [{'probability_ppm': 999998, 'final_cltv': 99, 'amount_msat': 1000, 'path': [{'short_channel_id': '0x1x0', @@ -184,8 +184,8 @@ def test_getroutes(node_factory): amount_msat=100000, layers=[], maxfee_msat=5000, - final_cltv=99) == {'probability_ppm': 999798, - 'routes': [{'probability_ppm': 999798, + final_cltv=99) == {'probability_ppm': 999797, + 'routes': [{'probability_ppm': 999797, 'final_cltv': 99, 'amount_msat': 100000, 'path': [{'short_channel_id': '0x1x0', @@ -247,11 +247,11 @@ def test_getroutes(node_factory): 10000000, [[{'short_channel_id': '0x2x1', 'next_node_id': nodemap[2], - 'amount_msat': 500000, + 'amount_msat': 505000, 'delay': 99 + 6}], [{'short_channel_id': '0x2x3', 'next_node_id': nodemap[2], - 'amount_msat': 9500009, + 'amount_msat': 9495009, 'delay': 99 + 6}]]) @@ -313,8 +313,8 @@ def test_getroutes_auto_sourcefree(node_factory): amount_msat=1000, layers=['auto.sourcefree'], maxfee_msat=1000, - final_cltv=99) == {'probability_ppm': 999999, - 'routes': [{'probability_ppm': 999999, + final_cltv=99) == {'probability_ppm': 999998, + 'routes': [{'probability_ppm': 999998, 'final_cltv': 99, 'amount_msat': 1000, 'path': [{'short_channel_id': '0x1x0', @@ -328,8 +328,8 @@ def test_getroutes_auto_sourcefree(node_factory): amount_msat=100000, layers=['auto.sourcefree'], maxfee_msat=5000, - final_cltv=99) == {'probability_ppm': 999798, - 'routes': [{'probability_ppm': 999798, + final_cltv=99) == {'probability_ppm': 999797, + 'routes': [{'probability_ppm': 999797, 'final_cltv': 99, 'amount_msat': 100000, 'path': [{'short_channel_id': '0x1x0', @@ -451,7 +451,6 @@ def test_fees_dont_exceed_constraints(node_factory): assert amount <= max_msat -@pytest.mark.xfail(strict=True) def test_live_spendable(node_factory, bitcoind): """Test we don't exceed spendable limits on a real network on nodes""" l1, l2, l3 = node_factory.get_nodes(3)