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>
We allow adding them, but crash when we remove the localmods. Yet
this could theoretically happen if a channel we modified was removed
from the gossmap, anyway.
Reported-by: Lagrang3 <lagrang3@protonmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I like the clarity, but this is a hot path. Fortunately these arrays
have very well defined lengths.
Before: 5.81 seconds
After: 1.06 seconds
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We only ever visit each node once, so we can just use an array. This
avoids calling tal() all the time, which is *especially* slow when we're
memory tracking.
I had an old canned gossmap which I benchmarked for these (and in
particular one node was unreachable, and that was slow):
Before: 17.27 seconds
After: 5.80 seconds
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows tools to validate that it is accessing the correct hsm_secret for this node!
This is extremely important for backups: if they are using VLS, they need to back *that*
up instead, for example.
Changelog-Added: `hsmtool`: `getnodeid` command derives the node id from the hsm_secret, to verify it's the correct secret.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We bump the version to minor version 0.2 to allow ourselves to bump on
patch level when we need to fix a bug or update a dependency.
Changelog-Changed: Plugins: `cln-grpc` Upgrade tonic version and
introduce new versioning scheme.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
