CI revealed one:
```
cc plugins/libplugin-pay.c
plugins/libplugin-pay.c: In function ‘payment_getroute’:
plugins/libplugin-pay.c:888:17: error: ‘errstr’ may be used uninitialized [-Werror=maybe-uninitialized]
888 | payment_fail(p, "%s", errstr);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
plugins/libplugin-pay.c:851:21: note: ‘errstr’ was declared here
851 | const char *errstr;
| ^~~~~~
cc1: all warnings being treated as errors
```
My local compiler gave another:
```
channeld/channeld.c: In function ‘resume_splice_negotiation’:
channeld/channeld.c:3734:23: error: ‘final_tx’ may be used uninitialized [-Werror=maybe-uninitialized]
3734 | msg = towire_channeld_splice_confirmed_signed(tmpctx, final_tx,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3735 | chan_output_index);
| ~~~~~~~~~~~~~~~~~~
channeld/channeld.c:3461:28: note: ‘final_tx’ was declared here
3461 | struct bitcoin_tx *final_tx;
| ^~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:298: channeld/channeld.o] Error 1
```
So fix both.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As side-effect, getroute(0) is special too.
Reported-by: MiddleW4y in Discord
Fixes: #6577
Changelog-Fixed: `pay` will still use an invoice routehint if path to it doesn't take 1-msat payments.
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
The alias may not be set for non-alias channels after they
confirm. The other branch is safe because we only consider active
channels.
Changelog-None
Fixes#6450
This is the simplest solution, not the best, but there's significant risk in try to remove the "we have a path" assumption in the code pay code.
Includes removing a `tal_steal` which was incorrect: the buffer has the same lifetime as the plugin, so if we steal it then things get messy when we free the struct payment.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `pay` will now pay your own invoices if you try.
Debugging a number of payments showed that we sometimes waste a number
of attempts routing through a channel via its alias, rather than its
scid. This is because while we annotate the scid when it has been set,
we do not do so for the alias. The alias then is picked for routing
despite not having enough capacity, failing the attempt locally.
It can also happen that we alternate between scid and alias, doubling
the number of failed attempts before we can make progress.
This patch sets the hint for the alias to a capacity of 0 and disables
it as if the peer were offline. This means when available we'll always
use the scid, which is also far easier to read in the logs.
Changelog-Fixed: pay: We now track spendable amounts when routing on both the local alias as well as the short channel ID
Otherwise we later copy the uninitialized memory to descendants,
triggering undefined behavior:
plugins/libplugin-pay.c:2882:34: runtime error: load of value 190, which is not a valid value for type 'bool'
"BOLT 4: Remove legacy format, make var_onion_optin compulsory."
This also renamed the redundant "tlv_payload" to "payload", so we
replace "tlv_tlv_payload" with "tlv_payload" everyhere!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
With the warning that we were trying to put "inf" into a u64, we can
see that this calculation was wrong to use integers!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The htlc_budget only exists iff the hint is a 'local' one; we were
failing to write to the htlc_budget field for non-local cases.
To avoid this, we make `local` into a struct that contains the fields
that pertain to local-only payments (in this case, `htlc_budget`).
Valgrind error file: valgrind-errors.1813487
==1813487== Conditional jump or move depends on uninitialised value(s)
==1813487== at 0x4A9C958: __vfprintf_internal (vfprintf-internal.c:1687)
==1813487== by 0x4AB0F99: __vsnprintf_internal (vsnprintf.c:114)
==1813487== by 0x1D2EF9: do_vfmt (str.c:66)
==1813487== by 0x1D3006: tal_vfmt_ (str.c:92)
==1813487== by 0x11A60A: paymod_log (libplugin-pay.c:167)
==1813487== by 0x11B749: payment_chanhints_apply_route (libplugin-pay.c:534)
==1813487== by 0x11EB36: payment_compute_onion_payloads (libplugin-pay.c:1707)
==1813487== by 0x12000F: payment_continue (libplugin-pay.c:2135)
==1813487== by 0x1245B9: adaptive_splitter_cb (libplugin-pay.c:3800)
==1813487== by 0x11FFB6: payment_continue (libplugin-pay.c:2123)
==1813487== by 0x1206BC: retry_step_cb (libplugin-pay.c:2301)
==1813487== by 0x11FFB6: payment_continue (libplugin-pay.c:2123)
==1813487==
{
<insert_a_suppression_name_here>
Memcheck:Cond
fun:__vfprintf_internal
fun:__vsnprintf_internal
fun:do_vfmt
fun:tal_vfmt_
fun:paymod_log
fun:payment_chanhints_apply_route
fun:payment_compute_onion_payloads
fun:payment_continue
fun:adaptive_splitter_cb
fun:payment_continue
fun:retry_step_cb
[sesh] 0:[tmux]*Z
Suggested-By: @nothingmuch
If we only specify the node_id, we get the "first" channel.
Closes: #5803
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: `pay` uses the correct local channel for payments when there are multiple available (not just always the first!)
Valgrind correctly reports it as uninitialized for this log message, and
the only way this can happen is channel_hints_update() when we receive a
temporary_channel_failure. Put a dummy value here in this case.
```
Valgrind error file: valgrind-errors.23404
==23404== Conditional jump or move depends on uninitialised value(s)
==23404== at 0x49E4B56: __vfprintf_internal (vfprintf-internal.c:1516)
==23404== by 0x49F6519: __vsnprintf_internal (vsnprintf.c:114)
==23404== by 0x1EBCEB: do_vfmt (str.c:66)
==23404== by 0x1EBDF8: tal_vfmt_ (str.c:92)
==23404== by 0x11A336: paymod_log (libplugin-pay.c:167)
==23404== by 0x11B4B2: payment_chanhints_apply_route (libplugin-pay.c:534)
==23404== by 0x11E999: payment_compute_onion_payloads (libplugin-pay.c:1707)
==23404== by 0x11FF4C: payment_continue (libplugin-pay.c:2135)
==23404== by 0x1245C0: adaptive_splitter_cb (libplugin-pay.c:3800)
==23404== by 0x11FEF3: payment_continue (libplugin-pay.c:2123)
==23404== by 0x1205FE: retry_step_cb (libplugin-pay.c:2301)
==23404== by 0x11FEF3: payment_continue (libplugin-pay.c:2123)
==23404==
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
make check-source-bolt CHECK_BOLT_PREFIX="--prefix=BOLT-route-blinding" BOLTVERSION=guilt/offers
```
Other than textual changes, this does:
1. Ensures we put total_amount_msat in onion final hop (reported by @t-bast).
2. Require that they put total_amount_msat in onion final hop.
3. Return `invalid_onion_blinding` exactly as defined by the spec (i.e. less
aggressive when we're the final hop) (also reported by @t-bast, but I already
knew).
See: #5823
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `offers` breaking blinded payments change (total_amount_sat required, Eclair compat)
Don't parse the listpeers.channels output ourselves: with two extra fields
we can simply reuse json_to_listpeers_channels().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Instead of returning a peers -> channels heirarchy, return (as callers
want!) a flat array of channels.
This is actually most of the transition work to make them work with
listpeerchannels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We no longer use offers for "I want to send you money", but we'll use
invoice_requests directly. Create a new table for them, and
associated functions.
The "localofferid" for "pay" and "sendpay" is now "localinvreqid".
This is an experimental-only option, so document the change under
experimental only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: JSON-RPC: `pay` and `sendpay` `localofferid` is now `localinvreqid`.
We simply take the first one, and route to the start of that. Then we
append the blinded path to the onion construction.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It means we consume the channel completely to the best of our
knowledge, so let that through.
Changelog-Fixed: pay: Squeezed out the last `msat` from our local view of the network
In the case of the local channel we set the estimation to the exact
value spendable, which is important when we want to drain a channel,
because there we actually want to get the last msat.
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`)
We were setting it on the root, but that doesn't get handed to
sendpay. Our schema doesn't *require* bolt11, either, so this was
missed (there could be a *bolt12* instead).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `listpays` always includes `bolt11` or `bolt12` field.
As per proposal in https://github.com/lightning/bolts/pull/962
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: protocol: support for legacy onion format removed, since everyone supports the new one.
1. The dijkstra can be temporary, doesn't need to last as long as pay cmd.
2. We fail multiple times in several places, so don't leak old failreason.
3. Make payments findable by our memleak detector.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we have the exact state of the channels from the `listpeers`
response we can filter out the ones that are not yet normal or not
anymore. This is more of a safety net, given that the `gossip_store`
should contain local disables, but better not be racy :-)
Changelog-None
And turn "" includes into full-path (which makes it easier to put
config.h first, and finds some cases check-includes.sh missed
previously).
config.h sets _GNU_SOURCE which really needs to be done before any
'#includes': we mainly got away with it with glibc, but other platforms
like Alpine may have stricter requirements.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: pay: The route selection will now use the log-propability-based channel selection to increase success rate and reduce time to completion
We bias by channel linearly by capacity, scaled by median fee.
This means that we effectively double the fee if we would use the
entire capacity, and only increase it by 50% if we would only use
1/2 the capacity.
This should drive us towards larger channels.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Plugins: `pay` now biases towards larger channels, improving success probability.
Rusty Russell
Dusty Daemon
Christian Decker
Matt Morehouse
niftynei
Ken Sedgwick
Andrew Toth
Simon Vrouwe
Rene Pickhardt