A failing payment would doom all subsequent ones. Now we step down the
amount a single satoshi so any prior channel_hints do not doom the
payment outright.
Changelog-None
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>
