Now we actually check the other fields too, as per BOLT!
Reported-by: https://github.com/hMsatsFixes: #7513
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In CI, this would sometimes fail: we would timeout waiting for the
fetchinvoice reply. Never happened locally, so was annoying to debug.
What happened was simple: we called injectonionmessage then when it
returned, put the "sent" object in the linked list so we could recognize
any reply onion messages.
However, we were getting that reply before the plugin processed the response
to injectonionmessage. This is possible because there are two fds for
plugins: one for it to receive notifications and hooks (like onion messages)
and one for normal RPC usage (like commands to inject onion messages).
The fix is simple: put in the list *before* calling JSON RPC.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a final sweep to match the current BOLT12 text:
1563d13999d342680140c693de0b9d65aa522372 ("More bolt12 test vectors.")
Only two code changes, to change the order of checks to match the bolt,
and to give a warning on decode if a path is empty.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: offers: `invoicerequest` will set a blinded path if we're an unannounced node.
Changelog-EXPERIMENTAL: offers: `sendinvoice` will use a blinded path in an invoice_request, if specified.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's an internal undocumented interface, which makes this change less painful.
We *do* check that the invreq_metadata maps to the given invreq_payer_id, which would
is required for us to sign it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They can do it now: before it would have been awkward to look up previous
payments to match it up for recurring offers (which need to use the same
key, hence the same invreq_metadata).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The latest spec allows this to be omitted iff there is a blinded path
and it would be made up anyway.
In that case, the key they will use to sign the invoice will be the final
blinded key in the path we use.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The latest draft allows these experimental ranges, which involves more
changes than I expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: offers: handle experimental ranges in offers/invoice_requests/invoices.
We generate a reply path by simply reversing the outgoing path:
A->B->C gives reply path B->A
A->B gives reply path A
But if we are not a public node, we can't use ourselves as the first
entry of the reply path: this happens if we directly connect to the
head of a blinded path (as we now support).
In this case, give the entire path as a blinded path. We could do
this all the time, but there are some cases where nodes don't like
sending replies where the node itself is the head of the blinded
path (like CLN v24.05 or before!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We iterate through the blinded paths until we can use one, and because we use
the modern code, we properly join paths if we need to route more than one hop
to reach the start of the blinded path.
Changelog-EXPERIMENTAL: fixed: fetchinvoice tries all blinded paths until one is usable, and handles case where we have to route more than one hop to reach the entry point.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We already parse some fields, so hand them directly rather than
having fetchinvoice behave as if it's a raw hook.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means only a single gossmap, and they already share the fetchinvoice-noconnect option
and autoconnect code.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: the `fetchinvoice` plugin has been combined into the `offers` plugin.
This means we can see the values in listconfigs, even if we haven't set
them yet.
In particular, we now see the following:
* autoclean-cycle.value_int=3600
* bitcoin-rpcclienttimeout.value_int=60
* bitcoin-retry-timeout.value_int=60
* funder-max-their-funding.value_str=4294967295sat
* funder-per-channel-min.value_str=10000sat
* funder-reserve-tank.value_str=0sat
* funder-fund-probability.value_int=100
Changelog-Changed: plugins: libplugin now shows plugin option default values (where they're non-trivial)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our fetchinvoice always creates a reply path which terminates at their peer,
so we need a dev overrride for that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: offers: we now understand blinded paths which use a short-channel-id(+direction) as entry point.
We don't actually support it yet, but this threads through the type change,
puts it in "decode" etc.
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.
This integrates them with configvars properly: they almost "just work"
in listconfigs now, and we don't put them in a special sub-object
under their plugin.
Unfortunately, this means `listconfigs` now has a loose schema: any
plugin can add something to it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: reloaded plugins get passed any vars from configuration files.
Changelog-Deprecated: Config: boolean plugin options set to `1` or `0` (use `true` and `false` like non-plugin options).
While the user trying to fetch an invoice by specifing the quantity we do
not work as expected.
Running the command
```
lightning-cli fetchinvoice -k offer='lno1qgsqvgnwgcg35z6ee2h3yczraddm72xrfua9uve2rlrm9deu7xyfzrcgqffqszsk2p6hycmgv9ek2grpyphxjcm9ypmkjer8v46pyzmhd9jxwet5wvhxxmmdzsqs593pq0ylsvakdua5h976f4g3eautgjt3udvtyga47eaw7339sjrhpwpwz' quantity=2
```
and we answer back with
```json
{
"code": -32602,
"message": "quantity parameter required"
}
```
This is caused because we forget to bind the `quanity` field from the
RPC into the `invrequest`.
Reported-by: @aaronbarnardsound
Link: https://github.com/ElementsProject/lightning/issues/6089
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-EXPERIMENTAL: fetchinvoice: fix: do not ignore the `quantity` field
into the invreq field.
```
make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-offers" BOLTVERSION=guilt/offers
```
In this case, only trivial mods.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is how we put new invoice_requests into the db; this will be used
by a new "invoicerequest" command which replaces "offerout".
The API is now the same as the offers api.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We no longer have to refer back to the offer for which we're making
the invoice_request, or to the invoice_request we made for an invoice,
as they are all mirrored (and we check!).
It's clearer to simply look at the object directly.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I know this is an unforgivably large diff, but the spec has changed so
much that most of this amounts to a rewrite.
Some points:
* We no longer have "offer_id" fields, we generate that locally, as all
offer fields are mirrored into invoice_request and then invoice.
* Because of that mirroring, field names all have explicit offer/invreq/invoice
prefixes.
* The `refund_for` fields have been removed from spec: will re-add locally later.
* quantity_min was removed, max == 0 now mean "must specify a quantity".
* I have put recurrence fields back in locally.
This brings us to 655df03d8729c0918bdacac99eb13fdb0ee93345 ("BOLT 12:
add explicit invoice_node_id.")
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had a scheme where lightningd itself would put a per-node secret in
the blinded path, then we'd tell the caller when it was used. Then it
simply checks the alias to determine if the correct path was used.
But this doesn't work when we start to offer multiple blinded paths.
So go for a far simpler scheme, where the secret is generated (and
stored) by the caller, and hand it back to them.
We keep the split "with secret" or "without secret" API, since I'm
sure callers who don't care about the secret won't check that it
doesn't exist! And without that, someone can use a blinded path for a
different message and get a response which may reveal the node.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
