From 552bfb589ac218b909c81efe9fdc5388d5926321 Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 8 Sep 2023 14:48:36 +0000 Subject: [PATCH 1/3] plugins: split load_plugins() --- electrum/plugin.py | 45 +++++++++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/electrum/plugin.py b/electrum/plugin.py index a2f7bff8f..5397ecdd2 100644 --- a/electrum/plugin.py +++ b/electrum/plugin.py @@ -29,7 +29,7 @@ import time import threading import sys from typing import (NamedTuple, Any, Union, TYPE_CHECKING, Optional, Tuple, - Dict, Iterable, List, Sequence, Callable, TypeVar, Mapping) + Dict, Iterable, List, Sequence, Callable, TypeVar, Mapping, Set) import concurrent from concurrent import futures from functools import wraps, partial @@ -56,12 +56,13 @@ hooks = {} class Plugins(DaemonThread): LOGGING_SHORTCUT = 'p' + pkgpath = os.path.dirname(plugins.__file__) + _all_found_plugins = None # type: Optional[Dict[str, dict]] @profiler def __init__(self, config: SimpleConfig, gui_name): DaemonThread.__init__(self) self.name = 'Plugins' # set name of thread - self.pkgpath = os.path.dirname(plugins.__file__) self.config = config self.hw_wallets = {} self.plugins = {} # type: Dict[str, BasePlugin] @@ -72,21 +73,33 @@ class Plugins(DaemonThread): self.add_jobs(self.device_manager.thread_jobs()) self.start() + @classmethod + def find_all_plugins(cls) -> Mapping[str, dict]: + """Return a map of all found plugins: name -> description. + Note that plugins not available for the current GUI are also included. + """ + if cls._all_found_plugins is None: + cls._all_found_plugins = dict() + for loader, name, ispkg in pkgutil.iter_modules([cls.pkgpath]): + full_name = f'electrum.plugins.{name}' + spec = importlib.util.find_spec(full_name) + if spec is None: # pkgutil found it but importlib can't ?! + raise Exception(f"Error pre-loading {full_name}: no spec") + try: + module = importlib.util.module_from_spec(spec) + # sys.modules needs to be modified for relative imports to work + # see https://stackoverflow.com/a/50395128 + sys.modules[spec.name] = module + spec.loader.exec_module(module) + except Exception as e: + raise Exception(f"Error pre-loading {full_name}: {repr(e)}") from e + d = module.__dict__ + assert name not in cls._all_found_plugins + cls._all_found_plugins[name] = d + return cls._all_found_plugins + def load_plugins(self): - for loader, name, ispkg in pkgutil.iter_modules([self.pkgpath]): - full_name = f'electrum.plugins.{name}' - spec = importlib.util.find_spec(full_name) - if spec is None: # pkgutil found it but importlib can't ?! - raise Exception(f"Error pre-loading {full_name}: no spec") - try: - module = importlib.util.module_from_spec(spec) - # sys.modules needs to be modified for relative imports to work - # see https://stackoverflow.com/a/50395128 - sys.modules[spec.name] = module - spec.loader.exec_module(module) - except Exception as e: - raise Exception(f"Error pre-loading {full_name}: {repr(e)}") from e - d = module.__dict__ + for name, d in self.find_all_plugins().items(): gui_good = self.gui_name in d.get('available_for', []) if not gui_good: continue From 8c9fec4ab8e82da6b96d0feabac12794b7852daf Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 8 Sep 2023 14:55:22 +0000 Subject: [PATCH 2/3] commands: getconfig to use default values, add existence checks - getconfig and setconfig now both check configvars for existence - getconfig returns default values when applicable - setconfig does not side-step type-checks for values fixes https://github.com/spesmilo/electrum/issues/8607 closes https://github.com/spesmilo/electrum/pull/8608 --- electrum/commands.py | 14 +++++++++++--- electrum/plugin.py | 9 +++++++++ electrum/simple_config.py | 17 +++++++++++++++++ electrum/tests/test_simple_config.py | 6 ++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/electrum/commands.py b/electrum/commands.py index 6be08777f..326f86a1e 100644 --- a/electrum/commands.py +++ b/electrum/commands.py @@ -59,7 +59,7 @@ from .lnutil import SENT, RECEIVED from .lnutil import LnFeatures from .lnutil import extract_nodeid from .lnpeer import channel_id_from_funding_tx -from .plugin import run_hook, DeviceMgr +from .plugin import run_hook, DeviceMgr, Plugins from .version import ELECTRUM_VERSION from .simple_config import SimpleConfig from .invoices import Invoice @@ -305,7 +305,11 @@ class Commands: @command('') async def getconfig(self, key): """Return a configuration variable. """ - return self.config.get(key) + if Plugins.is_plugin_enabler_config_key(key): + return self.config.get(key) + else: + cv = self.config.cv.from_key(key) + return cv.get() @classmethod def _setconfig_normalize_value(cls, key, value): @@ -326,7 +330,11 @@ class Commands: self.daemon.commands_server.rpc_user = value if self.daemon and key == SimpleConfig.RPC_PASSWORD.key(): self.daemon.commands_server.rpc_password = value - self.config.set_key(key, value) + if Plugins.is_plugin_enabler_config_key(key): + self.config.set_key(key, value) + else: + cv = self.config.cv.from_key(key) + cv.set(value) return True @command('') diff --git a/electrum/plugin.py b/electrum/plugin.py index 5397ecdd2..631f6a388 100644 --- a/electrum/plugin.py +++ b/electrum/plugin.py @@ -160,6 +160,15 @@ class Plugins(DaemonThread): p.close() self.logger.info(f"closed {name}") + @classmethod + def is_plugin_enabler_config_key(cls, key: str) -> bool: + if not key.startswith('use_'): + return False + # note: the 'use_' prefix is not sufficient to check, there are + # non-plugin-related config keys that also have it... hence: + name = key[4:] + return name in cls.find_all_plugins() + def toggle(self, name: str) -> Optional['BasePlugin']: p = self.get(name) return self.disable(name) if p else self.enable(name) diff --git a/electrum/simple_config.py b/electrum/simple_config.py index 38d70c2a7..50d797186 100644 --- a/electrum/simple_config.py +++ b/electrum/simple_config.py @@ -52,6 +52,9 @@ _logger = get_logger(__name__) FINAL_CONFIG_VERSION = 3 +_config_var_from_key = {} # type: Dict[str, 'ConfigVar'] + + class ConfigVar(property): def __init__( @@ -65,6 +68,7 @@ class ConfigVar(property): self._default = default self._type = type_ property.__init__(self, self._get_config_value, self._set_config_value) + _config_var_from_key[key] = self def _get_config_value(self, config: 'SimpleConfig'): with config.lock: @@ -132,6 +136,11 @@ class ConfigVarWithConfig: def __repr__(self): return f"" + def __eq__(self, other) -> bool: + if not isinstance(other, ConfigVarWithConfig): + return False + return self._config is other._config and self._config_var is other._config_var + class SimpleConfig(Logger): """ @@ -818,10 +827,18 @@ class SimpleConfig(Logger): """ class CVLookupHelper: def __getattribute__(self, name: str) -> ConfigVarWithConfig: + if name in ("from_key", ): # don't apply magic, just use standard lookup + return super().__getattribute__(name) config_var = config.__class__.__getattribute__(type(config), name) if not isinstance(config_var, ConfigVar): raise AttributeError() return ConfigVarWithConfig(config=config, config_var=config_var) + def from_key(self, key: str) -> ConfigVarWithConfig: + try: + config_var = _config_var_from_key[key] + except KeyError: + raise KeyError(f"No ConfigVar with key={key!r}") from None + return ConfigVarWithConfig(config=config, config_var=config_var) def __setattr__(self, name, value): raise Exception( f"Cannot assign value to config.cv.{name} directly. " diff --git a/electrum/tests/test_simple_config.py b/electrum/tests/test_simple_config.py index 0136a2dcf..62c6cdf72 100644 --- a/electrum/tests/test_simple_config.py +++ b/electrum/tests/test_simple_config.py @@ -199,6 +199,12 @@ class Test_SimpleConfig(ElectrumTestCase): config.NETWORK_MAX_INCOMING_MSG_SIZE = 2_222_222 self.assertEqual(5_555_555, config.NETWORK_MAX_INCOMING_MSG_SIZE) + def test_configvars_from_key(self): + config = SimpleConfig(self.options) + self.assertEqual(config.cv.NETWORK_SERVER, config.cv.from_key("server")) + with self.assertRaises(KeyError): + config.cv.from_key("server333") + def test_depth_target_to_fee(self): config = SimpleConfig(self.options) config.mempool_fees = [[49, 100110], [10, 121301], [6, 153731], [5, 125872], [1, 36488810]] From 87f5df1e8b07adf8e4765c9ed511d803c118615c Mon Sep 17 00:00:00 2001 From: SomberNight Date: Fri, 8 Sep 2023 15:09:17 +0000 Subject: [PATCH 3/3] config: add sanity check for duplicate config keys --- electrum/simple_config.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/electrum/simple_config.py b/electrum/simple_config.py index 50d797186..961748641 100644 --- a/electrum/simple_config.py +++ b/electrum/simple_config.py @@ -68,6 +68,7 @@ class ConfigVar(property): self._default = default self._type = type_ property.__init__(self, self._get_config_value, self._set_config_value) + assert key not in _config_var_from_key, f"duplicate config key str: {key!r}" _config_var_from_key[key] = self def _get_config_value(self, config: 'SimpleConfig'): @@ -105,8 +106,8 @@ class ConfigVar(property): return f"" def __deepcopy__(self, memo): - cv = ConfigVar(self._key, default=self._default, type_=self._type) - return cv + # We can be considered ~stateless. State is stored in the config, which is external. + return self class ConfigVarWithConfig: