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/lightning.py b/contrib/pylightning/lightning/lightning.py index fd0c22ce3f08..31ed1f05e0a9 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() @@ -291,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 @@ -300,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, diff --git a/contrib/pylightning/lightning/plugin.py b/contrib/pylightning/lightning/plugin.py index e7929bdcaebb..dabe4c933029 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,79 @@ 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. + + 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 + 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): """Controls interactions with lightningd, and bundles functionality. @@ -25,7 +99,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 @@ -39,13 +113,15 @@ 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 self.child_init = None - def add_method(self, name, func): + self.write_lock = RLock() + + 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`) @@ -65,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( @@ -72,7 +155,9 @@ def add_method(self, name, func): ) # Register the function with the name - self.methods[name] = (func, MethodType.RPCMETHOD) + method = Method(name, func, MethodType.RPCMETHOD) + method.background = background + self.methods[name] = method def add_subscription(self, topic, func): """Add a subscription to our list of subscriptions. @@ -129,24 +214,36 @@ def get_option(self, name): else: return self.options[name]['default'] - def method(self, method_name, *args, **kwargs): + 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) + self.add_method(method_name, f, background=False) 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: raise ValueError( "Method {} was already registered".format(name, self.methods[name]) ) - self.methods[name] = (func, MethodType.HOOK) + method = Method(name, func, MethodType.HOOK) + method.background = background + self.methods[name] = method def hook(self, method_name): """Decorator to add a plugin hook to the dispatch table. @@ -154,7 +251,17 @@ def hook(self, method_name): Internally uses add_hook. """ def decorator(f): - self.add_hook(method_name, f) + 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 @@ -169,12 +276,12 @@ def decorator(f): return decorator def _exec_func(self, func, request): - params = request['params'] + params = request.params sig = inspect.signature(func) 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: @@ -183,14 +290,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 is not inspect.Signature.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] @@ -201,42 +316,57 @@ 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: + 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) def _dispatch_request(self, request): - name = request['method'] + name = request.method if name not in self.methods: raise ValueError("No method {} found.".format(name)) - func, _ = self.methods[name] + method = self.methods[name] + request.background = method.background try: - result = { - 'jsonrpc': '2.0', - 'id': request['id'], - 'result': self._exec_func(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) @@ -249,9 +379,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 @@ -259,18 +390,28 @@ 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 = 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 # return anything. - if 'id' in request: + if request.id is not None: self._dispatch_request(request) else: self._dispatch_notification(request) @@ -291,27 +432,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 +467,7 @@ def _getmanifest(self, **kwargs): args.append("[%s]" % arg) methods.append({ - 'name': name, + 'name': method.name, 'usage': " ".join(args), 'description': doc }) diff --git a/contrib/pylightning/lightning/test_plugin.py b/contrib/pylightning/lightning/test_plugin.py deleted file mode 100644 index 07d2c8ca2f6a..000000000000 --- a/contrib/pylightning/lightning/test_plugin.py +++ /dev/null @@ -1,51 +0,0 @@ -from .plugin import Plugin -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] - } - - 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) - - 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) 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=[]) + ) diff --git a/tests/plugins/asynctest.py b/tests/plugins/asynctest.py new file mode 100755 index 000000000000..97dd4df61db9 --- /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.async_method('asyncqueue') +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)