This happens in the coming "real network" test!
We add fees and hit htlc_max, but don't have another flow to add to.
Rather than MCF again, we split the flow into two.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
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.
Rather than adding to the gossmap modifications directly, populate
the layer and have the normal layer application logic do it.
This is consistent when we query layers in the next patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And unify logging for better debugging.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `askrene` now has better logging, gives notifications of progress.
Multiple places in the pay lifecycle depend on mods to be set. By
setting the mods directly after the first listpeerchannels call,
subsequent calls to listpeerchannels are avoided.
Changelog-Fixed: pay-plugin: only call listpeerchannels once during a
payment lifecycle.
Since we don't compact the gossmap on the fly (FIXME!) we can
easily surpass 4GB in the gossmap, and 32 bit offsets are not
sufficient.
I'm a bit surprised we don't crash immediately, but we've definitely
seen issues.
Changelog-Fixed: gossipd: crash errors with large gossip_store (>4MB) growth on longer-running nodes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For nodes with many channels this is a tremendous improvement in pay
performance. PR #7611 improves payment performance from 15 seconds to
13.5 seconds on one of our nodes. This commit improves payment
performance from 13.5 seconds to 5.7 seconds.
Changelog-Fixed: Improved pathfinding speed for large nodes.
This minimizes the need to convert back and forth from and to sat
values, and it also removes a new instance of sats in the public
interface (`channel_hints`).
Suggested-By: Rusty Russell <@rustyrussell>
We were ignoring more reliable information because of the
stricter-is-better logic. This meant that we were also ignoring local
channel information, despite knowing better.
By simplifying the logic to pick the one with the newer timestamp we
ensure that later observations always trump earlier ones. There is a
bit of interplay between the relaxation of the constraints updating
the timestamp, and comparing to real observation timestamps, but that
should not impact us for local channels.
We were using `channel_hint` to temporarily tweak the graph inside of
a payment. However, these ad-hoc `channel_hints` are stickier than
their predecessors, in that they outlive the payment attempt itself,
and interfere with later ones.
Changelog-Changed: pay: Discarding an overly long or expensive route does not blocklist channels anymore.
If we have a large channel, fail to send through a small amount, and
then add a `channel_hint`, then it can happen that the call to
`channel_hint_set_update` is already late enough that it refills the
1msat we removed from the attempted amount, thus making the edge we
just excluded eligible again, which can lead into an infinite loop.
We slow down the updating of the channel_hints to once every
hysteresis timeout. This allows us to set tight constraints, while not
incurring in the accidental relaxation issue.
We need to know the overall channel capacity, i.e., the amount_msat
that the channel was funded with, in order to relax the channel_hint
to refill over time.
When cln is run without deprecated apis, the `listchannels` call would
still query `listpeerchannels`, even though it wouldn't use the result.
This adds unnecessary time to the call. Especially in case the node has
many channels. This commit skips the listpeerchannels call in case the
outcome won't be used.
Changelog-Fixed: Improve listchannels performance
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>
The code is a bit too complex for gcc to track it:
```
In file included from ccan/ccan/tal/str/str.h:7,
from plugins/askrene/askrene.c:11:
plugins/askrene/askrene.c: In function ‘do_getroutes’:
ccan/ccan/tal/tal.h:324:23: error: ‘routes’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
324 | #define tal_count(p) (tal_bytelen(p) / sizeof(*p))
| ^~~~~~~~~~~
plugins/askrene/askrene.c:476:24: note: ‘routes’ was declared here
476 | struct route **routes;
| ^~~~~~
plugins/askrene/askrene.c:475:29: error: ‘amounts’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
475 | struct amount_msat *amounts;
| ^~~~~~~
plugins/askrene/askrene.c:488:69: error: ‘probability’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
488 | json_add_u64(response, "probability_ppm", (u64)(probability * 1000000));
| ~~~~~~~~~~~~~^~~~~~~~~~
cc plugins/askrene/dijkstra.c
cc1: all warnings being treated as errors
```
On my local machine, it also warns in param_dev_channel, so I fixed that too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Lagrang3 points out it's less useful (when we time them out), and probably
a premature optimization anyway.
Suggested-by: Lagrang3 <lagrang3@protonmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
It was weird not to have a capacity associated with localmods channels, and
fixing it has some very nice side effects.
Now the gossmap_chan_get_capacity() call never fails (we prevented reading
of channels from gossmap in the partially-written case already), so we
make it return the capacity. We do this in msat, because that's what
all the callers want.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is actually what we want in several places: to only override one or
two fields in a channel_update.
We add a gossmap_local_setchan() with a similar API to the old
gossmap_local_updatechan(), for the case where we want to set every
field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
I got confused, as we had a struct containing two arrays. Simply expose the
reserve_hop struct and use arrays directly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
