Commit Graph

37 Commits

Author SHA1 Message Date
Rusty Russell
bcc8bd59c8 askrene: don't *completely* ignore fees to start.
I noticed that increasing mu a little bit sometimes made a big difference,
because by completely ignoring fees we were choosing the worst of two channels
in some cases.

Start at 1% fees; this saves a lot on initial fees in this test!

Here's the new stats on mu levels:

     96  mu=1
     90  mu=10
     41  mu=20
     30  mu=30
     24  mu=40
     19  mu=50
     22  mu=60
      8  mu=70
     95  mu=80
     19  mu=90

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `askrene` is now better at finding low-fee paths.
2024-10-15 09:58:04 +10:30
Rusty Russell
2a0f09fc2d askrene: calculate k value dynamically, using medians.
While the `k=8` value worked for the current main network tests with the
amounts in those tests, it wasn't robust across a wider range of values
(as demonstrated when other test changes broke tests!).

Time to do this properly: calculate the ratio at the time we combine them,
using median values.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-15 09:58:04 +10:30
Rusty Russell
08df93cb25 askrene: fix base fee.
I noticed this in the logs:

	plugin-cln-askrene: notify msg unusual: The flows had a fee of 151950msat, greater than max of 53697msat, retrying with mu of 10%...
	plugin-cln-askrene: notify msg unusual: The flows had a fee of 220126msat, greater than max of 53697msat, retrying with mu of 20%...

We would expect increasing mu to *reduce* the fee!

Turns out that our linear fee is a bad terrible approximation, because I
was using base_fee_penalty of 10.0.

 |
 |          /   __ <- real fee, with base: fee = base + propfee * amount.
 |         / __/
 |       _//
 |    __/
 | __/_/
 |/  _/
 | _/ <- linearized fee: fee = linear * amount
 |/
 +-----------------------------------

These cross over where linear = propfee + base / amount.  Assume we split the
payment into 10 parts, this implies that the base_fee_penalty should be 10 / amount
(this gives a slight penalty to the normal case, but that's ok).

This gives better results, too: we get down to 650099 sats in fees, vs 801613
before.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-15 09:58:04 +10:30
Rusty Russell
6273adbe47 askrene: calculate prob_cost_factor using ratio of typical mainnet channel.
During "test_real_data", then only successes with reduced fees were 92 on "mu=10", and only
1 on "mu=30": the rest went to mu=100 and failed.

I tried numerous approaches, and in the end, opted for the simplest:

The typical range of probability costs looks likes:
	min = 0, max = 924196240, mean = 10509.4, stddev = 1.9e+06

The typical range of linear fee costs looks like:
	min = 0, max = 101000000, mean = 81894.6, stddev = 2.6e+06

This implies a k factor of 8 makes the two comparable.

This makes the two numbers comparable, and thus makes "mu" much more
effective.  Here are the number of different mu values we succeeded at:

     87  mu=0
     90  mu=10
     42  mu=20
     24  mu=30
     17  mu=40
     19  mu=50
     19  mu=60
     11  mu=70
     95  mu=80
     19  mu=90

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-15 09:58:04 +10:30
Rusty Russell
83eee64fda pytest: test askrene with worse maxfee argument.
We ask it again, but reduce fees by 1msat from the previous answer.
This is really nasty, as it frequently exercises the case where we
only go over fee when we do the refinement step.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-15 09:58:04 +10:30
Rusty Russell
55fc7fc2e5 pytest: test askrene on real network data.
I checked the failures, they seem real.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-15 09:58:04 +10:30
Rusty Russell
1b82a3ad5b askrene: constrain to exact htlc_min/htlc_max values.
The fp16_t values are approximations (overestimate for htlc_max,
underestimate for htlc_min), so in the refinement step we should use
the exact values.

This also fixes a logic bug: flow_remaining_capacity returned the
total capacity, not the additional capacity!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `askrene` now honors exact htlc_maximum_msat limits.
2024-10-15 09:58:04 +10:30
Rusty Russell
22e7a57557 askrene: make auto.sourcefree a real layer, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `getroutes` now applies `auto.sourcefree` layer in the order specified, so doesn't alter channels changed in later layers.
2024-10-15 09:58:04 +10:30
Rusty Russell
b8acd3b37c askrene: more code tweaks on feedback from Lagrang3.
1. describe_disabled should point out if node itself is disabled.
2. Hoist constraint check for neater if branching.
3. Use amount_msat_max/min for greater clarity.
4. Simply disable channels, don't zero htlc_min/max when node disabled.

I also fixed the diagnostic of htlc_max correctly, which removes a FIXME.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
630ec6a566 askrene: give better feedback when we can't find a suitable route.
This turns out to be critical for users: also stops them from
bothering us when their node is offline or has insufficient capacity!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
c307b77d2f askrene: split askrene-create-channel into create-channel and update-channel.
This allows for explicit partial updates to channels (e.g. just change
fees, or just disable) without haveing to set the other fields.

This generalizes askrene-disable-channel, which is removed.

We also take the chance to use the proper BOLT 7 terms in the API:

- htlc_minimum_msat
- htlc_maximum_msat
- cltv_expiry_delta
- fee_base_msat
- fee_proportional_millionths

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
154ad585ec pytest: enhance test_getroutes_auto_sourcefree with same case *without* auto.sourcefree.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
321ec0875f askrene: rework constraints to exist in pairs.
This is a bit more efficient, but moreover the JSONRPC API is more
logical this way.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
3c5c22b17a askrene: change inform interface, take into account reserve.
Lagrang3 points out that if we hit a maximum, we should take into account
the reserve.  This is true, but it's hard for the caller to do, so change
the API to be slightly higher level.

Tell "inform" what happened, and it adjust the constraints appropriately.
This makes the least assumptions possible (a reserve does *not* mean that
the capacity was actually used at that time).

We also add a mode to say "this succeeded": for now this does nothing,
but it could reduce both min/max capacities, and add capacity in the
other direction.  This is useful for future payments, but not as useful
for the current one.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
d50838b60f askrene: implement listreservations
And actually write tests!

Suggested-by: Lagrang3 <lagrang3@protonmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
b88f4cb854 askrene: askrene-create-layer and askrene-remove-layer.
It's generally better to be explicit with these things: currently typos
would be ignored.  But it's also much easier to clean up entire layers
as we use them for temporary (per-payment) effects.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Rusty Russell
29cc227a53 askrene: use short_channel_id_dir in API.
It's generally much more convenient, and it's already present in
other APIs.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-10-04 11:27:53 +09:30
Lagrang3
33404b03a0 add askrene-disable-channel
Changelog-EXPERIMENTAL: askrene: add askrene-disable-channel RPC
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
2024-10-04 11:27:53 +09:30
Rusty Russell
9b60f6cc6d askrene: re-check min_htlc violations after correcting for MCF rounding.
Thanks to @Lagrang3 for spotting this!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
9d966b46a7 askrene: take into account the reduction in "spendable" with additional HTLCs.
"spendable" is for a single HTLC: if we own the channel, this amount
decreases with every HTLC, as we have to pay fees.  We have access to this since
we call listpeerchannels anyway, so we can calculate the additional costs and
use it in the refining phase.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
3e96955c72 pytest: test askrene treats htlc_maximum_msat and htlc_minimum_msat restrictions properly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
f0331cd82e askrene: add a "refining" step to add fees and handle corner cases.
This is the root cause of the problem worked around in 50949b7b9c
"askrene: hack in some padding so we don't overflow capacities."

When adding fees to flows, we didn't recheck the boundary conditions: in
renepay this is done by routebuilder.

Fortunately, we can use our "reservations" infrastructure to temporarily
use capacity as we process flows, so we handle the cases where they are
not independent correclty.

My assumption is that the resulting errors are small, so we divide
them between the remaining flows based on highest-to-least
probability.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
1b8a64f5e6 askrene: apply "auto.sourcefree" to channels created in layers, too.
We had a workaround for channels added by "auto.local", but instead we
should make it work properly.

I didn't do this before because we can't manipulate the localmods while
they're applied, but it's simple to do it in two stages.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
06957dc832 pytest: separate out routine which checks only some fields of getroutes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
e90be8d957 pyln-testing: add gossip_store_file arg to get_node()
This makes it much easier to use generated gossip_store files.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
5ea049ea12 pytest: duplicate test_live_spendable as a canned gossmap test.
Not quite the same, as it doesn't have the "auto.local" layer, but it exhibits
the same problem if we revert the fix for test_live_spendable.

And it's much faster!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
b11642538e pytest: ensure there are no duplicate paths in test_live_spendable, and check total.
There aren't, but make sure.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-09-19 12:16:53 +09:30
Rusty Russell
6150d1b3fa patch general-node-id-assign.patch 2024-09-19 12:16:53 +09:30
Rusty Russell
50949b7b9c 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 <rusty@rustcorp.com.au>
Fixes: https://github.com/ElementsProject/lightning/issues/7563
2024-08-23 18:52:15 +09:30
Rusty Russell
af011f9dac pytest: make spendable test for askrene work properly, give a better name.
Based on great test case from https://github.com/daywalker90

```
E       AssertionError: assert {'107x2x0/1': 'Path total 285720859 > spendable 285718000', '108x1x0/1': 'Path total 384721849 > spendable 384718000'} == {}
E         Left contains 2 more items:
E         {'107x2x0/1': 'Path total 285720859 > spendable 285718000',
E          '108x1x0/1': 'Path total 384721849 > spendable 384718000'}
E         Full diff:
E           {
E         -  ,
E         +  '107x2x0/1': 'Path total 285720859 > spendable 285718000',
E         +  '108x1x0/1': 'Path total 384721849 > spendable 384718000',
E           }
```

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-23 18:52:15 +09:30
Rusty Russell
b99fd02393 pytest: add spendable tests for askrene.
Make sure we're not exceeding the spendable amount of a local channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-19 10:12:51 -07:00
Rusty Russell
7fb7234da1 askrene: change finalcltv to final_cltv, and return it in response.
You need to know it to make an onion, and in theory if we decided to
fuzz it could be different for different paths.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-19 10:12:51 -07:00
Rusty Russell
f8b259d5e9 askrene: add "auto.localchans" layer.
This populates information on both topology (i.e. unannounced channels) and capacity for the local node using `listpeerchannels`.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-07 20:35:30 +09:30
Rusty Russell
296b3ce20c plugin/askrene: add "auto.sourcefree" layer.
This marks all channels around the source node as free (no delay, no fee).  This is normally what we want, if we are calculating a path for ourselves.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-07 20:35:30 +09:30
Rusty Russell
de19524b6d pytest: simple getroutes tests.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-07 20:35:30 +09:30
Rusty Russell
45814bf8ac plugins/askrene: attach getroutes call to MCF code.
Now getroutes actually does something!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-07 20:35:30 +09:30
Rusty Russell
22a32e6e09 pytest: test file for askrene.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
2024-08-07 20:35:30 +09:30