The recover command checks if a node has already issued bitcoin
addresses before allowing recovery. This check only looked at
bip32_max_index, but with BIP86 wallets, newaddr() increments
bip86_max_index instead.
Also, the recover test asserted on hex but now it's asserting on codex32 instead. We should probably go in and fix the end point. @rustyrussell what do you think?
In fact, you *must* use mnemonic to successfully recover a modern node!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Changed: JSON-RPC: `recover` takes a 12-word mnemonic for nodes created by v25.12 or later.
We don't expect an internal command to take 5 seconds to service
without explicitly pausing: if it does, log at a higher level.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We are seeing this in the CI logs, eg tests/test_connection.py::test_reconnect_sender_add1:
lightningd-1 2025-11-17T05:48:00.665Z DEBUG jsonrpc#84: Pausing parsing after 1 requests
followed by:
lightningd-1 2025-11-17T05:48:02.068Z **BROKEN** 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-connectd: wake delay for WIRE_CHANNEL_REESTABLISH: 8512msec
So, what is consuming lightningd for 8 or so seconds?
This message helped diagnose that the issue was dev-memleak: fixed in a different branch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have a reasonable number of commands now, and we *already* keep a
strmap for the usage strings. So simply keep the usage and the command
in the map, and skip the array.
tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, sqlite3):
Time (from start to end of l2 node): 95 seconds (was 102)
Worst latency: 4.5 seconds
tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, Postgres):
Time (from start to end of l2 node): 231 seconds
Worst latency: 4.8 seconds
Note the values compare against 25.09.2 (Postgres):
sqlite3:
Time (from start to end of l2 node): 403 seconds
Postgres:
Time (from start to end of l2 node): 671 seconds
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now that ccan/io rotates through callbacks, we can call io_always() to yield.
Though it doesn't fire on our benchmark, it's a good thing to do.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: the `rpc_command` hook can now specify a "filter" on what commands it is interested in.
Profiling shows us spending all our time in tal_arr_remove when dealing
with a giant number of output streams. This applies both for RPC output
and plugin output.
Use linked list instead.
tests/test_coinmoves.py::test_generate_coinmoves (2,000,000, sqlite3):
Time (from start to end of l2 node): 239 seconds **WAS 518**
Worst latency: 56.9 seconds **WAS 353**
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we have many commands, this is where we spend all our time, and it's
just for an old assertion.
tests/test_coinmoves.py::test_generate_coinmoves (100,000, sqlite3):
Time (from start to end of l2 node): 13 seconds **WAS 34**
Worst latency: 4.0 seconds **WAS 24*
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Basically, `devtools/reduce-includes.sh */*.c`.
Build time from make clean (RUST=0) (includes building external libs):
Before:
real 0m38.944000-40.416000(40.1131+/-0.4)s
user 3m6.790000-17.159000(15.0571+/-2.8)s
sys 0m35.304000-37.336000(36.8942+/-0.57)s
After:
real 0m37.872000-39.974000(39.5466+/-0.59)s
user 3m1.211000-14.968000(12.4556+/-3.9)s
sys 0m35.008000-36.830000(36.4143+/-0.5)s
Build time after touch config.vars (RUST=0):
Before:
real 0m19.831000-21.862000(21.5528+/-0.58)s
user 2m15.361000-30.731000(28.4798+/-4.4)s
sys 0m21.056000-22.339000(22.0346+/-0.35)s
After:
real 0m18.384000-21.307000(20.8605+/-0.92)s
user 2m5.585000-26.843000(23.6017+/-6.7)s
sys 0m19.650000-22.003000(21.4943+/-0.69)s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than forcing them to wrap their parameters in a "payload"
sub-object, copy in params directly. We include the "origin" field
one level up, if they care.
The next patch restores compatibility for the one place we currently use
them, which is the pay plugin.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Deprecated: pyln-client: plugin custom notifications origins and payload (use parameters directly)
This avoids making an extra copy of the escaped string.
Note that jsonrpc_command_add() no longer accepts usage strings
containing invalid escape sequences. (Previously, it would quietly
accept such a string without unescaping anything.)
Changelog-None
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: plugins which didn't accept string JSON RPC fields (deprecated v23.08, disabled by default in v24.11).
Note: won't work with grpc (or probably other tools), since the output
is different. But good for testing.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Config: option `xpay-handle-pay` can be used to call xpay when pay is used in many cases (but output is different from pay!)
It doesn't actually log, just gets the `struct logger`, so this name is better.
We're also going to implement command_log() as an actual logging
function in next commit.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If this is set, instead of mangling the request in-place, we will hand
it through raw. Plugins will use this.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to extend it to plugins, and we want it to be allowed to be async for more power,
so rather than not completing the cmd if we're checking, do it in command_check_done()
and call it.
This is cleaner than the special case we had before, and allows check to us all the
normal jsonrpc mechanisms, especially async requests (which we'll need if we want to
hand check requests to plugins!).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were duplicating the command name (e.g. "autocleaninvoice.autocleaninvoice"), and also not
handling listconfigs if they specified the (currently unused!) i-promise-to-fix-broken-api-user
option, which we are going to use next patch.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
In general, results should always be an object, so we can add new fields
later (e.g. warnings!). But we violated this for "stop", and when "recover"
used the same infrastructure, it started doing the same thing.
I'm assuming nobody cares, so we don't need to do a deprecation cycle.
Changelog-Changed: JSON-RPC: `stop` and `recover` now return a JSON object (not a raw string!) like every other command does.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Unfortunately, this is awkward: we just copy through most requests,
so we can't easily add a "deprecation" field to each one. So we do
a notification if the next command has a different deprecation status
than the global one, but that requires opt-in from the plugin.
We didn't previously document the subscriptions array, so do that now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `deprecated_oneshot` notifiction subscription to change deprecated status for a single command.
I did some CHANGELOG and git digging to see when these were deprecated, and
some were very old (v0.8.2!). But since they didn't warn users loudly, I
chose to do so this release only.
I renamed ld's `deprecated_apis` to `deprecated_ok` to make sure I
caught them all.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We still accept boolean: the plugin may not want to commit to a deprecation schedule.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: rpcmethods and options can set `deprecated` to a pair of version strings, not just a boolean.
This command allows more fine-grained testing, without having to change the config of the
lightning node.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `deprecations` to enable/disable deprecated APIs from this caller.
Changelog-Added: JSON-RPC: `recover` command to force (unused) lightningd node to restart with `--recover` flag.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes `check` much more thorough, and useful.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `check` now does much more checking on every command (not just basic parameter types).
Put an assertion inside db.c, and run every command we do (in testing) through
a `check` variant.
I inserted a deliberate bug (made addpsbtoutput call wallet_get_newindex()
before returning when running `check`, and indeed, backtrace as expected.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We often want to do more parameter checks after param(), so allow a
new param_check(), with the proviso that the caller needs to also return
command_check_done() after other checks if command_check_only(cmd) is true.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And require --developer to use them.
Also refuse redirection to deprecated APIs if deprecated APIs are disabled!
Changelog-Removed: `dev-sendcustommsg` (use `sendcustommsg`, which was added in v0.10.1)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Also requires us to expose memleak when !DEVELOPER, however we only
ever used the memleak tracking when the LIGHTNINGD_DEV_MEMLEAK
environment variable was set, so keep that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
`struct log` becomes `struct logger`, and the member which points to the
`struct log_book` becomes `->log_book` not `->lr`.
Also, we don't need to keep the log_book in struct plugin, since it has
access to ld's log_book.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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>
The function is tiny and was only used in one location. And that one
location was leaking memory.
Detected by ASan:
==2637667==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 7 byte(s) in 1 object(s) allocated from:
#0 0x4cd758 in __interceptor_strdup
#1 0x64c70c in json_stream_log_suppress_for_cmd lightning/lightningd/jsonrpc.c:597:31
#2 0x68a630 in json_getlog lightning/lightningd/log.c:974:2
...
SUMMARY: AddressSanitizer: 7 byte(s) leaked in 1 allocation(s).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Removed: JSON-RPC: require the `"jsonrpc": "2.0"` property (requests without this deprecated in v0.10.2).
This avoids any confusion between primitive and string ids, and in
particular stops an issue with commando once it starts chaining ids,
that weird ids can be double-escaped and commando will not recognize
the response, leaving the client hanging. It's the client's fault for
using a weird id, but it's still rude (and triggered by our tests!).
It also makes substituting the id in passthrough simpler, FTW.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This reverts commit 43b037ab0b.
Nicholas Dorier says BTC Payserver still wants this for another year
or so.
Changelog-Added: JSON-RPC: reverts requirement for "jsonrpc" "2.0" inside requests (still deprecated though, just for a while longer!)
The read_json() call expects len_read to be the amount of *new*
data read. If we call this back without resetting, it will parse
this much random junk in the buffer.
Fixes: #5766
Changelog-Fixed: Plugin: `autoclean` could misperform or get killed due to lightningd's invalid handling of JSON batching.
