One is called on every plugin return, and tells us whether to continue;
the other is only called if every plugin says ok.
This works for things like payload replacement, where we need to process
the results from each plugin, not just the final one!
We should probably turn everything into a chained callback next
release.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They callback must take ownership of the payload (almost all do, but
now it's explicit).
And since the payload and cb_arg arguments to plugin_hook_call_() are
always identical, make them a single parameter.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was a pointer into the list of plugins for the hook, but it was rather
unstable: if a plugin exits after handling the event we could end up skipping
a later plugin. We now rely on the much more stable `call_chain` list, so we
can clean up that useless field.
We are attaching the destructor to notify us when the plugin exits, but we
also need to clear them once the request is handled correctly, so we don't
call the destructor when it exits later.
We make the current state of `lightningd` explicit so we don't have to
identify a shutdown by its side-effects. We then use this in order to prevent
the killing and freeing of plugins to continue down the chain of registered
plugins.
This used to be necessary because we allocated the `plugin_hook_request` off
of the plugin instance (only tal allocated object we could grab at that
time. Now the plugin was replaced by a list, which itself is tal-allocated,
making that workaround pointless, or even wrong once we have multiple plugins
registering for that hook.
We will be using `plugin_hook_call_next` as part of the loop to traverse all
plugins that registered the hook, so group initialization in the init function
and move per-plugin logic into `plugin_hook_call_next`
We are about to call multiple plugins, and we'll have to pass the payload into
each call. Sadly the serialized stream gets consumed during the call, so keep
the unserialized payload around.
Switch from having a single plugin to a list of plugins. If the hook is of
type single we will enforce that constraint on the number of registered
plugins when attempting to add.
The newly introduced type is used to determine what the call semantics of the
hook are. We have `single` corresponding to the old behavior, as well as
`chain` which allows multiple plugins to register for the hook, and they are
then called sequentially (if all plugins return `{"result": "continue"}`) or
exit the chain if the hook event was handled.
Changelog-Changed: The hooks `db_write`, `invoice_payment`, and `rpc_command` now accept `{ "result": "continue" }` to mean "do default action", in addition to `true` (`db_write`), `{}` (`invoice_payment`), and `{"continue": true}` (`rpc_command`). The older "default" indicators are now deprecated and are now recognized only if `--deprecated-apis` is set.
This increments the `data_version` upon committing dirty transactions, reads
the last data_version upon startup, and tracks the number in memory in
parallel to the DB (see next commit for rationale).
Changelog-Changed: JSON-RPC: Added a `data_version` field to the `db_write` hook which returns a numeric transaction counter.
This adds 'plugin_unregister_hook' and 'plugin_unregister_hook_all'
functions to unregister a given hook a plugin registered, or all hooks a
plugin registered for. Since hooks can only be registered once, it's
useful in the case a new plugin is added which would be prefered for
hook registration over an already loaded plugin.
Before:
Plugin for invoice_payment returned non-result response
"subscriptions": [], "hooks": ["invoice_payment"]}}
�V
After:
Plugin for invoice_payment returned non-result response {"jsonrpc": "2.0", "id": 6, "error": "Error while processing invoice_payment: ValueError(\"invalid literal for int() with base 10: '5.0'\")"}
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This seems like overkill, at least for now. Handling the JSON
inline is clearer, for the existing examples at least.
We also remove the dummy hook, rather than fix it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I started by trying to change the current infrastructure, but this is
really the only completely sync hook which makes sense; it needs to avoid
doing the db_transation, as well as waiting, and using a callback is just
overkill.
So with some regret, I open coded it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We therefore keep a reference to the DB and will wrap and unwrap when
a hook returns.
Notice that this might cause behavior changes when moving logic into a
hook callback, since the continuation runs in a different transaction
than the event that triggered the hook in the first place. Should not
matter too much, since we don't use DB rollbacks at the moment, but
it's something to keep in mind.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This ties all the things together, using the serializer to transform
the payload into a valid `jsonrpc_request`, sending it to the plugin,
and then using the deserializer on the way back before calling the
hook callback with the appropriate information.
Notice that the serializer and deserializer is skipped if we don't
have a plugin that registered for this hook.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
This is the first use of the `hooks` autodata field, and it required a
dummy element in order for the section not to be dropped, it'll be
removed once we have actual hooks.
I might have gone a bit overboard with the type-checking, but
typesafe_cb_cast is quite nice to use, so why not. The macro to
register a new hook encapsulates the entire flow from param
serialization, to dispatch, parsing and callback dispatch in one
bundle. I was tempted to have the callback outside of the
registration, but it's unlikely that we'll have two calls to the same
hook with different callbacks.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
