pylightning: Warn users of plugins that may break due to extra args

We recently noticed that the way we unpack the call arguments for hooks and
notifications in pylightning breaks pretty quickly once you start changing the
hook and notification params. If you add params they will not get mapped
correctly causing the plugin to error out.

This can be fixed by adding a `VAR_KEYWORD` argument to the calbacks, i.e., by
adding a single `**kwargs` argument at the end of the signature. This commit
adds a check that such a catch-all argument exists, and emits a warning if it
doesn't.

It also fixes up the plugins that we ship ourselves.

Signed-off-by: Christian Decker <decker.christian@gmail.com>

contrib/plugins/helloworld.py

@@ -20,22 +20,22 @@ def hello(plugin, name="world"):
 
 
 @plugin.init()
-def init(options, configuration, plugin):
+def init(options, configuration, plugin, **kwargs):
     plugin.log("Plugin helloworld.py initialized")
 
 
 @plugin.subscribe("connect")
-def on_connect(plugin, id, address):
+def on_connect(plugin, id, address, **kwargs):
     plugin.log("Received connect event for peer {}".format(id))
 
 
 @plugin.subscribe("disconnect")
-def on_disconnect(plugin, id):
+def on_disconnect(plugin, id, **kwargs):
     plugin.log("Received disconnect event for peer {}".format(id))
 
 
 @plugin.subscribe("invoice_payment")
-def on_payment(plugin, invoice_payment):
+def on_payment(plugin, invoice_payment, **kwargs):
     plugin.log("Received invoice_payment event for label {}, preimage {},"
                " and amount of {}".format(invoice_payment.get("label"),
                                           invoice_payment.get("preimage"),
@@ -43,7 +43,7 @@ def on_payment(plugin, invoice_payment):
 
 
 @plugin.hook("htlc_accepted")
-def on_htlc_accepted(onion, htlc, plugin):
+def on_htlc_accepted(onion, htlc, plugin, **kwargs):
     plugin.log('on_htlc_accepted called')
     time.sleep(20)
     return {'result': 'continue'}

contrib/pylightning/lightning/plugin.py

@@ -183,6 +183,19 @@ class Plugin(object):
             raise ValueError(
                 "Topic {} already has a handler".format(topic)
             )
+
+        # Make sure the notification callback has a **kwargs argument so that it
+        # doesn't break if we add more arguments to the call later on. Issue a
+        # warning if it does not.
+        s = inspect.signature(func)
+        kinds = [p.kind for p in s.parameters.values()]
+        if inspect.Parameter.VAR_KEYWORD not in kinds:
+            self.log(
+                "Notification handler {} for notification {} does not have a "
+                "variable keyword argument. It is strongly suggested to add "
+                "`**kwargs` as last parameter to hook and notification "
+                "handlers.".format(func.__name__, topic), level="warn")
+
         self.subscriptions[topic] = func
 
     def subscribe(self, topic):
@@ -251,6 +264,19 @@ class Plugin(object):
             raise ValueError(
                 "Method {} was already registered".format(name, self.methods[name])
             )
+
+        # Make sure the hook callback has a **kwargs argument so that it
+        # doesn't break if we add more arguments to the call later on. Issue a
+        # warning if it does not.
+        s = inspect.signature(func)
+        kinds = [p.kind for p in s.parameters.values()]
+        if inspect.Parameter.VAR_KEYWORD not in kinds:
+            self.log(
+                "Hook handler {} for hook {} does not have a variable keyword "
+                "argument. It is strongly suggested to add `**kwargs` as last "
+                "parameter to hook and notification handlers.".format(
+                    func.__name__, name), level="warn")
+
         method = Method(name, func, MethodType.HOOK)
         method.background = background
         self.methods[name] = method

tests/plugins/dblog.py

@@ -24,7 +24,7 @@ def init(configuration, options, plugin):
 
 
 @plugin.hook('db_write')
-def db_write(plugin, writes):
+def db_write(plugin, writes, **kwargs):
     if not plugin.initted:
         plugin.log("deferring {} commands".format(len(writes)))
         plugin.sqlite_pre_init_cmds += writes

tests/plugins/fail_htlcs.py

@@ -6,7 +6,7 @@ plugin = Plugin()
 
 
 @plugin.hook("htlc_accepted")
-def on_htlc_accepted(htlc, onion, plugin):
+def on_htlc_accepted(onion, plugin, **kwargs):
     plugin.log("Failing htlc on purpose")
     plugin.log("onion: %r" % (onion))
     return {"result": "fail", "failure_code": 16399}

tests/plugins/hold_htlcs.py

@@ -17,7 +17,7 @@ plugin = Plugin()
 
 
 @plugin.hook("htlc_accepted")
-def on_htlc_accepted(htlc, onion, plugin):
+def on_htlc_accepted(htlc, onion, plugin, **kwargs):
     # Stash the onion so the test can check it
     fname = os.path.join(tempfile.mkdtemp(), "onion.json")
     with open(fname, 'w') as f:

tests/plugins/hold_invoice.py

@@ -9,7 +9,7 @@ plugin = Plugin()
 
 
 @plugin.hook('invoice_payment')
-def on_payment(payment, plugin):
+def on_payment(payment, plugin, **kwargs):
     time.sleep(float(plugin.get_option('holdtime')))
     return {}
 

tests/plugins/pretend_badlog.py

@@ -12,7 +12,7 @@ def init(configuration, options, plugin):
 
 
 @plugin.subscribe("warning")
-def notify_warning(plugin, warning):
+def notify_warning(plugin, warning, **kwargs):
     plugin.log("Received warning")
     plugin.log("level: {}".format(warning['level']))
     plugin.log("time: {}".format(warning['time']))

tests/plugins/reject.py

@@ -13,7 +13,7 @@ plugin = Plugin()
 
 
 @plugin.hook('peer_connected')
-def on_connected(peer, plugin):
+def on_connected(peer, plugin, **kwargs):
     if peer['id'] in plugin.reject_ids:
         print("{} is in reject list, disconnecting".format(peer['id']))
         return {'result': 'disconnect', 'error_message': 'You are in reject list'}

tests/plugins/reject_odd_funding_amounts.py

@@ -10,7 +10,7 @@ plugin = Plugin()
 
 
 @plugin.hook('openchannel')
-def on_openchannel(openchannel, plugin):
+def on_openchannel(openchannel, plugin, **kwargs):
     print("{} VARS".format(len(openchannel.keys())))
     for k in sorted(openchannel.keys()):
         print("{}={}".format(k, openchannel[k]))

tests/plugins/reject_some_invoices.py

@@ -10,7 +10,7 @@ plugin = Plugin()
 
 
 @plugin.hook('invoice_payment')
-def on_payment(payment, plugin):
+def on_payment(payment, plugin, **kwargs):
     print("label={}".format(payment['label']))
     print("msat={}".format(payment['msat']))
     print("preimage={}".format(payment['preimage']))

tests/plugins/shortcircuit.py

@@ -6,7 +6,7 @@ plugin = Plugin()
 
 
 @plugin.hook("htlc_accepted")
-def on_htlc_accepted(onion, htlc, plugin):
+def on_htlc_accepted(onion, htlc, plugin, **kwargs):
     return {"result": "resolve", "payment_key": "00" * 32}