Chore: Removing the Singleton (#1914)

Author: k9ert

src/cryptoadvance/specter/managers/node_manager.py

@@ -53,7 +53,7 @@ def update(self, data_folder=None):
         for node_alias in nodes_files:
             fullpath = os.path.join(self.data_folder, "%s.json" % node_alias)
             node_class = (
-                Node if nodes_files[node_alias]["external_node"] else InternalNode
+                Node if nodes_files[node_alias].get("external_node") else InternalNode
             )
             nodes[nodes_files[node_alias]["name"]] = node_class.from_json(
                 nodes_files[node_alias],

src/cryptoadvance/specter/managers/service_manager.py

@@ -10,7 +10,6 @@
 
 from cryptoadvance.specter.config import ProductionConfig
 from cryptoadvance.specter.device import Device
-from cryptoadvance.specter.managers.singleton import ConfigurableSingletonException
 from cryptoadvance.specter.specter_error import SpecterError
 from cryptoadvance.specter.user import User
 from cryptoadvance.specter.util.reflection import get_template_static_folder
@@ -20,10 +19,6 @@
 
 from ..services.service import Service
 from ..services import callbacks, ExtensionException
-from ..services.service_encrypted_storage import (
-    ServiceEncryptedStorageManager,
-    ServiceUnencryptedStorageManager,
-)
 from ..util.reflection import (
     _get_module_from_class,
     get_classlist_of_type_clazz_from_modulelist,
@@ -84,20 +79,6 @@ def __init__(self, specter, devstatus_threshold):
                 logger.info(
                     f"Service {clazz.__name__} not activated due to devstatus ( {self.devstatus_threshold} > {clazz.devstatus} )"
                 )
-
-        # Configure and instantiate the one and only ServiceEncryptedStorageManager
-        try:
-            ServiceEncryptedStorageManager.configure_instance(
-                specter.data_folder, specter.user_manager
-            )
-        except ConfigurableSingletonException as e:
-            # Test suite triggers multiple calls; ignore for now.
-            pass
-
-        specter.service_unencrypted_storage_manager = ServiceUnencryptedStorageManager(
-            specter.user_manager, specter.data_folder
-        )
-
         logger.info("----> finished service processing")
         self.execute_ext_callbacks("afterServiceManagerInit")
 
@@ -328,9 +309,8 @@ def user_has_encrypted_storage(self, user: User) -> bool:
         This check works even if the user doesn't have their plaintext_user_secret
         available."""
         encrypted_data = (
-            ServiceEncryptedStorageManager.get_instance().get_raw_encrypted_data(user)
+            self.specter.service_encrypted_storage_manager.get_raw_encrypted_data(user)
         )
-        print(f"encrypted_data: {encrypted_data} for {user}")
         return encrypted_data != {}
 
     def set_active_services(self, service_names_active):
@@ -363,7 +343,7 @@ def remove_all_services_from_user(self, user: User):
         if self.user_has_encrypted_storage(user=user):
             # Encrypted Service data is now orphaned since there is no
             # password. So wipe it from the disk.
-            ServiceEncryptedStorageManager.get_instance().delete_all_service_data(user)
+            app.specter.service_encrypted_storage_manager.delete_all_service_data(user)
 
     @classmethod
     def get_service_x_dirs(cls, x):

src/cryptoadvance/specter/managers/singleton.py

@@ -54,20 +54,20 @@ def callback(self, callback_id, *argv, **kwargv):
 
     @classmethod
     def set_current_user_service_data(cls, service_data: dict):
-        ServiceEncryptedStorageManager.get_instance().set_current_user_service_data(
+        app.specter.service_encrypted_storage_manager.set_current_user_service_data(
             service_id=cls.id, service_data=service_data
         )
 
     @classmethod
     def update_current_user_service_data(cls, service_data: dict):
-        ServiceEncryptedStorageManager.get_instance().update_current_user_service_data(
+        app.specter.service_encrypted_storage_manager.update_current_user_service_data(
             service_id=cls.id, service_data=service_data
         )
 
     @classmethod
     def get_current_user_service_data(cls) -> dict:
         return (
-            ServiceEncryptedStorageManager.get_instance().get_current_user_service_data(
+            app.specter.service_encrypted_storage_manager.get_current_user_service_data(
                 service_id=cls.id
             )
         )

src/cryptoadvance/specter/services/service.py

@@ -3,7 +3,7 @@
 import os
 
 from cryptoadvance.specter.managers.genericdata_manager import GenericDataManager
-from cryptoadvance.specter.managers.singleton import ConfigurableSingleton
+from cryptoadvance.specter.managers.user_manager import UserManager
 from cryptoadvance.specter.specter_error import SpecterError
 from cryptoadvance.specter.user import User
 
@@ -110,7 +110,7 @@ def __init__(self, data_folder: str, user: User, disable_decrypt: bool = False):
                 "ServiceUnencryptedStorage needs to be initialized with disable_decrypt = True"
             )
         if disable_decrypt:
-            super().__init__(data_folder, encryption_key=None, disable_decrypt=True)
+            super().__init__(data_folder, user, disable_decrypt=True)
 
     @property
     def data_file(self):
@@ -119,19 +119,15 @@ def data_file(self):
         )
 
 
-class ServiceEncryptedStorageManager(ConfigurableSingleton):
-    """Singleton that manages access to users' ServiceApiKeyStorage; context-aware so it
+class ServiceEncryptedStorageManager:
+    """manages access to users' ServiceApiKeyStorage; context-aware so it
     knows who the current_user is for the given request context.
-
-    Requires a one-time configuration call on startup in the ServiceManager.
     """
 
-    @classmethod
-    def configure_instance(cls, data_folder, user_manager):
-        super().configure_instance()
-        cls._instance.data_folder = data_folder
-        cls._instance.user_manager = user_manager
-        cls._instance.storage_by_user = {}
+    def __init__(self, data_folder, user_manager: UserManager):
+        self.user_manager = user_manager
+        self.data_folder = data_folder
+        self.storage_by_user = {}
 
     def get_raw_encrypted_data(self, user: User) -> dict:
         """Doesn't attempt to decrypt the ServiceEncryptedStorage, just returns the
@@ -184,7 +180,7 @@ def delete_all_service_data(self, user: User):
 
 
 class ServiceUnencryptedStorageManager(ServiceEncryptedStorageManager):
-    def __init__(self, user_manager, data_folder):
+    def __init__(self, data_folder, user_manager: UserManager):
         self.user_manager = user_manager
         self.data_folder = data_folder
         self.storage_by_user = {}

src/cryptoadvance/specter/services/service_encrypted_storage.py

@@ -5,12 +5,15 @@
 
 
 class BaseConfig:
+    # Unfortunately this setup is currently broken
+    # It worked for the original and now legacy api-dev.swanbitcoin.com
+    # But the new one doesn't have that configuration configured.
     SWAN_CLIENT_ID = "specter-dev"
     SWAN_CLIENT_SECRET = (
         "BcetcVcmueWf5P3UPJnHhCBMQ49p38fhzYwM7t3DJGzsXSjm89dDR5URE46SY69j"
     )
-    SWAN_API_URL = "https://dev-api.swanbitcoin.com"
-    SWAN_FRONTEND_URL = "https://dev-app.swanbitcoin.com/signup"
+    SWAN_API_URL = "https://api.dev.swanbitcoin.com"
+    SWAN_FRONTEND_URL = "https://app.dev.swanbitcoin.com/signup"
 
     # There is a whitelist at the oauth2-infra of swan which is hopefully in sync with this list:
     SWAN_ALLOWED_SPECTER_HOSTNAMES = [

src/cryptoadvance/specter/services/swan/config.py

@@ -20,6 +20,10 @@
 from urllib3.exceptions import NewConnectionError
 
 from cryptoadvance.specter.devices.device_types import DeviceTypes
+from cryptoadvance.specter.services.service_encrypted_storage import (
+    ServiceEncryptedStorageManager,
+    ServiceUnencryptedStorageManager,
+)
 
 from .helpers import clean_psbt, deep_update, is_liquid, is_testnet, get_asset_label
 from .internal_node import InternalNode
@@ -140,6 +144,16 @@ def __init__(
         if self.price_check and self.price_provider and checker_threads:
             self.price_checker.start()
 
+        # Configuring the two different storages (Universal json-files)
+        logger.info("Instantiate ServiceEncryptedStorageManager")
+        self.service_encrypted_storage_manager = ServiceEncryptedStorageManager(
+            self.data_folder, self.user_manager
+        )
+        logger.info("Instantiate ServiceUnencryptedStorageManager")
+        self.service_unencrypted_storage_manager = ServiceUnencryptedStorageManager(
+            self.data_folder, self.user_manager
+        )
+
         if threading.current_thread() is threading.main_thread():
             # Python signal handlers are always executed in the main Python thread and only the main thread is allowed to set a new signal handler.
             # This is for CTRL-C --> SIGINT

src/cryptoadvance/specter/specter.py

@@ -114,16 +114,15 @@ def test_calling_method_with_non_existing_parameters(client):
             "forwarded_request": True,
         },
     )
-    assert {  # TODO This is a temporary fix that should be handled in https://github.com/cryptoadvance/specter-desktop/pull/1693
-        "error": {
-            "code": -32000,
-            "message": "Internal error: HWIBridge.enumerate() got an unexpected keyword argument 'non_existing_parameter'.",
-        },
-        "id": 1,
-        "jsonrpc": "2.0",
-    } == json.loads(
-        req.data
+
+    resp = json.loads(req.data)
+    assert resp["error"]["code"] == -32000
+    assert resp["error"]["message"].startswith("Internal error:")
+    assert resp["error"]["message"].endswith(
+        "got an unexpected keyword argument 'non_existing_parameter'."
     )
+    assert resp["id"] == 1
+    assert resp["jsonrpc"] == "2.0"
 
 
 def test_call_not_connected_device(client):

tests/test_hwibridge.py

@@ -15,7 +15,7 @@ def test_ServiceManager2(mock_specter, mock_flaskapp, caplog):
     # We have passed the TestConfig which is (hopefully) not existing in the Swan Extension
     # So the ServiceManager will move up the dependency tree of TestConfig until it finds
     # a Config and will copy the keys into the flask-config
-    assert mock_flaskapp.config["SWAN_API_URL"] == "https://dev-api.swanbitcoin.com"
+    assert mock_flaskapp.config["SWAN_API_URL"] == "https://api.dev.swanbitcoin.com"
     assert sm.services["bitcoinreserve"] != None
     assert sm.services["swan"] != None