The "path_score" callback was supposed to evaluate the *entire path*,
but that was counter-intuitive and opened the door to a cost function
bug which caused this path cost to be less than the closer path.
In particular, the capacity bias code didn't understand this at all.
1. Rename the function to `channel_score` and remove the "distance"
parameter (always "1" since you're supposed to be evaluating a
single hop).
2. Rename "cost" to the more specific "fee": "score" is our
actual cost function result (we avoid the word "cost" as it
may get confused with satoshi amounts).
3. For capacity biassing, we do want to know the amount, but
explicitly hand that as a separate parameter "total".
4. Fix a minor bug where total handed to scoring function previously
included channel fee (this is wrong: fee is paid before sending into
channel).
5. Remove the now-unused total_delay member from the dijkstra
struct.
Here are the results of our test now (routing 4194303 msat, which
didn't crash the old code, so we could compare). In both cases
we could find routes to 615 nodes:
Linear success probability (when found): min-max(mean +/- stddev)
Before: 0.484764-0.999750(0.9781+/-0.049)
After: 0.487040-0.999543(0.952548+/-0.075)
Hops:
Before: 1-5(2.13821+/-0.66)
After: 1-5(2.98374+/-0.77)
Fees:
Before: 0-50041(2173.75+/-5.3e+03)
After: 0-50848(922.457+/-2.7e+03)
Delay (blocks):
Before: 0-294(83.1642+/-68)
After: 0-196(65.8081+/-60)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: https://github.com/ElementsProject/lightning/issues/7092
Changelog-Fixed: Plugins: `pay` would occasionally crash on routing.
Changelog-Fixed: Plugins: `pay` route algorithm fixed and refined to balance fees and capacity far better.
The amount is set not to crash by default, but run
"common/test/run-route-infloop 8388607" and you'll see a crash.
Sorry about the 7MB blob, but this testing was quite revealing and
I consider it worth adding.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Wrote a test program which passed num_channel_updates_rejected as NULL
(which we don't usually do), and valgrind complained:
```
==1048302== Conditional jump or move depends on uninitialised value(s)
==1048302== at 0x118B90: update_channel (gossmap.c:550)
==1048302== by 0x119EEE: map_catchup (gossmap.c:663)
==1048302== by 0x11A299: load_gossip_store (gossmap.c:726)
==1048302== by 0x11A352: gossmap_load (gossmap.c:1052)
==1048302== by 0x125362: main (run-route-infloop.c:90)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Thanks to amazing debugging assistance from grubles, we figured out
that indeed, my memory was correct: write and mmap are not consistent
on all platforms. The easiest fix is to disable mmap on OpenBSD for now:
the better fix is to do in-place updates using the mmap, and only rely
on write() for append (which always causes a remap anyway before it's accessed).
Fixes: https://github.com/ElementsProject/lightning/issues/7109
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Increasing the min version of the hsmd due that we
added new code that required the hsmd to sign an announcements.
One of the solution is to increase the min version in this way
a signer like VLS fails directly during the init phase.
Link: https://github.com/ElementsProject/lightning/issues/7074
Changelog-None: hsmd: increase the min version
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We weakened this progressively over time, and gossip v1.5 makes spam
impossible by protocol, so we can wait until then.
Removing this code simplifies things a great deal!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Protocol: we no longer ratelimit gossip messages by channel, making our code far simpler.
We never enabled it, because we seemed to be eliminating valid
channels. We discard zombie-marked records on loading.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, allow callers to see unknown records we ignore (and let
them fail as a result), and get called if we can't pack a
channel_update into our internal format.
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>
Before this it was channeld doing it, which was tied to a particular
channel. Create an API for lightningd to sign for any channel.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And add a request schema for multifundchannel.
Changelog-Added: JSON-RPC: `fundchannel` and `multifundchannel` now take an optional `channel_type` parameter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use an array of bit numbers. We could use an array of names, but the JSON typing is then harder.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Each feature has a name, and says when deprecation begins and ends.
There's an API coming to allow you to re-enable on a per-feature basis
even if it's ended (as long as it's not been removed from the code ofc!).
Default end is 6 months after deprecation, i.e. we complain about it
at that point, if we can detect its use.
e.g, a standard deprecation in v24.05:
v24.02: allowed
v24.02 with mods: allowed
master after v24.02: allowed unless deprecated APIs disabled.
v24.05: allowed unless deprecated APIs disabled.
v24.08: allowed unless deprecated APIs disabled.
v24.11: allowed unless deprecated APIs disabled, but logs at BROKEN level.
v25.02: allowed only if --i-promise-to-fix-broken-api-user=FEATURE.
v25.05: code is actually removed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: hsmd: Added hsmd_forget_channel to enable explicit channel deletion. ([#6987])
Motivation: Previously, a signer prematurely forgetting a channel led
to failures in unresolved channel requests. This update introduces
hsmd_forget_channel, allowing nodes to explicitly notify signers when
a channel is irrevocably resolved and can be safely forgotten. This
ensures synchronized channel cleanup between nodes and signers.
This change maintains backward and forward compatibility. Nodes
explicitly check whether a signer has `WIRE_HSMD_FORGET_CHANNEL`
capability before sending the message. Nodes without
`WIRE_HSMD_FORGET_CHANNEL` capability won't send this message. Signers
capable of handling this message but not receiving it will continue to
use conservative pruning methods.
Fixes#6987
Rename the offending functions from wally_foo to cln_wally_foo.
For the sake of a minimal diff, only calls which conflict with wally
v1.0.0 have been changed. However it is bad form to use the wally_
function namespace; the remaining such calls should also be renamed.
Changelog-None
Signed-off-by: Jon Griffiths <jon_p_griffiths@yahoo.com>
The integration with opentelemetry was sub-optimal: it was generating
jaeger-style traces, with short traceIds and we were considering the
entire lifetime as a single trace. This PR changes that to a trace for
startup and then a trace for any event that doesn't already have a
parent.
We also allow using the `CLN_TRACEPARENT` envvar to attach the startup
to a remote / external trace, potentially by whatever started the main
process. This is useful to see the startup trace in the wider context
of whatever tooling is built around it.
Changelog-Added: tracing: It is now possible to inject a parent for the startup trace by setting the `CLN_TRACEPARENT` envvar
As reported by @wtogami, LND nodes are using a default
min_final_cltv_expiry_delta of 9, which makes them unable to pay invoices
using the modern spec default of 18. Forcing inclusion of the c field
allows interoperability until broader support of the 18 block default.
Fixes: #6956
Changelog-Fixed: Default bolt11 invoices are payable by LND nodes.
On July 18th, @jgriffiths wrote:
> You need to set this to NULL after freeing it, otherwise if line 72 returns you have a dangling pointer and potential later use-after-free here. Alternately use wally_psbt_set_input_final_witness(NULL) which will free any existing witness and set the value to NULL.
Reported-By: @jgriffiths
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>
Reported-by: Shahana Farooqui
Changelog-Fixed: JSON-RPC: Plugin notification `msat` fields in `invoice_payment` and `invoice_created` hooks now a number, not a string with "msat" suffix.
Changelog-Fixed: JSON-RPC: Plugin hook `payment` `msat` field is now a number, not a string with "msat" suffix.
Adds tests for when the connection fails during
1) splice tx_signature
2) splice commitment_signed
Fleshed out the reestablish flow for these two cases and implemented the fixes to make these reestablish flows work.
Part of this work required changing commit process for splices: Now we send a single commit_part for the splice where previously we sent all commits, and accordingly, we no longer revoke in response.
Changelog-Fixed: Implemented splicing restart logic for tx_signature and commitment_signed. Splice commitments are reworked in a manner incompatible with the last version.
We don't actually use this internal to this method? Weird.
Anyway, if we don't want/need it allow the caller to signal that by
passing in NULL, if desired.
