Often I find this in the logs:
DEBUG plugin-cln-renepay: gossmap ignored 94692259263600 channel updates
Apparently gossmap_refresh does not initialize the input variable num_channel_updates_rejected.
Fix this by explicitely initializing it before calling gossmap_refresh.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
routefail object allocation was linked to route,
we had a crash of the plugin with the following error:
0x561f424fc07a send_backtrace
common/daemon.c:33
0x561f424fc102 crashdump
common/daemon.c:75
0x7f5b0e7dc04f ???
???:0
0x7f5b0e82ae2c ???
???:0
0x7f5b0e7dbfb1 ???
???:0
0x7f5b0e7c6471 ???
???:0
0x561f4252581f call_error
ccan/ccan/tal/tal.c:95
0x561f425258c8 check_bounds
ccan/ccan/tal/tal.c:169
0x561f425258f9 to_tal_hdr
ccan/ccan/tal/tal.c:179
0x561f42526283 tal_free
ccan/ccan/tal/tal.c:525
0x561f424e5379 routefail_end
plugins/renepay/routefail.c:52
0x561f424e557b handle_failure
plugins/renepay/routefail.c:431
apparently there was a race condition for which the route was first
freed before we arrived to routefail_end where we manually free
routefail. I don't see how this could have happened, but anyways
this subtle bug can be avoided by linking the routefail to the payment.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
Changelog-Fixed: renepay: finalized routes have to be processed and
determine the payment status even after the payment goes into the
background (no current command active). Not doing so leads to finalized
routes getting stuck in the payment internal data structure and their
associated HTLCs in the uncertainty network don't get released.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
We rather have a payment hash table to look up payments based on the
payment hash than having a list.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
Use success_data_from_listsendpays to check if there are "complete"
sendpays instead of imposing the presence of "complete" sendpays as a
precondition for success_data_from_listsendpays.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
Simply move the "computed routes" array from the payment to the
routetracker. It makes sense to put all temporary stages of routing into
a single data structure: the routetracker.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
There were some dummy rpc calls to waitblockheight in the payment workflow
to allow the function stack to clear. But it is better if some steps in
the payment are executed "atomically" to avoid strange race conditions.
For example: all steps between getting pending sendpays, computing new
routes and sending those routes.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
It feels unsafe to rely on the internal state of the plugin's database
to tell how many pending sendpays there are for the current payment.
The safest way is to assume lightningd knows and thus use listsendpay
before computing routes.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
- remove payment pointer from routetracker, fetch payment if necessary
from payment_hash;
- "have results" condition as a function call to routetracker.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
The time of decay "TIMER_FORGET_SEC" was set to 1 hour,
which is very low, it would make the plugin try depleted channels after
just a couple of seconds for very small amounts.
I set it to 1 week ~ 6e5 seconds.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
When you have *lots* of events in your bkpr database looking up a
specific event via calling bkpr-listaccountevents and using jq or
grep to filter gets very slow (and wasteful of CPU and disk resources).
This commit adds the paremeter payment_id to the call to filter for a
specific payment id via a where clause in the request to the database of bkpr.
Changelog-Added: Plugins: Add payment_id parameter to bkpr-listaccountevents to filter events.
Did a sweep to find any others, give this from sanitizer:
```
2024-08-09T18:06:45.1729472Z plugins/bkpr/recorder.c:2057:23: runtime error: load of value 190, which is not a valid value for type 'bool'
2024-08-09T18:06:45.1729877Z SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior plugins/bkpr/recorder.c:2057:23 in
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Valgrind found this. I think two PRs were added in parallel, which is why we
only found it after merge:
```
2024-08-09T01:57:23.7375752Z ==34263== Uninitialised byte(s) found during client check request
2024-08-09T01:57:23.7376275Z ==34263== at 0x172405: memcheck_ (mem.h:247)
2024-08-09T01:57:23.7376661Z ==34263== by 0x172585: db_bind_int (bindings.c:49)
2024-08-09T01:57:23.7377086Z ==34263== by 0x126233: log_chain_event (recorder.c:2057)
2024-08-09T01:57:23.7377544Z ==34263== by 0x11BF8B: json_utxo_deposit (bookkeeper.c:1735)
2024-08-09T01:57:23.7378207Z ==34263== by 0x12BED3: ld_command_handle (libplugin.c:1847)
2024-08-09T01:57:23.7378674Z ==34263== by 0x12C649: ld_read_json_one (libplugin.c:1998)
2024-08-09T01:57:23.7379114Z ==34263== by 0x12C780: ld_read_json (libplugin.c:2018)
2024-08-09T01:57:23.7379534Z ==34263== by 0x2990CB: next_plan (io.c:60)
2024-08-09T01:57:23.7379881Z ==34263== by 0x299D21: do_plan (io.c:422)
2024-08-09T01:57:23.7380230Z ==34263== by 0x299D88: io_ready (io.c:439)
2024-08-09T01:57:23.7380585Z ==34263== by 0x29C1BC: io_loop (poll.c:455)
2024-08-09T01:57:23.7380980Z ==34263== by 0x12D439: plugin_main (libplugin.c:2230)
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When renepay starts, one of the first operations it does is to check for
pending sendpays for the same invoice. Those are added up to an internal
database to keep track of. Also the amount they deliver to the
destination is computed so that the current payment rpc call would try
to complete the payment for whatever amount remains to pay.
For an incomplete payment after calling the RPC renepay I found this in
the logs:
2024-05-22T19:44:19.853Z DEBUG plugin-cln-renepay: There are pending sendpays to this invoice. groupid = 6 delivering = 0msat, last_partid = 10
Where delivering should be the sum of the amounts delivered by pending routes.
The `pay` plugin, as well as other plugins making use of the tree-pay
executor, will now emit their observations as they see them. The
notifications are sent on the `channel_hint_updated` topic, and any
subscriber will get them.
We also added a `timestamp` to the `struct channel_hint`, as these
observations now outlive the `pay` call, and have to be attenuated /
relaxed as they age, until we can eliminate them completely (when the
restriction is equal to the structural information gathered from the
gossip).
Changelog-Added: pay: Payments now emit `channel_hint_updated` notification to share inferred balances and observations across multiple payments.
It might be nice to let the bookkeeper keep track of external accounts
as well as the internal onchain wallet? To this end, we add some new
custom notifications, which the bookkeeper will ingest and add to its
ledger.
Suggested-By: @chrisguida
Changelog-Added: PLUGINS: `bookkeeper` now listens for two custom events: `utxo_deposit` and `utxo_spend`. This allows for 3rd party plugins to send onchain coin events to the `bookkeeper`. See the new plugins/bkpr/README.md for details on how these work!
We do some fancy accounting for channel closures; since we're tagging
splice txs as closes we need to mark them as splices so we can treat them
as any other 'normal' on chain event.
We weren't properly notifying that a channel output has been spent in
the case of it being spent in a splice. This fixes the notification side
of the equation, however there's still some issues remaining for the
bookkeeper side (to come).
Changelog-Fixed: We now send a `coin_movement` notification for splice confirmations of channel funding outpoint spends.
Changelog-Added: pay: The pay plugin now checks whether we have enough spendable capacity before computing a route, returning a clear error message if we don't
This allows us to directly returnan error code based on where we
decided to abort, rather than attemtping to infer it from the parts.
Changelog-Added: pay: The pay plugin now returns better error codes
This populates information on both topology (i.e. unannounced channels) and capacity for the local node using `listpeerchannels`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This marks all channels around the source node as free (no delay, no fee). This is normally what we want, if we are calculating a path for ourselves.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In general, we should be using tmpctx unless there's a specific reason not to.
It's clear, and simplifies the code somewhat.
If tmpctx is not cleaned often enough, we can look at a per-MCF context, but this
seems like premature optimization.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We let the caller choose mu, and iterate if necessary: it can also
check its limits for fees, etc. Rationalize it to 0-100 inclusive for
human consumption.
This means we don't loop internally, and in fact there's only one
failure mode: we cannot find enough capacity.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
