Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order specifications for hooks #4168

Merged
merged 12 commits into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions common/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ void clear_softref_(const tal_t *outer, size_t outersize, void **ptr);
/* Note: p is never a complex expression, otherwise this multi-evaluates! */
#define tal_arr_expand(p, s) \
do { \
size_t n = tal_count(*(p)); \
tal_resize((p), n+1); \
(*(p))[n] = (s); \
size_t n_ = tal_count(*(p)); \
tal_resize((p), n_+1); \
(*(p))[n_] = (s); \
} while(0)

/**
Expand Down
22 changes: 18 additions & 4 deletions contrib/pyln-client/pyln/client/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ def __init__(self, name: str, func: Callable[..., JSONType],
self.desc = desc
self.long_desc = long_desc
self.deprecated = deprecated
self.before: List[str] = []
self.after: List[str] = []


class Request(dict):
Expand Down Expand Up @@ -405,7 +407,9 @@ def decorator(f: Callable[..., JSONType]) -> Callable[..., JSONType]:
return decorator

def add_hook(self, name: str, func: Callable[..., JSONType],
background: bool = False) -> None:
background: bool = False,
before: Optional[List[str]] = None,
after: Optional[List[str]] = None) -> None:
"""Register a hook that is called synchronously by lightningd on events
"""
if name in self.methods:
Expand All @@ -427,15 +431,23 @@ def add_hook(self, name: str, func: Callable[..., JSONType],

method = Method(name, func, MethodType.HOOK)
method.background = background
method.before = []
if before:
method.before = before
method.after = []
if after:
method.after = after
self.methods[name] = method

def hook(self, method_name: str) -> JsonDecoratorType:
def hook(self, method_name: str,
before: List[str] = None,
after: List[str] = None) -> JsonDecoratorType:
Comment on lines +443 to +444
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now becomes an Optional[List[str]]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for forgetting this in my first round of comments :-)

"""Decorator to add a plugin hook to the dispatch table.

Internally uses add_hook.
"""
def decorator(f: Callable[..., JSONType]) -> Callable[..., JSONType]:
self.add_hook(method_name, f, background=False)
self.add_hook(method_name, f, background=False, before=before, after=after)
return f
return decorator

Expand Down Expand Up @@ -689,7 +701,9 @@ def _getmanifest(self, **kwargs) -> JSONType:
continue

if method.mtype == MethodType.HOOK:
hooks.append(method.name)
hooks.append({'name': method.name,
'before': method.before,
'after': method.after})
continue

doc = inspect.getdoc(method.func)
Expand Down
23 changes: 17 additions & 6 deletions doc/PLUGINS.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ example:
"disconnect"
],
"hooks": [
"openchannel",
"htlc_accepted"
{ "name": "openchannel", "before": ["another_plugin"] },
{ "name": "htlc_accepted" }
],
"features": {
"node": "D0000000",
Expand Down Expand Up @@ -678,14 +678,25 @@ declares that it'd like to be consulted on what to do next for certain
events in the daemon. A hook can then decide how `lightningd` should
react to the given event.

When hooks are registered, they can optionally specify "before" and
"after" arrays of plugin names, which control what order they will be
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General question about 'plugin names': Is this same as for cli plugin start/stop commands, meaning either full path (useless for this feature) or basename? If so, we may note that the plugin name includes its extension, meaning full filename.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, filename or full path name.

called in. If a plugin name is unknown, it is ignored, otherwise if the
hook calls cannot be ordered to satisfy the specifications of all
plugin hooks, the plugin registration will fail.

The call semantics of the hooks, i.e., when and how hooks are called, depend
on the hook type. Most hooks are currently set to `single`-mode. In this mode
only a single plugin can register the hook, and that plugin will get called
for each event of that type. If a second plugin attempts to register the hook
it gets killed and a corresponding log entry will be added to the logs. In
`chain`-mode multiple plugins can register for the hook type and they are
called sequentially if a matching event is triggered. Each plugin can then
handle the event or defer by returning a `continue` result like the following:
it gets killed and a corresponding log entry will be added to the logs.

In `chain`-mode multiple plugins can register for the hook type and
they are called in any order they are loaded (i.e. cmdline order
first, configuration order file second: though note that the order of
plugin directories is implementation-dependent), overriden only by
`before` and `after` requirements the plugin's hook registrations specify.
Each plugin can then handle the event or defer by returning a
`continue` result like the following:

```json
{
Expand Down
9 changes: 6 additions & 3 deletions doc/lightningd-config.5
Original file line number Diff line number Diff line change
Expand Up @@ -519,14 +519,17 @@ additional paths too:

\fBplugin\fR=\fIPATH\fR
Specify a plugin to run as part of c-lightning\. This can be specified
multiple times to add multiple plugins\.
multiple times to add multiple plugins\. Note that unless plugins themselves
specify ordering requirements for being called on various hooks, plugins will
be ordered by commandline, then config file\.


\fBplugin-dir\fR=\fIDIRECTORY\fR
Specify a directory to look for plugins; all executable files not
containing punctuation (other than \fI\.\fR, \fI-\fR or \fI_) in 'DIRECTORY\fR are
loaded\. \fIDIRECTORY\fR must exist; this can be specified multiple times to
add multiple directories\.
add multiple directories\. The ordering of plugins within a directory
is currently unspecified\.


\fBclear-plugins\fR
Expand Down Expand Up @@ -581,4 +584,4 @@ Main web site: \fIhttps://github.com/ElementsProject/lightning\fR
Note: the modules in the ccan/ directory have their own licenses, but
the rest of the code is covered by the BSD-style MIT license\.

\" SHA256STAMP:6b275a3227db6565ad3c5369b95d3eefe9472864b8ed9e6c5c768981cdf23b8f
\" SHA256STAMP:c927bd3afb61288bb67d941d113cdeefe1363b0c7a28936ef30d43af3ce66098
7 changes: 5 additions & 2 deletions doc/lightningd-config.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,16 @@ additional paths too:

**plugin**=*PATH*
Specify a plugin to run as part of c-lightning. This can be specified
multiple times to add multiple plugins.
multiple times to add multiple plugins. Note that unless plugins themselves
specify ordering requirements for being called on various hooks, plugins will
be ordered by commandline, then config file.

**plugin-dir**=*DIRECTORY*
Specify a directory to look for plugins; all executable files not
containing punctuation (other than *.*, *-* or *\_) in 'DIRECTORY* are
loaded. *DIRECTORY* must exist; this can be specified multiple times to
add multiple directories.
add multiple directories. The ordering of plugins within a directory
is currently unspecified.

**clear-plugins**
This option clears all *plugin*, *important-plugin*, and *plugin-dir* options
Expand Down
Loading