From 9a07d076598de27f4c42506bf96cb6aebb33bf2b Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Tue, 22 Jan 2019 17:45:50 +0100
Subject: [PATCH 01/10] pylightning: Wrap the plugin methods in a class

Sending around unnamed tuples is bound to cause some issues sooner or
later, so we just create a quick class that holds all the information
about a plugin method.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py | 43 +++++++++++++++++--------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index e7929bdcaebb..d0d7cec4147d 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -15,6 +15,20 @@ class MethodType(Enum):
     HOOK = 1
 
 
+class Method(object):
+    """Description of methods that are registered with the plugin.
+
+    These can be one of the following:
+
+     - RPC exposed by RPC passthrough
+     - HOOK registered to be called synchronously by lightningd
+    """
+    def __init__(self, name, func, mtype=MethodType.RPCMETHOD):
+        self.name = name
+        self.func = func
+        self.mtype = mtype
+
+
 class Plugin(object):
     """Controls interactions with lightningd, and bundles functionality.
 
@@ -25,7 +39,7 @@ class Plugin(object):
     """
 
     def __init__(self, stdout=None, stdin=None, autopatch=True):
-        self.methods = {'init': (self._init, MethodType.RPCMETHOD)}
+        self.methods = {'init': Method('init', self._init, MethodType.RPCMETHOD)}
         self.options = {}
 
         # A dict from topics to handler functions
@@ -72,7 +86,8 @@ def add_method(self, name, func):
             )
 
         # Register the function with the name
-        self.methods[name] = (func, MethodType.RPCMETHOD)
+        method = Method(name, func, MethodType.RPCMETHOD)
+        self.methods[name] = method
 
     def add_subscription(self, topic, func):
         """Add a subscription to our list of subscriptions.
@@ -146,7 +161,8 @@ def add_hook(self, name, func):
             raise ValueError(
                 "Method {} was already registered".format(name, self.methods[name])
             )
-        self.methods[name] = (func, MethodType.HOOK)
+        method = Method(name, func, MethodType.HOOK)
+        self.methods[name] = method
 
     def hook(self, method_name):
         """Decorator to add a plugin hook to the dispatch table.
@@ -211,13 +227,13 @@ def _dispatch_request(self, request):
 
         if name not in self.methods:
             raise ValueError("No method {} found.".format(name))
-        func, _ = self.methods[name]
+        method = self.methods[name]
 
         try:
             result = {
                 'jsonrpc': '2.0',
                 'id': request['id'],
-                'result': self._exec_func(func, request)
+                'result': self._exec_func(method.func, request)
             }
         except Exception as e:
             result = {
@@ -291,27 +307,26 @@ def run(self):
     def _getmanifest(self, **kwargs):
         methods = []
         hooks = []
-        for name, entry in self.methods.items():
-            func, typ = entry
+        for method in self.methods.values():
             # Skip the builtin ones, they don't get reported
-            if name in ['getmanifest', 'init']:
+            if method.name in ['getmanifest', 'init']:
                 continue
 
-            if typ == MethodType.HOOK:
-                hooks.append(name)
+            if method.mtype == MethodType.HOOK:
+                hooks.append(method.name)
                 continue
 
-            doc = inspect.getdoc(func)
+            doc = inspect.getdoc(method.func)
             if not doc:
                 self.log(
-                    'RPC method \'{}\' does not have a docstring.'.format(name)
+                    'RPC method \'{}\' does not have a docstring.'.format(method.name)
                 )
                 doc = "Undocumented RPC method from a plugin."
             doc = re.sub('\n+', ' ', doc)
 
             # Handles out-of-order use of parameters like:
             # def hello_obfus(arg1, arg2, plugin, thing3, request=None, thing5='at', thing6=21)
-            argspec = inspect.getargspec(func)
+            argspec = inspect.getargspec(method.func)
             defaults = argspec.defaults
             num_defaults = len(defaults) if defaults else 0
             start_kwargs_idx = len(argspec.args) - num_defaults
@@ -327,7 +342,7 @@ def _getmanifest(self, **kwargs):
                     args.append("[%s]" % arg)
 
             methods.append({
-                'name': name,
+                'name': method.name,
                 'usage': " ".join(args),
                 'description': doc
             })

From 25e12fc3b18747a4c984c67b9bc2a69e4f17ef1f Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Tue, 22 Jan 2019 19:25:00 +0100
Subject: [PATCH 02/10] pylightning: Wrap request in an object

We well need this in the next commit to be able to return from an
asynchronous call. We also guard stdout access with a reentrant lock
since we are no longer guaranteed that all communication happens on
the same thread.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py      | 116 ++++++++++++++-----
 contrib/pylightning/lightning/test_plugin.py |  27 +++--
 2 files changed, 103 insertions(+), 40 deletions(-)

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index d0d7cec4147d..f953af518244 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -1,6 +1,7 @@
 from collections import OrderedDict
-from lightning import LightningRpc
 from enum import Enum
+from lightning import LightningRpc
+from threading import RLock
 
 import inspect
 import json
@@ -15,6 +16,12 @@ class MethodType(Enum):
     HOOK = 1
 
 
+class RequestState(Enum):
+    PENDING = 'pending'
+    FINISHED = 'finished'
+    FAILED = 'failed'
+
+
 class Method(object):
     """Description of methods that are registered with the plugin.
 
@@ -27,6 +34,59 @@ def __init__(self, name, func, mtype=MethodType.RPCMETHOD):
         self.name = name
         self.func = func
         self.mtype = mtype
+        self.background = False
+
+
+class Request(dict):
+    """A request object that wraps params and allows async return
+    """
+    def __init__(self, plugin, req_id, method, params, background=False):
+        self.method = method
+        self.params = params
+        self.background = background
+        self.plugin = plugin
+        self.state = RequestState.PENDING
+        self.id = req_id
+
+    def getattr(self, key):
+        if key == "params":
+            return self.params
+        elif key == "id":
+            return self.id
+        elif key == "method":
+            return self.method
+
+    def set_result(self, result):
+        if self.state != RequestState.PENDING:
+            raise ValueError(
+                "Cannot set the result of a request that is not pending, "
+                "current state is {state}".format(self.state))
+        self.result = result
+        self._write_result({
+            'jsonrpc': '2.0',
+            'id': self.id,
+            'result': self.result
+        })
+
+    def set_exception(self, exc):
+        if self.state != RequestState.PENDING:
+            raise ValueError(
+                "Cannot set the exception of a request that is not pending, "
+                "current state is {state}".format(self.state))
+        self.exc = exc
+        self._write_result({
+            'jsonrpc': '2.0',
+            'id': self.id,
+            "error": "Error while processing {method}: {exc}".format(
+                method=self.method, exc=repr(exc)
+            ),
+        })
+
+    def _write_result(self, result):
+        with self.plugin.write_lock:
+            json.dump(result, fp=self.plugin.stdout)
+            self.plugin.stdout.write('\n\n')
+            self.plugin.stdout.flush()
 
 
 class Plugin(object):
@@ -59,6 +119,8 @@ def __init__(self, stdout=None, stdin=None, autopatch=True):
         self.rpc = None
         self.child_init = None
 
+        self.write_lock = RLock()
+
     def add_method(self, name, func):
         """Add a plugin method to the dispatch table.
 
@@ -185,7 +247,7 @@ def decorator(f):
         return decorator
 
     def _exec_func(self, func, request):
-        params = request['params']
+        params = request.params
         sig = inspect.signature(func)
 
         arguments = OrderedDict()
@@ -223,36 +285,30 @@ def _exec_func(self, func, request):
         return func(*ba.args, **ba.kwargs)
 
     def _dispatch_request(self, request):
-        name = request['method']
+        name = request.method
 
         if name not in self.methods:
             raise ValueError("No method {} found.".format(name))
         method = self.methods[name]
+        request.background = method.background
 
         try:
-            result = {
-                'jsonrpc': '2.0',
-                'id': request['id'],
-                'result': self._exec_func(method.func, request)
-            }
+            result = self._exec_func(method.func, request)
+            if not method.background:
+                # Only if this is not an async (background) call do we need to
+                # return the result, otherwise the callee will eventually need
+                # to call request.set_result or request.set_exception to
+                # return a result or raise an exception.
+                request.set_result(result)
         except Exception as e:
-            result = {
-                'jsonrpc': '2.0',
-                'id': request['id'],
-                "error": "Error while processing {}: {}".format(
-                    request['method'], repr(e)
-                ),
-            }
+            request.set_exception(e)
             self.log(traceback.format_exc())
-        json.dump(result, fp=self.stdout)
-        self.stdout.write('\n\n')
-        self.stdout.flush()
 
     def _dispatch_notification(self, request):
-        name = request['method']
-        if name not in self.subscriptions:
-            raise ValueError("No subscription for {} found.".format(name))
-        func = self.subscriptions[name]
+        if request.method not in self.subscriptions:
+            raise ValueError("No subscription for {name} found.".format(
+                name=request.method))
+        func = self.subscriptions[request.method]
 
         try:
             self._exec_func(func, request)
@@ -265,9 +321,10 @@ def notify(self, method, params):
             'method': method,
             'params': params,
         }
-        json.dump(payload, self.stdout)
-        self.stdout.write("\n\n")
-        self.stdout.flush()
+        with self.write_lock:
+            json.dump(payload, self.stdout)
+            self.stdout.write("\n\n")
+            self.stdout.flush()
 
     def log(self, message, level='info'):
         # Split the log into multiple lines and print them
@@ -282,11 +339,18 @@ def _multi_dispatch(self, msgs):
         """
         for payload in msgs[:-1]:
             request = json.loads(payload)
+            request = Request(
+                plugin=self,
+                req_id=request.get('id', None),
+                method=request['method'],
+                params=request['params'],
+                background=False,
+            )
 
             # If this has an 'id'-field, it's a request and returns a
             # result. Otherwise it's a notification and it doesn't
             # return anything.
-            if 'id' in request:
+            if request.id is not None:
                 self._dispatch_request(request)
             else:
                 self._dispatch_notification(request)
diff --git a/contrib/pylightning/lightning/test_plugin.py b/contrib/pylightning/lightning/test_plugin.py
index 07d2c8ca2f6a..0c90dea9ff87 100644
--- a/contrib/pylightning/lightning/test_plugin.py
+++ b/contrib/pylightning/lightning/test_plugin.py
@@ -1,22 +1,21 @@
-from .plugin import Plugin
+from .plugin import Plugin, Request
 import itertools
 
 
 def test_positional_inject():
     p = Plugin()
-    rdict = {
-        'id': 1,
-        'jsonrpc':
-        '2.0',
-        'method': 'func',
-        'params': {'a': 1, 'b': 2, 'kwa': 3, 'kwb': 4}
-    }
-    rarr = {
-        'id': 1,
-        'jsonrpc': '2.0',
-        'method': 'func',
-        'params': [1, 2, 3, 4]
-    }
+    rdict = Request(
+        plugin=p,
+        req_id=1,
+        method='func',
+        params={'a': 1, 'b': 2, 'kwa': 3, 'kwb': 4}
+    )
+    rarr = Request(
+        plugin=p,
+        req_id=1,
+        method='func',
+        params=[1, 2, 3, 4],
+    )
 
     def pre_args(plugin, a, b, kwa=3, kwb=4):
         assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)

From 9f3502cd7fe584879acc8c22000270fd758bb2a9 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Tue, 22 Jan 2019 23:09:42 +0100
Subject: [PATCH 03/10] pylightning: Exception if we have unfulfilled
 positional arguments

This caused me to backtrack quite a bit, so this should help debugging
in the future.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py      | 13 +++++++++++--
 contrib/pylightning/lightning/test_plugin.py | 20 ++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index f953af518244..9d897167d790 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -252,7 +252,7 @@ def _exec_func(self, func, request):
 
         arguments = OrderedDict()
         for name, value in sig.parameters.items():
-            arguments[name] = inspect.Signature.empty
+            arguments[name] = inspect._empty
 
         # Fill in any injected parameters
         if 'plugin' in arguments:
@@ -267,7 +267,7 @@ def _exec_func(self, func, request):
         else:
             pos = 0
             for k, v in arguments.items():
-                if v is not inspect.Signature.empty:
+                if v != inspect._empty:
                     continue
                 if pos < len(params):
                     # Apply positional args if we have them
@@ -280,6 +280,15 @@ def _exec_func(self, func, request):
                     arguments[k] = sig.parameters[k].default
                 pos += 1
 
+        missing = [k for k, v in arguments.items() if v == inspect._empty]
+        if missing:
+            raise TypeError("Missing positional arguments ({given} given, "
+                            "expected {expected}): {missing}".format(
+                                missing=", ".join(missing),
+                                given=len(arguments) - len(missing),
+                                expected=len(arguments)
+                            ))
+
         ba = sig.bind(**arguments)
         ba.apply_defaults()
         return func(*ba.args, **ba.kwargs)
diff --git a/contrib/pylightning/lightning/test_plugin.py b/contrib/pylightning/lightning/test_plugin.py
index 0c90dea9ff87..9293fd69a66c 100644
--- a/contrib/pylightning/lightning/test_plugin.py
+++ b/contrib/pylightning/lightning/test_plugin.py
@@ -1,5 +1,6 @@
 from .plugin import Plugin, Request
 import itertools
+import pytest
 
 
 def test_positional_inject():
@@ -42,9 +43,28 @@ def extra_def_arg(a, b, c, d, e=42):
         """
         assert (a, b, c, d, e) == (1, 2, 3, 4, 42)
 
+    def count(plugin, count, request):
+        assert count == 42 and plugin == p
+
     funcs = [pre_args, in_args, post_args, post_kwargs, in_multi_args]
 
     for func, request in itertools.product(funcs, [rdict, rarr]):
         p._exec_func(func, request)
 
     p._exec_func(extra_def_arg, rarr)
+
+    p._exec_func(count, Request(
+        plugin=p,
+        req_id=1,
+        method='func',
+        params=[42],
+    ))
+
+    # This should fail since it is missing one positional argument
+    with pytest.raises(ValueError):
+        p._exec_func(count, Request(
+            plugin=p,
+            req_id=1,
+            method='func',
+            params=[])
+        )

From 0d2431c23522b9647221fb29f557beaecb7f8e02 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Thu, 21 Feb 2019 18:30:15 +0100
Subject: [PATCH 04/10] pylightning: Add support for *args and **kwargs in
 plugin dispatch

These are a bit special and are handled separately.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index 9d897167d790..6b38ec643d71 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -261,14 +261,22 @@ def _exec_func(self, func, request):
         if 'request' in arguments:
             arguments['request'] = request
 
+        args = []
+        kwargs = {}
         # Now zip the provided arguments and the prefilled a together
         if isinstance(params, dict):
-            arguments.update(params)
+            for k, v in params.items():
+                if k in arguments:
+                    arguments[k] = v
+                else:
+                    kwargs[k] = v
         else:
             pos = 0
             for k, v in arguments.items():
-                if v != inspect._empty:
+                # Skip already assigned args and special catch-all args
+                if v != inspect._empty or k in ['args', 'kwargs']:
                     continue
+
                 if pos < len(params):
                     # Apply positional args if we have them
                     arguments[k] = params[pos]
@@ -279,6 +287,18 @@ def _exec_func(self, func, request):
                     # For the remainder apply default args
                     arguments[k] = sig.parameters[k].default
                 pos += 1
+            if len(arguments) < len(params):
+                args = params[len(arguments):]
+
+        if 'kwargs' in arguments:
+            arguments['kwargs'] = kwargs
+        elif len(kwargs) > 0:
+            raise TypeError("Extra arguments given: {kwargs}".format(kwargs=kwargs))
+
+        if 'args' in arguments:
+            arguments['args'] = args
+        elif len(args) > 0:
+            raise TypeError("Extra arguments given: {args}".format(args=args))
 
         missing = [k for k, v in arguments.items() if v == inspect._empty]
         if missing:

From 661cf2570d4a2e4ee0464d8d36c33bebea46de9d Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Tue, 22 Jan 2019 23:11:37 +0100
Subject: [PATCH 05/10] pylightning: Don't always use request ID 0

This isn't a problem for now since we don't support multithreading,
and only allow synchronous calls, but eventually this'll become
important.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/lightning.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/contrib/pylightning/lightning/lightning.py b/contrib/pylightning/lightning/lightning.py
index fd0c22ce3f08..779e453985e6 100644
--- a/contrib/pylightning/lightning/lightning.py
+++ b/contrib/pylightning/lightning/lightning.py
@@ -130,6 +130,7 @@ def __init__(self, socket_path, executor=None, logger=logging, encoder=json.JSON
 
         # Do we require the compatibility mode?
         self._compat = True
+        self.next_id = 0
 
     def _writeobj(self, sock, obj):
         s = json.dumps(obj, cls=self.encoder)
@@ -208,8 +209,9 @@ def call(self, method, payload=None):
         self._writeobj(sock, {
             "method": method,
             "params": payload,
-            "id": 0
+            "id": self.next_id,
         })
+        self.next_id += 1
         resp, _ = self._readobj_compat(sock)
         sock.close()
 

From 7e65b50b2e2c16905448a386dccb732f733514ed Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Tue, 22 Jan 2019 23:13:08 +0100
Subject: [PATCH 06/10] pylightning: Add the `background` keyword to hooks and
 methods

This indicates that the method or hook will accepts a request
parameter, and will use that to return the result or raise an
exception instead of returning the return value. This allows the hook
or method to stash the incomplete request or pass it around, without
blocking the JSON-RPC interface.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index 6b38ec643d71..57b66a06d537 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -113,7 +113,7 @@ def __init__(self, stdout=None, stdin=None, autopatch=True):
         if os.getenv('LIGHTNINGD_PLUGIN') and autopatch:
             monkey_patch(self, stdout=True, stderr=True)
 
-        self.add_method("getmanifest", self._getmanifest)
+        self.add_method("getmanifest", self._getmanifest, background=False)
         self.rpc_filename = None
         self.lightning_dir = None
         self.rpc = None
@@ -121,7 +121,7 @@ def __init__(self, stdout=None, stdin=None, autopatch=True):
 
         self.write_lock = RLock()
 
-    def add_method(self, name, func):
+    def add_method(self, name, func, background=False):
         """Add a plugin method to the dispatch table.
 
         The function will be expected at call time (see `_dispatch`)
@@ -141,6 +141,13 @@ def add_method(self, name, func):
         plugin and request argument should always be the last two
         arguments and have a default on None.
 
+        The `background` argument can be used to specify whether the method is
+        going to return a result that should be sent back to the lightning
+        daemon (`background=False`) or whether the method will return without
+        sending back a result. In the latter case the method MUST use
+        `request.set_result` or `result.set_exception` to return a result or
+        raise an exception for the call.
+
         """
         if name in self.methods:
             raise ValueError(
@@ -149,6 +156,7 @@ def add_method(self, name, func):
 
         # Register the function with the name
         method = Method(name, func, MethodType.RPCMETHOD)
+        method.background = background
         self.methods[name] = method
 
     def add_subscription(self, topic, func):
@@ -206,17 +214,17 @@ def get_option(self, name):
         else:
             return self.options[name]['default']
 
-    def method(self, method_name, *args, **kwargs):
+    def method(self, method_name, background=True):
         """Decorator to add a plugin method to the dispatch table.
 
         Internally uses add_method.
         """
         def decorator(f):
-            self.add_method(method_name, f)
+            self.add_method(method_name, f, background=background)
             return f
         return decorator
 
-    def add_hook(self, name, func):
+    def add_hook(self, name, func, background=False):
         """Register a hook that is called synchronously by lightningd on events
         """
         if name in self.methods:
@@ -224,15 +232,16 @@ def add_hook(self, name, func):
                 "Method {} was already registered".format(name, self.methods[name])
             )
         method = Method(name, func, MethodType.HOOK)
+        method.background = background
         self.methods[name] = method
 
-    def hook(self, method_name):
+    def hook(self, method_name, background=False):
         """Decorator to add a plugin hook to the dispatch table.
 
         Internally uses add_hook.
         """
         def decorator(f):
-            self.add_hook(method_name, f)
+            self.add_hook(method_name, f, background=background)
             return f
         return decorator
 

From 73bad4cff69e450a68c7b09d847c98cbc45e5104 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Tue, 22 Jan 2019 23:23:34 +0100
Subject: [PATCH 07/10] pylightning: Add a small test for async rpcmethods

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 tests/plugins/asynctest.py | 30 ++++++++++++++++++++++++++++++
 tests/test_plugin.py       | 25 +++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100755 tests/plugins/asynctest.py

diff --git a/tests/plugins/asynctest.py b/tests/plugins/asynctest.py
new file mode 100755
index 000000000000..9e8c849ab96d
--- /dev/null
+++ b/tests/plugins/asynctest.py
@@ -0,0 +1,30 @@
+#!/usr/bin/env python3
+"""This plugin is used to check that async method calls are working correctly.
+
+The plugin registers a method `callme` with an argument. All calls are
+stashed away, and are only resolved on the fifth invocation. All calls
+will then return the argument of the fifth call.
+
+"""
+from lightning import Plugin
+
+plugin = Plugin()
+
+
+@plugin.init()
+def init(configuration, options, plugin):
+    plugin.requests = []
+
+
+@plugin.method('asyncqueue', sync=False)
+def async_queue(request, plugin):
+    plugin.requests.append(request)
+
+
+@plugin.method('asyncflush')
+def async_flush(res, plugin):
+    for r in plugin.requests:
+        r.set_result(res)
+
+
+plugin.run()
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index a21adb0f7c2c..9706c3c6009b 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -5,6 +5,7 @@
 
 import pytest
 import subprocess
+import time
 
 
 def test_option_passthrough(node_factory):
@@ -146,3 +147,27 @@ def test_plugin_connected_hook(node_factory):
 
     peer = l1.rpc.listpeers(l3.info['id'])['peers']
     assert(peer == [] or not peer[0]['connected'])
+
+
+def test_async_rpcmethod(node_factory, executor):
+    """This tests the async rpcmethods.
+
+    It works in conjunction with the `asynctest` plugin which stashes
+    requests and then resolves all of them on the fifth call.
+    """
+    l1 = node_factory.get_node(options={'plugin': 'tests/plugins/asynctest.py'})
+
+    results = []
+    for i in range(10):
+        results.append(executor.submit(l1.rpc.asyncqueue))
+
+    time.sleep(3)
+
+    # None of these should have returned yet
+    assert len([r for r in results if r.done()]) == 0
+
+    # This last one triggers the release and all results should be 42,
+    # since the last number is returned for all
+    l1.rpc.asyncflush(42)
+
+    assert [r.result() for r in results] == [42] * len(results)

From 130afb0ad2e3dcea3169d3eeb66870a700a2ad57 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Thu, 24 Jan 2019 16:57:56 +0100
Subject: [PATCH 08/10] pylightning: Rename peer_id to node_id in getroute

Technically this is a node, not a direct peer, so this is correct.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/lightning.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/pylightning/lightning/lightning.py b/contrib/pylightning/lightning/lightning.py
index 779e453985e6..31ed1f05e0a9 100644
--- a/contrib/pylightning/lightning/lightning.py
+++ b/contrib/pylightning/lightning/lightning.py
@@ -293,7 +293,7 @@ def listnodes(self, node_id=None):
         }
         return self.call("listnodes", payload)
 
-    def getroute(self, peer_id, msatoshi, riskfactor, cltv=9, fromid=None, fuzzpercent=None, seed=None, exclude=[]):
+    def getroute(self, node_id, msatoshi, riskfactor, cltv=9, fromid=None, fuzzpercent=None, seed=None, exclude=[]):
         """
         Show route to {id} for {msatoshi}, using {riskfactor} and optional
         {cltv} (default 9). If specified search from {fromid} otherwise use
@@ -302,7 +302,7 @@ def getroute(self, peer_id, msatoshi, riskfactor, cltv=9, fromid=None, fuzzperce
         seed. {exclude} is an optional array of scid/direction to exclude.
         """
         payload = {
-            "id": peer_id,
+            "id": node_id,
             "msatoshi": msatoshi,
             "riskfactor": riskfactor,
             "cltv": cltv,

From 832ba0a6bde46b2602b255b8510c33fcac713096 Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Thu, 21 Feb 2019 15:22:07 +0100
Subject: [PATCH 09/10] pylightning: Split @method and @async_method decorators

Suggested-by: Rusty Russell <@rustyrussell>
Suggested-by: Conor Scott <@conscott>
Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 contrib/pylightning/lightning/plugin.py | 28 +++++++++++++++++++++----
 tests/plugins/asynctest.py              |  2 +-
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index 57b66a06d537..b0cdbd3ad185 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -214,13 +214,23 @@ def get_option(self, name):
         else:
             return self.options[name]['default']
 
-    def method(self, method_name, background=True):
+    def async_method(self, method_name):
+        """Decorator to add an async plugin method to the dispatch table.
+
+        Internally uses add_method.
+        """
+        def decorator(f):
+            self.add_method(method_name, f, background=True)
+            return f
+        return decorator
+
+    def method(self, method_name):
         """Decorator to add a plugin method to the dispatch table.
 
         Internally uses add_method.
         """
         def decorator(f):
-            self.add_method(method_name, f, background=background)
+            self.add_method(method_name, f, background=False)
             return f
         return decorator
 
@@ -235,13 +245,23 @@ def add_hook(self, name, func, background=False):
         method.background = background
         self.methods[name] = method
 
-    def hook(self, method_name, background=False):
+    def hook(self, method_name):
         """Decorator to add a plugin hook to the dispatch table.
 
         Internally uses add_hook.
         """
         def decorator(f):
-            self.add_hook(method_name, f, background=background)
+            self.add_hook(method_name, f, background=False)
+            return f
+        return decorator
+
+    def async_hook(self, method_name):
+        """Decorator to add an async plugin hook to the dispatch table.
+
+        Internally uses add_hook.
+        """
+        def decorator(f):
+            self.add_hook(method_name, f, background=True)
             return f
         return decorator
 
diff --git a/tests/plugins/asynctest.py b/tests/plugins/asynctest.py
index 9e8c849ab96d..97dd4df61db9 100755
--- a/tests/plugins/asynctest.py
+++ b/tests/plugins/asynctest.py
@@ -16,7 +16,7 @@ def init(configuration, options, plugin):
     plugin.requests = []
 
 
-@plugin.method('asyncqueue', sync=False)
+@plugin.async_method('asyncqueue')
 def async_queue(request, plugin):
     plugin.requests.append(request)
 

From 93be4d9d1c468262bc679f841ea7e0947ab5054a Mon Sep 17 00:00:00 2001
From: Christian Decker <decker.christian@gmail.com>
Date: Thu, 21 Feb 2019 18:59:15 +0100
Subject: [PATCH 10/10] pylightning: Add plugin dispatch tests to check-python
 and fix them

These weren't checked by CI yet, and they are really short so I just added
them to the check-python target.

Signed-off-by: Christian Decker <decker.christian@gmail.com>
---
 Makefile                                     |   2 +
 contrib/pylightning/lightning/plugin.py      |  19 +--
 contrib/pylightning/lightning/test_plugin.py |  70 ----------
 contrib/pylightning/tests/test_plugin.py     | 140 ++++++++++++++-----
 4 files changed, 117 insertions(+), 114 deletions(-)
 delete mode 100644 contrib/pylightning/lightning/test_plugin.py

diff --git a/Makefile b/Makefile
index d53559ddfff6..e7be04557f3c 100644
--- a/Makefile
+++ b/Makefile
@@ -302,6 +302,8 @@ check-python:
 	@# W503: line break before binary operator
 	@flake8 --ignore=E501,E731,W503 --exclude=contrib/pylightning/lightning/__init__.py ${PYSRC}
 
+	PYTHONPATH=contrib/pylightning:$$PYTHONPATH $(PYTEST) contrib/pylightning/
+
 check-includes:
 	@tools/check-includes.sh
 
diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py
index b0cdbd3ad185..dabe4c933029 100644
--- a/contrib/pylightning/lightning/plugin.py
+++ b/contrib/pylightning/lightning/plugin.py
@@ -390,20 +390,23 @@ def log(self, message, level='info'):
         for line in message.split('\n'):
             self.notify('log', {'level': level, 'message': line})
 
+    def _parse_request(self, jsrequest):
+        request = Request(
+            plugin=self,
+            req_id=jsrequest.get('id', None),
+            method=jsrequest['method'],
+            params=jsrequest['params'],
+            background=False,
+        )
+        return request
+
     def _multi_dispatch(self, msgs):
         """We received a couple of messages, now try to dispatch them all.
 
         Returns the last partial message that was not complete yet.
         """
         for payload in msgs[:-1]:
-            request = json.loads(payload)
-            request = Request(
-                plugin=self,
-                req_id=request.get('id', None),
-                method=request['method'],
-                params=request['params'],
-                background=False,
-            )
+            request = self._parse_request(json.loads(payload))
 
             # If this has an 'id'-field, it's a request and returns a
             # result. Otherwise it's a notification and it doesn't
diff --git a/contrib/pylightning/lightning/test_plugin.py b/contrib/pylightning/lightning/test_plugin.py
deleted file mode 100644
index 9293fd69a66c..000000000000
--- a/contrib/pylightning/lightning/test_plugin.py
+++ /dev/null
@@ -1,70 +0,0 @@
-from .plugin import Plugin, Request
-import itertools
-import pytest
-
-
-def test_positional_inject():
-    p = Plugin()
-    rdict = Request(
-        plugin=p,
-        req_id=1,
-        method='func',
-        params={'a': 1, 'b': 2, 'kwa': 3, 'kwb': 4}
-    )
-    rarr = Request(
-        plugin=p,
-        req_id=1,
-        method='func',
-        params=[1, 2, 3, 4],
-    )
-
-    def pre_args(plugin, a, b, kwa=3, kwb=4):
-        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
-
-    def in_args(a, plugin, b, kwa=3, kwb=4):
-        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
-
-    def post_args(a, b, plugin, kwa=3, kwb=4):
-        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
-
-    def post_kwargs(a, b, kwa=3, kwb=4, plugin=None):
-        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
-
-    def in_multi_args(a, request, plugin, b, kwa=3, kwb=4):
-        assert request in [rarr, rdict]
-        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
-
-    def in_multi_mix_args(a, plugin, b, request=None, kwa=3, kwb=4):
-        assert request in [rarr, rdict]
-        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
-
-    def extra_def_arg(a, b, c, d, e=42):
-        """ Also uses a different name for kwa and kwb
-        """
-        assert (a, b, c, d, e) == (1, 2, 3, 4, 42)
-
-    def count(plugin, count, request):
-        assert count == 42 and plugin == p
-
-    funcs = [pre_args, in_args, post_args, post_kwargs, in_multi_args]
-
-    for func, request in itertools.product(funcs, [rdict, rarr]):
-        p._exec_func(func, request)
-
-    p._exec_func(extra_def_arg, rarr)
-
-    p._exec_func(count, Request(
-        plugin=p,
-        req_id=1,
-        method='func',
-        params=[42],
-    ))
-
-    # This should fail since it is missing one positional argument
-    with pytest.raises(ValueError):
-        p._exec_func(count, Request(
-            plugin=p,
-            req_id=1,
-            method='func',
-            params=[])
-        )
diff --git a/contrib/pylightning/tests/test_plugin.py b/contrib/pylightning/tests/test_plugin.py
index 60e37de2e024..60f642874e91 100644
--- a/contrib/pylightning/tests/test_plugin.py
+++ b/contrib/pylightning/tests/test_plugin.py
@@ -1,6 +1,6 @@
 from lightning import Plugin
-
-
+from lightning.plugin import Request
+import itertools
 import pytest
 
 
@@ -15,13 +15,13 @@ def test1(name):
         """Has a single positional argument."""
         assert name == 'World'
         call_list.append(test1)
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test1',
         'params': {'name': 'World'}
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1]
 
     @p.method("test2")
@@ -29,13 +29,13 @@ def test2(name, plugin):
         """Also asks for the plugin instance. """
         assert plugin == p
         call_list.append(test2)
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test2',
         'params': {'name': 'World'}
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1, test2]
 
     @p.method("test3")
@@ -43,13 +43,13 @@ def test3(name, request):
         """Also asks for the request instance. """
         assert request is not None
         call_list.append(test3)
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test3',
         'params': {'name': 'World'}
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1, test2, test3]
 
     @p.method("test4")
@@ -57,13 +57,13 @@ def test4(name):
         """Try the positional arguments."""
         assert name == 'World'
         call_list.append(test4)
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test4',
         'params': ['World']
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1, test2, test3, test4]
 
     @p.method("test5")
@@ -73,13 +73,13 @@ def test5(name, request, plugin):
         assert request is not None
         assert p == plugin
         call_list.append(test5)
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test5',
         'params': ['World']
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1, test2, test3, test4, test5]
 
     answers = []
@@ -92,23 +92,23 @@ def test6(name, answer=42):
         call_list.append(test6)
 
     # Both calls should work (with and without the default param
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test6',
         'params': ['World']
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1, test2, test3, test4, test5, test6]
     assert answers == [42]
 
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test6',
         'params': ['World', 31337]
-    }
-    p._dispatch(request)
+    })
+    p._dispatch_request(request)
     assert call_list == [test1, test2, test3, test4, test5, test6, test6]
     assert answers == [42, 31337]
 
@@ -119,14 +119,14 @@ def test_methods_errors():
     p = Plugin(autopatch=False)
 
     # Fails because we haven't added the method yet
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test1',
         'params': {}
-    }
+    })
     with pytest.raises(ValueError):
-        p._dispatch(request)
+        p._dispatch_request(request)
     assert call_list == []
 
     @p.method("test1")
@@ -138,34 +138,102 @@ def test1(name):
         p.add_method("test1", test1)
 
     # Fails because it is missing the 'name' argument
-    request = {'id': 1, 'jsonrpc': '2.0', 'method': 'test1', 'params': {}}
+    request = p._parse_request({'id': 1, 'jsonrpc': '2.0', 'method': 'test1', 'params': {}})
     with pytest.raises(TypeError):
-        p._dispatch(request)
+        p._exec_func(test1, request)
     assert call_list == []
 
     # The same with positional arguments
-    request = {'id': 1, 'jsonrpc': '2.0', 'method': 'test1', 'params': []}
+    request = p._parse_request({'id': 1, 'jsonrpc': '2.0', 'method': 'test1', 'params': []})
     with pytest.raises(TypeError):
-        p._dispatch(request)
+        p._exec_func(test1, request)
     assert call_list == []
 
     # Fails because we have a non-matching argument
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test1',
         'params': {'name': 'World', 'extra': 1}
-    }
+    })
     with pytest.raises(TypeError):
-        p._dispatch(request)
+        p._exec_func(test1, request)
     assert call_list == []
 
-    request = {
+    request = p._parse_request({
         'id': 1,
         'jsonrpc': '2.0',
         'method': 'test1',
         'params': ['World', 1]
-    }
+    })
+
     with pytest.raises(TypeError):
-        p._dispatch(request)
+        p._exec_func(test1, request)
     assert call_list == []
+
+
+def test_positional_inject():
+    p = Plugin()
+    rdict = Request(
+        plugin=p,
+        req_id=1,
+        method='func',
+        params={'a': 1, 'b': 2, 'kwa': 3, 'kwb': 4}
+    )
+    rarr = Request(
+        plugin=p,
+        req_id=1,
+        method='func',
+        params=[1, 2, 3, 4],
+    )
+
+    def pre_args(plugin, a, b, kwa=3, kwb=4):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def in_args(a, plugin, b, kwa=3, kwb=4):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def post_args(a, b, plugin, kwa=3, kwb=4):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def post_kwargs(a, b, kwa=3, kwb=4, plugin=None):
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def in_multi_args(a, request, plugin, b, kwa=3, kwb=4):
+        assert request in [rarr, rdict]
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def in_multi_mix_args(a, plugin, b, request=None, kwa=3, kwb=4):
+        assert request in [rarr, rdict]
+        assert (plugin, a, b, kwa, kwb) == (p, 1, 2, 3, 4)
+
+    def extra_def_arg(a, b, c, d, e=42):
+        """ Also uses a different name for kwa and kwb
+        """
+        assert (a, b, c, d, e) == (1, 2, 3, 4, 42)
+
+    def count(plugin, count, request):
+        assert count == 42 and plugin == p
+
+    funcs = [pre_args, in_args, post_args, post_kwargs, in_multi_args]
+
+    for func, request in itertools.product(funcs, [rdict, rarr]):
+        p._exec_func(func, request)
+
+    p._exec_func(extra_def_arg, rarr)
+
+    p._exec_func(count, Request(
+        plugin=p,
+        req_id=1,
+        method='func',
+        params=[42],
+    ))
+
+    # This should fail since it is missing one positional argument
+    with pytest.raises(TypeError):
+        p._exec_func(count, Request(
+            plugin=p,
+            req_id=1,
+            method='func',
+            params=[])
+        )