This means (temporarily) that blacklisting won't work (fix later), and
means that old-style (commando.py) master-secret-override doesn't work.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: Plugins: `commando` no longer allows datastore ['commando', 'secret'] to override master secret (re-issue runes if you were using that!).
In preparation for going async:
1. Split try_command's tail into a new function called execute_command() after
the rune checks have succeeded.
2. Put all the info execute_command() needs into struct cond_info, to make it
a simple callback style.
So we create new_cond_info() which dynamically allocates `struct cond_info`
and sets the destructor.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We would create a `struct commando` to marshal our incoming messages,
then try_command would create a *new* one. We can simply reuse, but
when I did I noticed a trick: the new one was not in the `incomings`
array, so didn't work towards the ratelimit. So we need to remove it
from `incomings` in `try_command`, but at least it's now explicit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We used to activate on the first rune creation, but we're no longer in charge
of runes, so we can't make that call.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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
The presplitter modifier would split a payment before trying the first
attempt based on some common sizes. Its goal was to have smaller parts
in flight over different paths, in order to make it more difficult for
a forwarding node to learn payment amount. However it was causing some
issues for direct payments, and estimates on spendable amounts which
considers only the first HTLC being added, but presplitter would
always cause multiple HTLCs to be kicked off, causing the estimate to
be off.
Removing the presplitter fixes this, making draining channels easier,
and worse success rates, due to more HTLCs in flight directly
impacting the changes of getting stuck.
Changelog-Removed: pay: `pay` no longer splits based on common size, as it was causing issues in various scenarios.
This is almost always true already; fix up the few non-standard ones.
This is enforced with an assert, and I ran the entire test suite to
double-check.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously, our code checked for the presence of the `lightning:`
prefix while decoding a bolt11 string. Although this prefix is valid
and accepted by the core lightning pay command, it was causing issues
with how we managed invoices. Specifically, we were skipping the prefix
when creating a copy of the invoice string and storing the raw invoice
(including the prefix) in the database, which caused inconsistencies
in the user experience.
To address this issue, we need to strip the `lightning:` prefix before
calling each core lightning command. In addition, we should
modify the invstring inside the db with the canonical one.
This commit fixes the issue by stripping the `lightning:` prefix
from the `listsendpays` function, which will improve the
user experience and ensure consistency in our invoice management (see
next commit).
Reported-by: @johngribbin
Link: ElementsProject#6207
Fixes: debbdc0
Changelog-Fixes: trim the `lightning:` prefix from invoice everywhere.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
We usually have access to `ld`, so avoid the global.
The only place generic code needs it is for the json command struct,
and that already has accessors: add one for libplugin and lightningd
to tell it if deprecated apis are OK.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids the mess where we override db_fatal for teqsts, and keeps it
generic.
Also allows us to get rid of one #if DEVELOPER, and an ugly global for
bookkeeper.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If we're opening a channel with a peer which support anchors (and
we do), we tell fundpsbt/utxopsbt to enforce the emergency reserve;
this matters, as it doesn't know about the channel yet, and thus
won't (if it's our first anchor channel).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `fundchannel` and `multifundchannel` will refuse to spend funds below `min-emergency-msat` if we have any anchor channels (or are opening one).
This was added to fundpsbt/utxopsbt in v0.10, but the spender plugin
didn't take advantage of it, instead calculating its own change amount
and output.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This was added to fundpsbt/utxopsbt in v0.10, but the txprepare plugin
didn't take advantage of it, instead calculating its own change amount
and output.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were marking our inputs very late, which means any early failure
would not know to unreserve them.
This becomes particularly bad when we start enforcing emergency reserves.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
mfc->feerate_str is *never* NULL, since we set it in getfeerate; this is
confusing, as many places check for NULL.
Indeed, the logic in perform_fundpsbt() was *wrong* in this case: it used
`normal` (if it was NULL, which it never was) instead of `opening` to fundpsbt.
And the correct thing is for multifundchannel to not use a string here at
all, but to use the exact feerate it is counting on (even the same
string may have different values now if a block has come in).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since we didn't hash the descriptions properly (see previous commit), we
cannot immediately deprecate omitting the descriptions (since you'd
have to omit them for backwards compat!).
And move the "must have description or hash" test into bolt11.c core.
Changelog-Deprecated: `pay` has *undeprecated* paying a description-hash invoice without providing the description.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This means we need to push off requring this for another full deprecation cycle!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `pay` and `decodepay` with description now correctly handle JSON escapes (e.g " inside description)
I added a plugin arg and was surprised that compile didn't break.
This is because typesafe_cb et al are conditional casts: if the type
isn't as expected it has no effect, but we're passing plugin_option() through
varargs, so everything is accepted!
Add a noop inline to check type, and fix up the two cases where we
used `const char *` instead of `char *`.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previously, if these failed we always exited; once we have dymamic
configs this would be a (tiny) memory leak, so use tmpctx.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When core lightning is asking the information about
the blockchain with `getchaininfo` command lightningd
know already the information about the min and max block height.
the problem is when we have a smarter Bitcoin backend that is able
to switch between different clients in some cases is helpful
give the information about current known height by lightningd and
pass it down to the plugin.
In this way, the plugin knows what is the correct known height from lightnind, and can
try to fix some problems if any exit.
This is particularly useful when you are syncing a new backend from scratch
like https://github.com/cloudhead/nakamoto and we avoid returning the
lower height from the known, and avoid the crash of core lightning.
With this information, the plugin can start to sync the chain and return
the answer back only when the chain is in sync with the current status of
lightningd.
Another reason to add this field and not wait the correct block in core
lightning itself is because Bitcoin Core is extremely slow to sync up,
so the question here is, how long should we wait? The time depends
on various factors.
With this approach of informing the plugin about the height, in some cases,
you can start the syncing but move the execution to another backend until
the previous one is ready.
The problem I want to solve is that I don't want to be left in the dark when
we run `getchaininfo`, and I want to have the opportunity to wait for
the blockchain sync or decide to dispatch the request elsewhere.
Changelog-Added: Pass the current known block height down to the getchaininfo call.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Previously any attempt would result in "is not an integer field"; we
now recognize valid JSON integers as integer fields.
Changelog-Fixed: Plugins: `commando` runes can now compare integer parameters using '<' and '>' as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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'
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).
We use multi-specifiable options elsewhere, this is just another.
Otherwise you can't add, you can only set them all.
Changelog-Added: Config: `accept-htlc-tlv-type` (replaces awkward-to-use `accept-htlc-tlv-types`)
Changelog-Deprecated: Config: `accept-htlc-tlv-types` (use `accept-htlc-tlv-type` multiple times)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
gcc 13 add an extra check for the enum in the definition
of a method. In our case the code was failing with the
following error, and the compiler is right, our definition
is different from the implementation.
```
$ make
CC: cc -DBINTOPKGLIBEXECDIR="../libexec/c-lightning" -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -Werror -Wno-maybe-uninitialized -Wshadow=local -std=gnu11 -g -fstack-protector-strong -Og -I ccan -I external/libwally-core/include/ -I external/libwally-core/src/secp256k1/include/ -I external/jsmn/ -I external/libbacktrace/ -I external/gheap/ -I external/x86_64-redhat-linux/libbacktrace-build -I external/libsodium/src/libsodium/include -I external/libsodium/src/libsodium/include/sodium -I external/x86_64-redhat-linux/libsodium-build/src/libsodium/include -I . -I/usr/local/include -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS -DCOMPAT_V052=1 -DCOMPAT_V060=1 -DCOMPAT_V061=1 -DCOMPAT_V062=1 -DCOMPAT_V070=1 -DCOMPAT_V072=1 -DCOMPAT_V073=1 -DCOMPAT_V080=1 -DCOMPAT_V081=1 -DCOMPAT_V082=1 -DCOMPAT_V090=1 -DCOMPAT_V0100=1 -DCOMPAT_V0121=1 -DBUILD_ELEMENTS=1 -c -o
LD: cc -Og config.vars -Lexternal/x86_64-redhat-linux -lwallycore -lsecp256k1 -ljsmn -lbacktrace -lsodium -L/usr/local/include -lm -lgmp -lsqlite3 -lz -o
cc plugins/spender/multifundchannel.c
plugins/spender/multifundchannel.c:71:6: error: conflicting types for ‘fail_destination_msg’ due to enum/integer mismatch; have ‘void(struct multifundchannel_destination *, enum jsonrpc_errcode, const char *)’ [-Werror=enum-int-mismatch]
71 | void fail_destination_msg(struct multifundchannel_destination *dest,
| ^~~~~~~~~~~~~~~~~~~~
In file included from plugins/spender/multifundchannel.c:13:
./plugins/spender/multifundchannel.h:263:6: note: previous declaration of ‘fail_destination_msg’ with type ‘void(struct multifundchannel_destination *, int, const char *)’
263 | void fail_destination_msg(struct multifundchannel_destination *dest,
| ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make: *** [Makefile:307: plugins/spender/multifundchannel.o] Error 1
```
The gcc 13 is not released yet, but fedora beta is out for public testing,
so it is useful fix this error in this release candidate cycle.
Changelog-Fixed: Build: Compilation with upcoming gcc 13
Reported-by: @grubles
Link: https://github.com/ElementsProject/lightning/issues/6175
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This was pointed out by Daywalker [1]: we are synchronously processing
events from `lightningd` which means that if processing one of the
hooks or requests was slow or delayed, we would not get notifications,
hooks, or RPC requests, massively impacting the flexbility.
This highlights the issue with a failing test (it times out), and in
the next commit we will isolate event processing into their own task,
so to free the event loop from having to wait for an eventual
response.
[1] https://community.corelightning.org/c/developers/hold-invoice-plugin#comment_wrapper_16754493
This was always the intent, but now we have to reconstruct from the
disparate fields.
This means `option_anchor_outputs` is now redundant.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: #4473
Changelog-Deprecated: Plugins: `estimatefees` returning feerates by name (e.g. "opening"); use `fee_floor` and `feerates`.
Changelog-Fixed: Plugins: `bcli` now tells us the minimal possible feerate, such as with mempool congestion, rather than assuming 1 sat/vbyte.
Turns out the two bcli replacements I checked (`sauron` and
`trustedcoin`) don't even implement this, and the multiplier makes
more sense in lightningd, especially as we move to bcli just providing
raw feerate estimates.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"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>
We didn't handle the case of an array inside a subobject. But that
happens when we add the next commit!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
While we are cleaning up the list forwards with the autoclean plugin we are
not taking into count the forward's payments with the status set to
`local_failed`. In this case, the forwards have no resolved
time because it was not resolved by us due to some local error.
So, this commit is fixing the auto clean plugin by allowing to delete
of the forwards with status set to local_failed by taking into count
the received_time, with the assumption that the received_time, in this case,
is equal to the resolved time (?)
Reported-by: @denis2342
Link: https://github.com/ElementsProject/lightning/issues/6058
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Changelog-Fixed: plugin: autoclean: considerer the forwards with status
set to `local_failed`.
Libwally update breaks compatibility, so
we do this in one large step.
Changelog-Changed: JSON-RPC: elements network PSET now only supports PSETv2.
Changelog-Added: JSON-RPC: PSBTv2 supported for fundchannel_complete, openchannel_update, reserveinputs, sendpsbt, signpsbt, withdraw and unreserveinputs parameter psbt, openchannel_init and openchannel_bump parameter initialpsbt, openchannel_signed parameter signed_psbt and utxopsbt parameter utxopsbt
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.
When plugins receive a "shutdown" notification, then can call this
method which will shutdown `cln_plugin`.
Then they can await `plugin.join()` and do any remaining cleanup
there.
This helps avoid a pain-point where plugin authors need to handle
2 separate plugin shutdown mechanisms https://github.com/ElementsProject/lightning/issues/6040
It's still deprecated: we need the description since
1. This information is useful for any validation we want to do, such as
the HSM, or runes.
2. We want this information in listpays so we can tell what we actually paid.
3. In general, we should never sign commitments to things we don't have!
I expect to have this information about payments *whatever the frontend* is,
which is why we deprecated (and then removed) this unintended use. The spec
is pretty clear on this:
BOLT #11:
```
A reader:
...
- MUST check that the SHA2 256-bit hash in the `h` field exactly matches the hashed
description.
```
However, neither BTCPayServer nor lnbits updated despite the long deprecation
period, so revert 2afe7a1856.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Seems like LND is hanging up on receiving these messages, even though
they're odd :(
So, when a peer connects, check if it supplies or wants peer backup
(even if it doesn't support both, it shouldn't hang up, and I didn't
want to separate the two paths).
And when we go to send our own, updated backup, check features before
sending.
Fixes: #6065
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: `experimental-peer-storage` caused LND to hang up on us, so only send to peers which support it.
Without this patch, we only ever loaded the "nodes" table once, then
didn't see updates.
How this ever got past CI is a mystery; perhaps valgrind was so slow that
the updated node_announcement hit the gossmap before we even asked sql
on l3 about the nodes table?
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: `sql` nodes table now gets refreshed when gossip changes.
Since it is an optional field in the `listconfigs` output we can't use
the `rpc_scan` mechanism (doesn't handle optionals yet). We'll use
that list of accepted types later to avoid stripping them.
This commit will disable the peerstorage plugins
when the feature is not enabled.
I found this issue with lnprototest, and I guess
we did not find it with normal run because
other the unknown messages are ingored?
Changelog-Fixed: Disable the protocol messages when peerstorage is disabled.
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
If we had two things to clean, we fired off two requests (eg.
listforwards and listinvoices) and both marked the timer as finished,
triggering an assert.
We already have a refcount for outstanding requests to avoid this
for e.g. outstanding del commands, so use it here too!
```
Jan 19 19:20:00 lightningd[748044]: autoclean: plugins/libplugin.c:445: timer_complete: Assertion `p->in_timer > 0' failed.
Jan 19 19:20:00 lightningd[748044]: autoclean: FATAL SIGNAL 6 (version v22.11.1)
Jan 19 19:20:00 lightningd[748044]: 0x562c388136e4 send_backtrace
Jan 19 19:20:00 lightningd[748044]: common/daemon.c:33
Jan 19 19:20:00 lightningd[748044]: 0x562c3881376c crashdump
Jan 19 19:20:00 lightningd[748044]: common/daemon.c:46
Jan 19 19:20:00 lightningd[748044]: 0x7f26d0898d5f ???
Jan 19 19:20:00 lightningd[748044]: ./signal/../sysdeps/unix/sysv/linux/x86_64/sigaction.c:0
Jan 19 19:20:00 lightningd[748044]: 0x7f26d0898ce1 __GI_raise
Jan 19 19:20:00 lightningd[748044]: ../sysdeps/unix/sysv/linux/raise.c:51
Jan 19 19:20:00 lightningd[748044]: 0x7f26d0882536 __GI_abort
Jan 19 19:20:00 lightningd[748044]: ./stdlib/abort.c:79
Jan 19 19:20:00 lightningd[748044]: 0x7f26d088240e __assert_fail_base
Jan 19 19:20:00 lightningd[748044]: ./assert/assert.c:92
Jan 19 19:20:00 lightningd[748044]: 0x7f26d0891661 __GI___assert_fail
Jan 19 19:20:00 lightningd[748044]: ./assert/assert.c:101
Jan 19 19:20:00 lightningd[748044]: 0x562c3880355d timer_complete
Jan 19 19:20:00 lightningd[748044]: plugins/libplugin.c:445
Jan 19 19:20:00 lightningd[748044]: 0x562c38800b54 clean_finished
Jan 19 19:20:00 lightningd[748044]: plugins/autoclean.c:122
Jan 19 19:20:00 lightningd[748044]: 0x562c388010ed clean_finished_one
Jan 19 19:20:00 lightningd[748044]: plugins/autoclean.c:132
Jan 19 19:20:00 lightningd[748044]: 0x562c388011b6 del_done
Jan 19 19:20:00 lightningd[748044]: plugins/autoclean.c:149
Jan 19 19:20:00 lightningd[748044]: 0x562c388058b5 handle_rpc_reply
Jan 19 19:20:00 lightningd[748044]: plugins/libplugin.c:768
Jan 19 19:20:00 lightningd[748044]: 0x562c38805a39 rpc_read_response_one
Jan 19 19:20:00 lightningd[748044]: plugins/libplugin.c:944
Jan 19 19:20:00 lightningd[748044]: 0x562c38805ad7 rpc_conn_read_response
Jan 19 19:20:00 lightningd[748044]: plugins/libplugin.c:968
Jan 19 19:20:00 lightningd[748044]: 0x562c38876b60 next_plan
Jan 19 19:20:00 lightningd[748044]: ccan/ccan/io/io.c:59
Jan 19 19:20:00 lightningd[748044]: 0x562c38876fe7 do_plan
Jan 19 19:20:00 lightningd[748044]: ccan/ccan/io/io.c:407
Jan 19 19:20:00 lightningd[748044]: 0x562c38877080 io_ready
Jan 19 19:20:00 lightningd[748044]: ccan/ccan/io/io.c:417
Jan 19 19:20:00 lightningd[748044]: 0x562c3887823c io_loop
Jan 19 19:20:00 lightningd[748044]: ccan/ccan/io/poll.c:453
Jan 19 19:20:00 lightningd[748044]: 0x562c38805d11 plugin_main
Jan 19 19:20:00 lightningd[748044]: plugins/libplugin.c:1801
Jan 19 19:20:00 lightningd[748044]: 0x562c38801c7a main
Jan 19 19:20:00 lightningd[748044]: plugins/autoclean.c:613
```
Fixes: #5912
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Since it's not spec-final yet (hell, it's not even properly specified
yet!) we need to put it behind an experimental flag.
Unfortunately, we don't have support for doing this in a plugin; a
plugin must present features before parsing options. So we need to do
it in core.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When you return an allocated pointer, you should always hand in the
context you want it allocated from. This is more explicit, because it may
really matter to the caller!
This also folds some simple operations, and avoids doing too much
variable assignment in the declarations themselves: some coding styles
prohibit such initializers, but that's a bit exteme.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And we should always represent them as is, not as optional: it's
possible in future we could *require* "WANT_PEER_BACKUP_STORAGE".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
v2 opens require you to use native segwit inputs
Changelog-Added: JSONRPC: `upgradewallet` command, sweeps all p2sh-wrapped outputs to a native segwit output
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!)
I noticed that our subtables were not being cleaned, despite being "ON
DELETE CASCADE". This is because foreign keys were not enabled, but
then I got foreign key errors: rowid cannot be a foreign key anyway!
So create a real "rowid" column. We want "ON DELETE CASCADE" for
nodes and channels (and other tables in future) where we update
partially.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
This *would* be a 1-line change (add it to Makefile) except that we
previously assumed a "list" prefix on commands.
These use the default refreshing, but they could be done better using
the time-range parameters.
Suggested-by: @niftynei
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In particular, we generate the schema part from the plugin itself.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `sql` plugin command to perform server-side complex queries.
We now add tables to the strmap as we allocate them, since we don't
want to call "finish_td" when we're merely invoked for the
documentation, and don't need a database.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For now, we ignore every deprecated field, but put in the logic so
that future deprecations will work as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This requires us to rename "index" fields, rename fields if we have a
sub-object, and create sub-tables if we have an array, and handle the
fact that some listX commands don't contain array X (listsendpays
contains "payments").
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than two arrays "columns" (for names) and "fieldtypes" (for
types), use a struct. This makes additions easier for successive
patches.
Also pull process_json_obj() out of the loop in list_done().
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's actually two separate u16 fields, so actually treat it as
such!
Cleans up zombie handling code a bit too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's a core concept in the spec which isn't directly exposed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listchannels` added a `direction` field (0 or 1) as per gossip specification.
Our pay code handles this correctly, but decode was still using an old model
where there was a payinfo per hop, not per path.
Reported-by: @t-bast
See: #5823
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
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>
```
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)
Also, we don't need to pass the total length to the field parsers,
just the length for this field (confusingly, this was called
"data_length").
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
1. When we receive a commando command from a remote using the `filter`
field, use it.
2. Add a `filter` parameter to `commando` to send it: this is usually
more efficient than using filtering locally.
Of course, older remote nodes will ignore the filter, but that's
harmless.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `commando` now supports `filter` as a parameter (for send and receive).
This was reported a while ago: now do it properly.
Fixes: #5637
Changelog-Fixed: Plugins: `commando` now responds to remote JSON calls with the correct JSON `id` field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We change the libplugin API so commando can provide its own ID base.
This id chaining enables much nicer diagnostics!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We don't do this yet, so we add deprecated to those test (until next
patch!).
Changelog-Deprecated: plugins: `commando` JSON commands without an `id` (see doc/lightningd-rpc.7.md for how to construct a good id field).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When called with `"id": 1` we replied with `"id": "1"`. lightningd doesn't
actually care, but it's weird.
Copy the entire token: this way we don't have to special case anything.
Also, remove the doubled test in json_add_jsonstr.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
local_connected allocates a struct node_map off of the struct
listchannels_opts but fails to ever release the internal table, so those
table allocations hang around forever. Add node_map_clear as a
destructor for the struct node_map to ensure that the internal table is
freed when the enclosing struct is freed.
Changelog-Fixed: topology: Fixed memleak in `listchannels`
Signed-off-by: Matt Whitlock <c-lightning@mattwhitlock.name>
There are several cases where you want to access to the configuration,
and given the nature of the rust API, there is no way to access to
the `configuration` field at any point after the configuration logic.
Suggested-by: Sergi Delgado Segura <@sr-gi>
Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
This commit updates outdated dependencies and hangs all
bitcoin-related dependencies off of the `bitcoin` crate, using its
re-exports. This means that as long as the bitcoin crate matches, all
of its dependents will also match.
Changelog-None
Deprecated v0.11.0.
Changelog-Removed: JSON-RPC: `pay` for a bolt11 which uses a `description_hash`, without setting `description` (deprecated v0.11.0).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We only incremented this if it was the wrong state.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: `autoclean-once` response `uncleaned` count is now correct.
The autoclean plugin would assume we have a `resolved_time` which may
not be true for oldish nodes that predate our annotations.
Changelog-None Unreleased change
Reported-by: <@devastgh>
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 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 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>
It's just to a direct peer, and we only create one, but this is
enough to test, and make payments to non-public nodes work.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is needed for offers to generate blinded paths.
No documentation changes since listincoming is an undocumented
internal hack interface which topology presents for production
of routehints.
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>
We actually want lightningd to create these, since it wants to put the
path_id secret in the last element. So best API is actually a generic
one, rather than separate APIs to create first and last ones.
And really, the more explicit initialization makes the users clearer.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
Since the "struct command" is different from plugins and lightningd, we
need an accessor for this to work (the plugin one is a dummy for now!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Mainly, field name changes.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: Protocol: Support for forwarding blinded payments (as per latest draft)
We need to print out first_node_id, and "node_id" is now called
"blinded_node_id" in the spec.
And the schema didn't include the payment fields in the blinded path
for invoices (which broke as soon as we actually tested one!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's 2b7ad577d7a790b302bd1aa044b22c809c76e49d, which reverts the
point32 changes.
It also restores send_invoice in `invoice`, which we had removed
from spec and put into the recurrence patch.
I originally had implemented compatibility, but other changes
which followed this are far too widespread.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-EXPERIMENTAL: offers: complete rework of spec from other teams (yay!) breaks previous compatibility (boo!)
We use the saved previous outputs (plus maybe some new ones?) to build a
psbt for an RBF request.
RBFs utxo reuse is now working so we can unfail the test (and update
it to reflect that the lease sticks around through an RBF cycle).
Changelog-Fixed: Plugins: `funder` now honors lease requests across RBFs
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Plugins: `keysend` now removes unknown even (technically illegal!) fields, to try to accept more payments.
We filter based on the environment variable `CLN_PLUGIN_LOG`,
defaulting to `info` as that is not as noisy as `debug` or `trace`, at
least libraries will not spam us too heavily.
Changelog-Added cln-plugin: The logs level from cln-plugins can be configured by the `CLN_PLUGIN_LOG` environment variable.
We had a bit of a chicken-and-egg problem, where we instantiated the
`state` to be managed by the `Plugin` during the very first step when
creating the `Builder`, but then the state might depend on the
configuration we only get later. This would force developers to add
placeholders in the form of `Option` into the state, when really
they'd never be none after configuring.
This defers the binding until after we get the configuration and
cleans up the semantics:
- `Builder`: declare options, hooks, etc
- `ConfiguredPlugin`: we have exchanged the handshake with
`lightningd`, now we can construct the `state` accordingly
- `Plugin`: Running instance of the plugin
Changelog-Changed: cln-plugin: Moved the state binding to the plugin until after the configuration step
We will now simply reject old-style ones as invalid. Turns out the
only trace we could find is a channel between two nodes unconnected to
the rest of the network.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: Protocol: We now require all channel_update messages include htlc_maximum_msat (as per latest BOLTs)
Many changes to gossmap (including the pending ones!) don't actually
concern readers, as long as they obey certain rules:
1. Ignore unknown messages.
2. Treat all 16 upper bits of length as flags, ignore unknown ones.
So now we split the version byte into MAJOR and MINOR, and you can
ignore MINOR changes.
We don't expose the internal version (for creating the map)
programmatically: you should really hardcode what major version you
understand!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is actually what the autoclean plugin wants, especially since
you can't otherwise delete a payment which has failed then succeeded.
But insist on neither or both being specified, at least for now.
Changelog-Added: JSON-RPC: `delpay` takes optional `groupid` and `partid` parameters to specify exactly what payment to delete.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Previous commit was a hack which *always* batched where possible, this
is a more sophisticated opt-in varaint, with a timeout sanity check.
Final performance for cleaning up 1M pays/forwards/invoices:
```
$ time l1-cli autoclean-once succeededpays 1
{
"autoclean": {
"succeededpays": {
"cleaned": 1000000,
"uncleaned": 26895
}
}
}
real 6m9.828s
user 0m0.003s
sys 0m0.001s
$ time l2-cli autoclean-once succeededforwards 1
{
"autoclean": {
"succeededforwards": {
"cleaned": 1000000,
"uncleaned": 40
}
}
}
real 3m20.789s
user 0m0.004s
sys 0m0.001s
$ time l3-cli autoclean-once paidinvoices 1
{
"autoclean": {
"paidinvoices": {
"cleaned": 1000000,
"uncleaned": 0
}
}
}
real 6m47.941s
user 0m0.001s
sys 0m0.000s
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `batching` command to allow database transactions to cross multiple back-to-back JSON commands.
autoclean was using 98% of its time in memmove; we should simply keep
an offset, and memmove when it's empty. And also, only memmove the
used region, not the entire buffer!
Running on product of giantnodes.py:
$ time l1-cli autoclean-once failedpays 1
{
"autoclean": {
"failedpays": {
"cleaned": 26895,
"uncleaned": 1000000
}
}
}
Before:
real 20m46.579s
user 0m0.000s
sys 0m0.001s
After:
real 2m10.568s
user 0m0.001s
sys 0m0.000s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's more natural: we will eventually support dynamic config variables,
so this will be quite nice.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `autoclean` can now delete old forwards, payments, and invoices automatically.
And take the opportunity to rename l0 and l1 in the tests to the
more natural l1 l2 (since we add l3).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This can happen, and in fact does below in our test_autoclean_once
test where we update the datastore, and return from the cmd.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"Who needs specs?" FFS...
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Protocol: `keysend` will now attach the longest valid text field in the onion to the invoice (so you can have Sphinx.chat users spam you!)
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.
Changelog-Added: JSON-RPC: `fundchannel`, `multifundchannel` and `fundchannel_start` now accept a `reserve` parameter to indicate the absolute reserve to impose on the peer.
This is what we do in lightningd, which makes memleak much more forgiving:
you can hang temporaries off cmd without getting reports of leaks (also
when send_outreq called).
We remove all the notleak() calls in plugins which worked around this!
And avoid multiple notleak labels, since both send_outreq() and
command_still_pending() can be called multiple times.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Add memleak_ignore_children() so callers can do exclusions themselves.
Having two exclusions was always such a hack!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This allows GDB to print values, but also allows us to use them in
'case' statements. This wasn't allowed before because they're not
constant terms.
This also made it clear there's a clash between two error codes,
so move one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: Error code from bcli plugin changed from 400 to 500.
This is usually fine, but without this, commando (another branch!) has
a race:
1. A command has multiple parts.
2. We start sending them out.
3. We get a response, which completes the cmd.
4. We go to send the next one out, but it's been freed.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Shahana Farooqui
Rusty Russell
Christian Decker
Vincenzo Palazzo
Greg Sanders
Matt Morehouse
niftynei
Justin Moon
Alex Myers
adi2011
Ken Sedgwick
Matt Whitlock
Sergi Delgado Segura
antongalitch