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>
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
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>
It's an output field (which we don't complain about), not an input field!
Fixes: https://github.com/ElementsProject/lightning/issues/7652
Changelog-Fixed: Logging: removed bogus "**BROKEN** plugin-topology: DEPRECATED API USED: listchannels.include_private" message.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids globals (and means memleak traverses the variables!): we
only change over the test plugin though, to avoid unnecessary churn.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
is_local, ie. "is this side of the channel ours?" is not needed since we
can determine that predicate by evaluating
`scidd->dir == node_id_idx(self, peer)`
Add spendable/receivable and is_local fields to the callback function
used in gossmods_from_listpeerchannels. This allows to do more fine
grained use of the listpeerchannels call.
The first use case is renepay, for which we need to ignore the htlc_max
of the local channels only.
It's a u64, we should pass by copy. This is a big sweeping change,
but mainly mechanical (change one, compile, fix breakage, repeat).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This has the benefit of being shorter, as well as more reliable (you
will get a link error if we can't print it, not a runtime one!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I noticed that run-route-infloop chose some worse-looking paths after
routing was fixed, eg the second node:
Before:
Destination node, success, probability, hops, fees, cltv, scid...
02b3aa1e4ed31be83cca4bd367b2c01e39502cb25e282a9b4520ad376a1ba0a01a,1,0.991856,2,1004,40,2572260x39x0/1,2131897x45x0/0
After:
Destination node, success, probability, hops, fees, cltv, scid...
02b3aa1e4ed31be83cca4bd367b2c01e39502cb25e282a9b4520ad376a1ba0a01a,1,0.954540,3,1046,46,2570715x21x0/1,2346882x26x14/1,2131897x45x0/0
This is because although the final costs don't reflect it, routing was taking
into account local channels, and 2572260x39x0/1 has a base fee of 2970.
There's an easy fix: when we the pay plugin creates localmods for our
gossip graph, add all local channels with delay and fees equal to 0.
We do the same thing in our unit test. This improves things across
the board:
Linear success probability (when found): min-max(mean +/- stddev)
Before: 0.487040-0.999543(0.952548+/-0.075)
After: 0.486985-0.999750(0.975978+/-0.053)
Hops:
Before: 1-5(2.98374+/-0.77)
After: 1-5(2.09593+/-0.63)
Fees:
Before: 0-50848(922.457+/-2.7e+03)
After: 0-50041(861.621+/-2.7e+03)
Delay (blocks):
Before: 0-196(65.8081+/-60)
After: 0-190(60.3285+/-60)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `pay` route algorithm doesn't bias against our own "expensive" channels any more.
gossip_store_del - takes a gossmap-style offset-of-msg not offset-of-hdr.
gossip_store_flag: set an arbitrary flag on a gossip_store hdr.
gossip_store_get_timestamp/gossip_store_set_timestamp: access gossip_store hdr.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The only way you'll see private channel_updates is if you put them
there yourself with localmods.
I also renamed the confusing gossmap_chan_capacity to gossmap_chan_has_capacity.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: `listchannels` no longer uses local knowledge to set `active` to false if disconnected.
This alters a few remaining tests, as well.
[ Inclused even more test fixes from Alex Myers <alex@endothermic.dev>! ]
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: JSON-RPC: `listchannels` listing private channels: use listpeerchannels
This breaks our tests a bit, which assumed we can always see ourselves
even if we don't have a proper channel.
Usually we would deprecate this first, but it's unlikely to break
anyone since it's a bit obscure that this worked at all.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `listnodes` no longer shows private (peer) nodes: use listpeers
We temporarily use a second gossmap so we can just switch private info off
for listincoming and not listchannels.
Note that listchannels now uses the local alias (if no scid), so we have
to change that in the routehint caller.
Since we now *always* use a channel alias in hints if one exists, a
test broke, so fix that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have multiple different routines here, and we want to wean them off private
gossip one at a time. This converts `getroute`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is more thorough than the minimal one required for getroute(), including the feerates
and cltv deltas.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This breaks Dijkstra, which is presumably why it was actually
disabled. Remove the code altoghether, instead.
Changelog-Fixed: JSON-RPC: `getroute` now documents that it ignores `fuzzpercent`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have a report that LND said our (unannounced) channel was disabled, so we didn't
use it for routehints. We're better off ignoring that in this case (if the peer is
actually not connected, the routehint code will check that and ignore anyway).
Fixes: #6555
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: pay: use channels in routehints even if peer says they're "disabled" (LND compat)
Update gossip routiens and various other hecks on the channel state to consider AWAITING_SPLICE to be routable and treated similar to CHANNELD_NORMAL.
Small updates to psbt interface
Changelog-None
It's a core concept in the spec which isn't directly exposed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listchannels` added a `direction` field (0 or 1) as per gossip specification.
local_connected allocates a struct node_map off of the struct
listchannels_opts but fails to ever release the internal table, so those
table allocations hang around forever. Add node_map_clear as a
destructor for the struct node_map to ensure that the internal table is
freed when the enclosing struct is freed.
Changelog-Fixed: topology: Fixed memleak in `listchannels`
Signed-off-by: Matt Whitlock <c-lightning@mattwhitlock.name>
This is needed for offers to generate blinded paths.
No documentation changes since listincoming is an undocumented
internal hack interface which topology presents for production
of routehints.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was deprecated in v0.10.1, but only one channel on the network
doesn't set it now anyway, and we'll be ignoring that soon.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have them split over common/param.c, common/json.c,
common/json_helpers.c, common/json_tok.c and common/json_stream.c.
Change that to:
* common/json_parse (all the json_to_xxx routines)
* common/json_parse_simple (simplest the json parsing routines, for cli too)
* common/json_stream (all the json_add_xxx routines)
* common/json_param (all the param and param_xxx routines)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is consistent with our output changes, and increases consistency.
It also keeps future sanity checks happy, that we only use JSON msat
helpers with '_msat' fields.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `invoice`, `sendonion`, `sendpay`, `pay`, `keysend`, `fetchinvoice`, `sendinvoice`: `msatoshi` argument is now called `amount_msat` to match other fields.
Changelog-Deprecated: JSON-RPC: `invoice`, `sendonion`, `sendpay`, `pay`, `keysend`, `fetchinvoice`, `sendinvoice` `msatoshi` (use `amount_msat`)
Requiring the caller to allocate them is ugly, and differs from
other types.
This means we need a context arg if we don't have one already.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
