describe_scidd() was labeling any channel as "the invoice's route hint"
when there was a single-hop route hint, even if the error occurred on
an unrelated intermediate channel.
Now we check the channel actually matches the route hint's
short_channel_id before using that label.
Fixes: #8252
Changelog-Fixed: xpay: error messages no longer incorrectly label intermediate channels as "the invoice's route hint".
Fixes#6978 where bolt11 annotations were lost when sendonion failed early and payment was retried.
When sendonion RPC fails before saving payment to database, invstring_used flag would remain true, causing retry attempts to omit bolt11 parameter. Successful retries would then save to DB without bolt11 annotation.
Move invstring_used flag setting from payment_createonion_success to payment_sendonion_success. This ensures the flag is only set after sendonion actually succeeds. The bolt11 will be sent with every sendonion attempt until the first successful one, accepting the minor redundancy for cleaner state management.
Changelog-Fixed: Plugins: `listpays` can be missing the bolt11 information in some cases where `pay` is used.
The original method name was lsps-lsps2-invoice but I somehow messed it
up and renamed during a rebase.
Changelog-Changed: lsps-jitchannel is now lsps-lsps2-invoice
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
This avoids latency spikes when we ask lightningd to give us 2M entries.
tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, sqlite3):
Time (from start to end of l2 node): 88 seconds (was 95)
Worst latency: 0.028 seconds **WAS 4.5**
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we've found all the issues, the latency spike (4 seconds on my laptop)
for querying 2M elements remains.
Restore the limited sampling which we reverted, but make it 10,000 now.
This doesn't help our worst-case latency, because sql still asks for all 2M entries on
first access. We address that next.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, sqlite3):
Time (from start to end of l2 node): 102 seconds **WAS 126**
Worst latency: 4.5 seconds **WAS 5.1**
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, sqlite3):
Time (from start to end of l2 node): 135 seconds **WAS 227**
Worst latency: 12.1 seconds **WAS 62.4**
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
xpay is relying on the destructor to send another request. This means
that it doesn't actually submit the request until *next time* we wake.
This has been in xpay from the start, but it is not noticeable until
xpay stops subscribing to every command on the rpc_command hook.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This reverts commit 1dda0c0753 so we can test
what its like to be flooded with logs again.
This benefits from other improvements we've made this release, to handling
plugin input (i.e. converting to use common/jsonrpc_io), so this doesn't
make much difference.
tests/test_coinmoves.py::test_generate_coinmoves (100,000, sqlite3):
Time (from start to end of l2 node): 211 seconds
Worst latency: 108 seconds
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This reverts `bookkeeper: only read listchannelmoves 1000 entries at a time.` commit,
so we can properly fix the scalability in the coming patches.
tests/test_coinmoves.py::test_generate_coinmoves (100,000):
Time (from start to end of l2 node): 207 seconds
Worst latency: 106 seconds
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Nobody has hit this yet, but we're about to with our tests.
The size of the db is going to be whatever the total size of the tables are; bigger nodes,
bigger db.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I noticed this in the logs:
```
listinvoices: description/bolt11/bolt12 not found (
{"jsonrpc":"2)
```
And we make the same formatting mistake in several places.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This covers the other corner case, where we crash before actually
signing and sending the PSBT. We can spot this because the channel is
in AWAITING_LOCKIN and we have a PSBT, but it's not signed yet.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: offers: require peers for blinded paths to have `option_onion_messages`, due to reports of LND not forwarding our blinded payments correctly.
Somehow I missed this when deprecating `short_channel_id` being null.
Changelog-Deprecated: Plugins: `channel_state_changed` notification `message` field being `null`: it will be omitted instead.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This attempts to solve a problem we have with Phoenix clients:
This payment has been split in two many parts by the sender: 31 parts vs max 6 parts allowed for on-the-fly funding.
The problem is that we don't have any way in bolt11 or bolt12 to
specify the maximum number of HTLCs.
As a workaround, we start by restricting askrene to 6 parts if the
node is not openly reachable, and if it struggles, we remove the
restriction. This would work much better if askrene handled maxparts
more completely!
See-Also: https://github.com/ElementsProject/lightning/issues/8331
Changelog-Fixed: `xpay` will not try to send too many HTLCs through unknown channels (6, as that is Phoenix's limit) unless it has no choice
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We generate fake scids for routehints and blinded paths. But then we were
placing reservations on them as if they were global. If there are two xpays
going at once these reservations will clash, even though the same scid refers
to different channels.
Reported-by: @Lagrang3
Changelog-Fixed: xpay: fixed theoretical clash with simultanous payments via routehints and blinded paths.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have the issue of aliases: xpay uses scids like 0x0x0 for
routehints and blinded paths, and then can apply reservations to them. But
generally, reservations are *global*, so we need to differentiate.
Changelog-Added: Plugins: `askrene-reserve` and `askrene-unreserve` can take an optional `layer` inside `path` elements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Pointed out by @Lagrang3; he's right, while it's a temporary leak the
way we use flows, it's still a trap.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we simply call it at the end. We need to check it hasn't violated fee maxima, but
otherwise it's simple.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: `askrene` now handles limits on number of htlcs much more gracefully.
We added _noidx versions of the sort functions, but now they're the only ones, we can
rename them to the old names.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't need to convert to strings, we can compare directly. This removes the final
use of the index arrays.
This of course changes the order of returned routes, which alters test_real_biases, since
that biases against the final channel in the *first* route.
Took me far too long to diagnose that!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This removes the index array from code after increase_flows()m, so we use the flows
array directly.
The next step will be to make increase_flows() use the flows array, and remove the
index array indirection entirely.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't need the indexes array, we can use this directly.
We still set up the indexes array (for now) after we call this.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It can only fail on overflow, but if it did, the fail path frees working_ctx
and returns "error_message".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is not worth optimizing that I can see. Using a non-debug build I get
the following times for tests/test_askrene.py::test_real_data
Before:
143 seconds
After:
141 seconds.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I added amount_msat_accumulate for the "a+=b" case, but I was struggling
with a name for the subtractive equivalent. After some prompting, ChatGPT
suggested deduct.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is immune to things like clock changes, and has the convenient side-effect that
it will *not* be overridden when we override time for developer purposes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This doesn't happen much in real life, but it's certainly possible, so do what pay does here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: https://github.com/ElementsProject/lightning/issues/8612
Changelog-Added: `xpay` will now wait if it suspects a payment failure is due to a height disagreement with the final node.
Changelog-Added: askrene-bias-node: an RPC command to set a bias on node's outgoing or incoming channels.
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
We add one more field to biases: "timestamp".
With the timestamp variable old biases can be removed with the
askrene-age command.
Changelog-Added: Plugins: askrene channel biases now have an associated timestamp, and are timed out by askrene-age
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
