Currently, only one plugin at a time can register for each hook. If two plugins want to register the same hook, then they have to be merged into a single plugin. This gets worse, if multiple plugins want to register the same hook.
I propose to add to each hook, the ability to chain hooks.
This is useful for many things.
Currently, it is possible to connect only a single "backup" plugin to your node.
However, consider that you might want to have not just a single backup policy, but multiple:
We could write a single plugin that does all the above and maintain a complicated plugin whose many functionalities are controlled by multiple options.
Or we could instead write one plugin for each backup policy and retain simplicity, and simply connect multiple plugins to the db_hook.
In principle db_hook is a "broadcast"-type hook, with the added wrinkle that all backup plugins should be in lockstep.
htlc_acceptedCurrently we could use htlc_accepted for many usecases:
Combining the disparate intended uses into a single plugin would be ridiculously unwieldy.
When we define a hook in C-Lightning, we should add to the hook data also the below:
combiner_functiondefault_resultThen, hook callbacks no longer have to check if there is a result or not --- semantically, there is always a result.
Each plugin_hook then maintains a list of plugins connected to that hook. When a plugin is killed, it is removed from the list of plugins (care must be taken in the implementation of the list and list traversal in that a plugin may die either before, during, or after it is called on a hook).
If there is no plugin connected to the hook, the default_result is parsed as JSON and hook callbacks respond to that.
If there is at least one plugin connected, then after one plugin successfully returns in some result, we always invoke the combiner_function. It is an async function, and is give a callback (combined_result_cb) that will be invoked if it can provide the result now. Generally its choices are:
plugin_hook_combine_next(combined_result_cb, combined_result_cb_arg) - Call the next plugin; if there is a next plugin, call the combiner_function on it and do this processing again. If there is no next plugin, return default_result.combined_result_cb(buffer, toks, combined_result_cb_arg) - Return the given result immediately and do not call other plugins.Formally, the combiner function is a function you would pass to a foldr, and the default result is the zero you pass to foldr. You then invoke the list, i.e. foldr combiner_function default_result $ map (\f -> f hook_payload) registered_plugins_list.
Then, for existing hooks:
openchannel - default_result = {'result': 'continue'}, combiner_function - if result is continue then call plugin_hook_combine_next(), else call combined_result_cb with current result.peer_connected - default_result = {'result': 'continue'}, combiner_function - if result is continue then call plugin_hook_combine_next(), else call combined_result_cb with current result.htlc_accepted - default_result = {'result': 'continue'}, combiner_function - if result is continue then call plugin_hook_combine_next(), else call combined_result_cb with current result.invoice_payment - default_result = {}, combiner_function - if failure_code field does not exist, then call plugin_hook_combine_next(), else call combined_result_cb with current result.db_hook - default_result = {'result': True}, combiner_function - if result is True then call plugin_hook_combine_next(), else create a new result callback and call plugin_hook_combine_next, but return this result to the current callback when the new result callback is later invoked. This causes the db_hook to always be propagated to all plugins, but if any fail, then the hook as a whole fails and we fatal (we want to have as many backups as we can with the latest data, but if any backup fails, there is now a risk of backups getting descynched, so we have to fail: operator then has to pick the backups with most recent data and overwrite other backups manually).What do you think @rustyrussell @cdecker @niftynei ?
PING.
Fixes #2666
It would be better if self-paying plugins just use htlc_accepted (as it would not leave any traces in C-Lightning memory at all) instead of paycodes but that requires that we support chaining (because otherwise you will be unable to deploy multiple plugins that need htlc_accepted).
@cdecker suggests in https://github.com/ElementsProject/lightning/pull/2794#issuecomment-508918251 to make plugins within plugins, but please. In any case the result of hooks is mostly a Monoid type and the combiner_function is the associative binary operation and the default_result is the identity element. So we could just as well implement the Monoid operation in C-Lightning and dispense with the plugin-within-plugin structure.
This is useful for many things.
This sounds like the swiss-pocket-knife :-). Can you name one concrete example or user that currently need this?
TBH I find it hard to keep up with all the changes, but that's probably because I am not a computer scientist. What is the risk-reward ratio (i.e. risk for bugs/confusion)?
I'm coming around to this idea, I must've been too short-sighted, sorry @ZmnSCPxj.
My concrete case is that I'm starting to pile up a bunch of plugins that are using htlc_accepted (our most prominent hook so far), and I'm tempted to have a cascade of hooks. Other hooks might require different semantics, so I think this is a per-hook type:
{"result": "continue"} call the next one, until one responds with a concrete action, or we reach the end of the chain at which point we always call the default internal actionI think we should be able to get the first ones working rather quickly, but the interface for the latter needs some careful consideration.
The first and the last can be subsumed into the same mechanism. The combiner simply has to do lazy evaluation of the next, thus the internal interface I described. The interface I described is continuation-passing-style, which can be used to implement laziness semantics a la Haskell.
A number of hooks are now chainable, and while I would prefer every hook to be chainable, the existing set is fine and we can move the non-chained hooks slowly towards chained-ness.