We should really unify the cases of a local request, vs a forwarded
request, but for now, don't steal the request onto the plugin, and
if we return from the plugin and the request is gone, don't get upset.
This uncovered a case where we weren't inside a transaction, in
test_hook_crash, so fix that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had special code to fail a forwarded request, but not for an
internally-generated request. Instead, we should pretend the (dead)
plugin responded with a PLUGIN_TERMINATED error, and handle the
request through the normal paths.
This breaks the case where a plugin crashes (or stops itself) in a
hook, so we handle that next.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was always a bit weird they weren't, and it seems a premature
optimization to make the callbacks to this themselves.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's per-plugin, so why is there a single map for all plugins? It
works because we always make unique ids, but it's weird.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: plugins can now specify (unknown) even messages we should accept from peers.
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>
If it's a plugin opt, we'll need a callback, so reshuffle logic. Also
add infra to map option name to plugin and option.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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).
This was found by tests/test_plugin.py::test_important_plugin and
was NOT a flake!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-None: only just committed
And document support for it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: `getmanfest` response can contain `nonnumericids` to indicate support for modern string-based JSON request ids.
Changelog-Deprecated: Plugins: numeric JSON request ids: modern ones will be strings (see doc/lightningd-rpc.7.md!)
because:
- shutdown_subdaemons can trigger db write, comments in that function say so at least
- resurrecting the main event loop with subdaemons still running is counter productive
in shutting down activity (such as htlc's, hook_calls etc.)
- custom behavior injected by plugins via hooks should be consistent, see test
in previous commmit
IDEA:
in shutdown_plugins, when starting new io_loop:
- A plugin that is still running can return a jsonrpc_request response, this triggers
response_cb, which cannot be handled because subdaemons are gone -> so any response_cb should be blocked/aborted
- jsonrpc is still there, so users (such as plugins) can make new jsonrpc_request's which
cannot be handled because subdaemons are gone -> so new rpc_request should also be blocked
- But we do want to send/receive notifications and log messages (handled in jsonrpc as jsonrpc_notification)
as these do not trigger subdaemon calls or db_write's
Log messages and notifications do not have "id" field, where jsonrpc_request *do* have an "id" field
PLAN (hypothesis):
- hack into plugin_read_json_one OR plugin_response_handle to filter-out json with
an "id" field, this should
block/abandon any jsonrpc_request responses (and new jsonrpc_requests for plugins?)
Q. Can internal (so not via plugin) jsonrpc_requests called in the main io_loop return/revive in
the shutdown io_loop?
A. No. All code under lightningd/ returning command_still_pending depends on either a subdaemon, timer or
plugin. In shutdown loop the subdaemons are dead, timer struct cleared and plugins will be taken
care of (in next commits).
fixup: we can only io_break the main io_loop once
This adds a `u32 checksum` field to the plugin struct that is used
to identify if a plugin is outdated and needs to be restarted on `rescan`.
Note: Only affects non-important plugins.
Changelog-Added: Plugin: Restart plugin on `rescan` when binary was changed.
Makes it possible to write a decent JSON schema, but means we need to carry
additional data, so we create a `struct plugin_command`.
We remove the unused struct dynamic_plugin, too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We will eventually start emitting and dispatching custom notifications
from plugins just like we dispatch internal notifications. In order to
get reasonable error messages we need to make sure that the topics
plugins are asking for were correctly registered. When doing this we
don't really care about whether the plugin that registered the
notification is still alive or not (it might have died, but
subscribers should stay up and running), so we keep a list of all
topics attached to the `struct plugins` which gathers global plugin
information.
We currently log every kill at INFO level, even if it's during shutdown.
Change those to debug, but lift those where we got a malformed response.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
"multi" means that specifying a parameter twice will append, not override.
Multi args are always given as a JSON array, even if only one.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: new "multi" field allows an option to be specified multiple times.
We previously registered hooks up in who-replies-to-getmanifest-first
order, but then if any had dependencies it would scatter that order.
This allows users to manually set dependencies developers have
forgotten by specifying the plugins manually in their configuration or
cmdline. This was an excellent consideration by @mschmook.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
time lightning-cli -R --network=regtest --lightning-dir /tmp/ltests-k8jhvtty/test_pay_stress_1/lightning-1/ listpays > /dev/null
Before:
real 0m42.741s
user 0m0.149s
sys 0m0.016s
After:
real 0m13.674s
user 0m0.131s
sys 0m0.024s
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: JSON-RPC: significant speedups for plugins which create large JSON replies (e.g. listpays on large nodes).
This lets us handle it the same way we handle builtin commands, and also
lets us warn if they use deprecated apis and allow-deprecated-apis=false.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: Plugins: can now mark their options and commands deprecated.
Changelog-Added: New option `--important-plugin` loads a plugin is so important that if it dies, `lightningd` will exit rather than continue. You can still `--disable-plugin` it, however, which trumps `--important-plugin` and it will not be started at all.
The previous implementation was a bit lazy: in particular, since we didn't
remember the disabled plugins, we would load them on rescan.
Changelog-Changed: config: the `plugin-disable` option works even if specified before the plugin is found.
That's more convenient for most callers, which don't need a fmt.
Fixed-by: Darosior <darosior@protonmail.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will allow the dynamic starting code to use them too.
Also lets us move dev_debug_subprocess under #if DEVELOPER.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This will let us unify the startup and runtime-started infrastructure.
Note that there are two kinds of notifications:
1. Starting a single plugin (i.e. `plugin start`)
2. Starting multiple plugins (i.e. `plugin rescan` or `plugin startdir`).
In the latter case, we want the command to complete only once *all*
the plugins are dead/finished.
We also call plugin_kill() in all cases, and correctly return afterwards
(it matters once we use the same paths for dynamic plugins, which don't
cause a fatal error if they don't startup).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The symptom (under heavy load and valgrind) in test_plugin_command:
lightningd: common/json_stream.c:237: json_stream_output_: Assertion `!js->reader' failed.
This is because we try to call `getmanifest` again on `pay` which has not yet
responded to init.
The minimal fix for this is to keep proper state, so we can tell the
difference between "not yet called getmanifest" and "not yet finished
init".
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We use the new function `plugins_free` to define the correct deallocation
order on shutdown, since under normal operation the allocation tree is
organized to allow plugins to terminate and automatically free all dependent
resources. During shutdown the deallocation order is under-defined since
siblings may get freed in any order, but we implicitly rely on them staying
around.
We now track all pending RPC passthrough calls, and terminate them with an
error if the plugin dies.
Changelog-Fixed: JSON-RPC: Pending RPC method calls are now terminated if the handling plugin exits prematurely.
Rusty Russell
Simon Vrouwe
Michael Schmoock
Christian Decker
ZmnSCPxj jxPCSnmZ