pyln: Add a dynamic configs and a callback for changes

contrib/pyln-client/pyln/client/plugin.py

@@ -1,10 +1,3 @@
-from .lightning import LightningRpc, Millisatoshi
-from binascii import hexlify
-from collections import OrderedDict
-from enum import Enum
-from threading import RLock
-from typing import Any, Callable, Dict, List, Optional, Tuple, Union
-
 import inspect
 import io
 import json
@@ -14,6 +7,14 @@
 import re
 import sys
 import traceback
+from binascii import hexlify
+from collections import OrderedDict
+from dataclasses import dataclass
+from enum import Enum
+from threading import RLock
+from typing import Any, Callable, Dict, List, Optional, Tuple, Union
+
+from .lightning import LightningRpc, Millisatoshi
 
 # Notice that this definition is incomplete as it only checks the
 # top-level. Arrays and Dicts could contain types that aren't encodeable. This
@@ -183,6 +184,38 @@ def progress(self,
         self._notify("progress", d)
 
 
+@dataclass
+class Option:
+    name: str
+    default: Optional[Any]
+    description: Optional[str]
+    opt_type: str
+    value: Optional[Any]
+    multi: bool
+    deprecated: Optional[Union[bool, List[str]]]
+    dynamic: bool
+    on_change: Optional[Callable[["Plugin", str, Optional[Any]], None]]
+
+    def __getitem__(self, key):
+        """Backwards compatibility for callers who directly asked for ['value']"""
+        if key == 'value':
+            return self.value
+        raise KeyError(f"Key {key} not supported, only 'value' is")
+
+    def json(self) -> Dict[str, Any]:
+        ret = {
+            'name': self.name,
+            'description': self.description,
+            'type': self.opt_type,
+            'multi': self.multi,
+            'deprecated': self.deprecated,
+            'dynamic': self.dynamic,
+        }
+        if self.default is not None:
+            ret['default'] = self.default
+        return ret
+
+
 # If a hook call fails we need to coerce it into something the main daemon can
 # handle. Returning an error is not an option since we explicitly do not allow
 # those as a response to the calls, otherwise the only option we have is to
@@ -224,7 +257,7 @@ def __init__(self, stdout: Optional[io.TextIOBase] = None,
             'setconfig': Method('setconfig', self._set_config, MethodType.RPCMETHOD)
         }
 
-        self.options: Dict[str, Dict[str, Any]] = {}
+        self.options: Dict[str, Option] = {}
         self.notification_topics: List[str] = []
         self.custom_msgs = custom_msgs
 
@@ -297,7 +330,7 @@ def add_method(self, name: str, func: Callable[..., Any],
                    category: Optional[str] = None,
                    desc: Optional[str] = None,
                    long_desc: Optional[str] = None,
-                   deprecated: Union[bool, List[str]] = None) -> None:
+                   deprecated: Optional[Union[bool, List[str]]] = None) -> None:
         """Add a plugin method to the dispatch table.
 
         The function will be expected at call time (see `_dispatch`)
@@ -388,11 +421,14 @@ def decorator(f: Callable[..., None]) -> Callable[..., None]:
             return f
         return decorator
 
-    def add_option(self, name: str, default: Optional[str],
+    def add_option(self, name: str, default: Optional[Any],
                    description: Optional[str],
-                   opt_type: str = "string", deprecated: Union[bool, List[str]] = None,
+                   opt_type: str = "string",
+                   deprecated: Optional[Union[bool, List[str]]] = None,
                    multi: bool = False,
-                   dynamic=False) -> None:
+                   dynamic=False,
+                   on_change: Optional[Callable[["Plugin", str, Optional[Any]], None]] = None,
+                   ) -> None:
         """Add an option that we'd like to register with lightningd.
 
         Needs to be called before `Plugin.run`, otherwise we might not
@@ -406,22 +442,28 @@ def add_option(self, name: str, default: Optional[str],
 
         if opt_type not in ["string", "int", "bool", "flag"]:
             raise ValueError(
-                '{} not in supported type set (string, int, bool, flag)'
+                '{} not in supported type set (string, int, bool, flag)'.format(opt_type)
             )
 
-        self.options[name] = {
-            'name': name,
-            'default': default,
-            'description': description,
-            'type': opt_type,
-            'value': None,
-            'multi': multi,
-            'deprecated': deprecated,
-            "dynamic": dynamic
-        }
+        if on_change is not None and not dynamic:
+            raise ValueError(
+                'Option {} has on_change callback but is not dynamic'.format(name)
+            )
+
+        self.options[name] = Option(
+            name=name,
+            default=default,
+            description=description,
+            opt_type=opt_type,
+            value=None,
+            dynamic=dynamic,
+            on_change=on_change,
+            multi=multi,
+            deprecated=deprecated if deprecated is not None else False,
+        )
 
     def add_flag_option(self, name: str, description: str,
-                        deprecated: Union[bool, List[str]] = None,
+                        deprecated: Optional[Union[bool, List[str]]] = None,
                         dynamic: bool = False) -> None:
         """Add a flag option that we'd like to register with lightningd.
 
@@ -437,19 +479,19 @@ def add_notification_topic(self, topic: str):
         """
         self.notification_topics.append(topic)
 
-    def get_option(self, name: str) -> str:
+    def get_option(self, name: str) -> Optional[Any]:
         if name not in self.options:
             raise ValueError("No option with name {} registered".format(name))
 
-        if self.options[name]['value'] is not None:
-            return self.options[name]['value']
+        if self.options[name].value is not None:
+            return self.options[name].value
         else:
-            return self.options[name]['default']
+            return self.options[name].default
 
     def async_method(self, method_name: str, category: Optional[str] = None,
                      desc: Optional[str] = None,
                      long_desc: Optional[str] = None,
-                     deprecated: Union[bool, List[str]] = None) -> NoneDecoratorType:
+                     deprecated: Optional[Union[bool, List[str]]] = None) -> NoneDecoratorType:
         """Decorator to add an async plugin method to the dispatch table.
 
         Internally uses add_method.
@@ -826,16 +868,19 @@ def print_usage(self):
                 parts.append(options_header)
                 options_header = None
 
-            doc = textwrap.indent(opt['description'], prefix="    ")
+            if opt.description:
+                doc = textwrap.indent(opt.description, prefix="    ")
+            else:
+                doc = ""
 
-            if opt['multi']:
+            if opt.multi:
                 doc += "\n\n    This option can be specified multiple times"
 
             parts.append(option_tpl.format(
-                name=opt['name'],
+                name=opt.name,
                 doc=doc,
-                default=opt['default'],
-                typ=opt['type'],
+                default=opt.default,
+                typ=opt.opt_type,
             ))
 
         sys.stdout.write("".join(parts))
@@ -921,7 +966,7 @@ def _getmanifest(self, **kwargs) -> JSONType:
                 m["long_description"] = method.long_desc
 
         manifest = {
-            'options': list({k: v for k, v in d.items() if v is not None} for d in self.options.values()),
+            'options': list(d.json() for d in self.options.values()),
             'rpcmethods': methods,
             'subscriptions': list(self.subscriptions.keys()),
             'hooks': hooks,
@@ -967,18 +1012,23 @@ def verify_bool(d: Dict[str, JSONType], key: str) -> bool:
         self.rpc = LightningRpc(path)
         self.startup = verify_bool(configuration, 'startup')
         for name, value in options.items():
-            self.options[name]['value'] = value
+            self.options[name].value = value
 
         # Dispatch the plugin's init handler if any
         if self.child_init:
             return self._exec_func(self.child_init, request)
         return None
 
-    def _set_config(self, **_) -> None:
+    def _set_config(self, config: str, val: Optional[Any]) -> None:
         """Called when the value of a dynamic option is changed
-        For now we don't do anything.
         """
-        pass
+        opt = self.options[config]
+        cb = opt.on_change
+        if cb is not None:
+            # This may throw an exception: caller will turn into error msg for user.
+            cb(self, config, val)
+
+        opt.value = val
 
 
 class PluginStream(object):

tests/plugins/dynamic_option.py

@@ -1,12 +1,30 @@
 #!/usr/bin/env python3
-from pyln.client import Plugin
+from pyln.client import Plugin, RpcException
+from typing import Any, Optional
 
 plugin = Plugin()
 
+
+@plugin.method('dynamic-option-report')
+def record_lookup(plugin):
+    return {'test-dynamic-config': plugin.get_option('test-dynamic-config')}
+
+
+def on_config_change(plugin, config: str, value: Optional[Any]) -> None:
+    """Callback method called when a config value is changed.
+    """
+    plugin.log(f"Setting config {config} to {value}")
+    if value == 'bad value':
+        raise RpcException("I don't like bad values!")
+
+
 plugin.add_option(
     name="test-dynamic-config",
     description="A config option which can be changed at run-time",
     default="initial",
-    dynamic=True)
+    dynamic=True,
+    on_change=on_config_change,
+)
+
 
 plugin.run()

tests/plugins/zeroconf-selective.py

@@ -21,8 +21,8 @@ def on_openchannel(openchannel, plugin, **kwargs):
 
 plugin.add_option(
     'zeroconf-allow',
+    '03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f',
     'A node_id to allow zeroconf channels from',
-    '03864ef025fde8fb587d989186ce6a4a186895ee44a926bfc370e2c366597a3f8f'
 )
 
 plugin.add_option(

tests/test_plugin.py

@@ -4314,8 +4314,20 @@ def test_dynamic_option_python_plugin(node_factory):
 
     assert result["configs"]["test-dynamic-config"]["value_str"] == "initial"
 
+    assert ln.rpc.dynamic_option_report() == {'test-dynamic-config': 'initial'}
     result = ln.rpc.setconfig("test-dynamic-config", "changed")
     assert result["config"]["value_str"] == "changed"
+    assert ln.rpc.dynamic_option_report() == {'test-dynamic-config': 'changed'}
+
+    ln.daemon.wait_for_log(
+        'dynamic_option.py:.*Setting config test-dynamic-config to changed'
+    )
+
+    with pytest.raises(RpcError, match="I don't like bad values!"):
+        ln.rpc.setconfig("test-dynamic-config", "bad value")
+
+    # Does not alter value!
+    assert ln.rpc.dynamic_option_report() == {'test-dynamic-config': 'changed'}
 
 
 def test_renepay_not_important(node_factory):