Feature: several things for Spectrum preparation (#1913)

* change Node persistence format

* Removing the Singleton

* mainly refactoring

* intermediate commit

* fix test

* Updated diagram

* fix tests

* removing simplejsons references

* fix tests

* remove update, tests working

* abstract Node

* refining Migration framework: run migrations until they suceed

* BrokenCoreConnectionException added to handle lost RPC + some minor error handling fixes

* clicking redirects to node config if there is no node connection

* AbstractNode

* BrokenCoreConnectionException added to handle lost RPC + some minor error handling fixes

* clicking redirects to node config if there is no node connection

* handling broken connection when there is no other error superseding (i.e. no wallets)

* Abstract Node improvements and issue fixing

* improving node_info handling

* fix test results, property device_manager in user, bcce in get_rpc in node, improved rpc property in node

* typos

* provide data_folders for extensions

* parametrizing include for node_info

* Remove initial_node_contribution refactoring welcome

* adding convenience methods

* extension data-storage

* Tests green

* Better failure resistence

* Enabling Node settings

* error handling

* adjust_view_model callback

* exception Handling

* Shielding core from extension flaws

* More consistent logging

* Refactoring wallets endpoints

* Make wallet_overview extendable

* butgifex and cleanup

* feedback by Manolis

* rename BusinessObject to PersistentObject

* fix Error-management and rollback information

* Nodes.md corrections

* frontend-aspects.md (small) corrections

* fix tests and address feedback and errormanagement

* fix tests, proper error-handling

* Fix test_util_reflection

* just some little addons to test_util_reflection

* fix cleanup_on_exit

* migration fix

* remove check, add redirect after saving node

* fix test (not that cool but what to do?)

* upgrade Flask as Flask-SQLAlchemy needs higher Flask version

* Revert "upgrade Flask as Flask-SQLAlchemy needs higher Flask version"

This reverts commit bf24120.

* update bug fixed + node manager added to node test

* cleanup external_node

Co-authored-by: moneymanolis <[email protected]>

Author: k9ert

docs/extensions/data-storage.md

@@ -15,24 +15,22 @@ It is up to each `Service` implementation to decide what data is stored; the `Se
 This is also where `Service`-wide configuration or other information should be stored, _**even if it is not secret**_ (see above intro about not polluting other existing data stores).
 
 ## ServiceEncryptedStorageManager
-Because the `ServiceEncryptedStorage` is specific to each individual user, this manager provides convenient access to automatically retrieve the `current_user` from the Flask context and provide the correct user's `ServiceEncryptedStorage`. It is implemented as a `Singleton` which can be retrieved simply by importing the class and calling `get_instance()`.
+Because the `ServiceEncryptedStorage` is specific to each individual user, this manager provides convenient access to automatically retrieve the `current_user` from the Flask context and provide the correct user's `ServiceEncryptedStorage`.
 
 This simplifies code to just asking for:
 ```python
 from .service_encrypted_storage import ServiceEncryptedStorageManager
 
-ServiceEncryptedStorageManager.get_instance().get_current_user_service_data(service_id=some_service_id)
+app.specter.service_encrypted_storage_manager.get_current_user_service_data(service_id=some_service_id)
 ```
 
 As a further convenience, the `Service` base class itself encapsulates `Service`-aware access to this per-`User` encrypted storage:
 ```python
 @classmethod
 def get_current_user_service_data(cls) -> dict:
-    return ServiceEncryptedStorageManager.get_instance().get_current_user_service_data(service_id=cls.id)
+    return app.specter.service_encrypted_storage_manager.get_current_user_service_data(service_id=cls.id)
 ```
 
-Whenever possible, external code should not directly access these `Service`-related support classes but rather should ask for them through the `Service` class.
-
 ## `ServiceUnencryptedStorage`
 A disadvantage of the `ServiceEncryptedStorage` is, that the user needs to be freshly logged in in order to be able to decrypt the secrets. If you want to avoid that login but your extension should still store data on disk, you can use the `ServiceUnencryptedStorage`.
 
@@ -49,7 +47,7 @@ _Note: current `Service` implementations have not yet needed this feature so dis
 
 Unfortunately, the two unencrypted classes are derived from the encrypted one rather than having it the other way around or having abstract classes. This makes the diagram maybe a bit confusing.
 
-[![](https://mermaid.ink/img/pako:eNqVVMFuwjAM_ZUqJzaVw66IIU0D7bRd0G6VItO4LFvqoCRlqhj_PpeWASLduhyiqH7v-dlOsxO5VSgmIjfg_VzD2kGZUcKr3Z-Q0Ol8DgGegWCNLpl-jcfJEt1W57ig3NWbgGoZrONoS-oJXjBfCaPcPxI-ENkAQVvyF7RHS4VeVw5WBpea1gaDpZbZZ6eT_9VyzMK18_8oZeIuE8l4fMsn4tOQRvZm7FPra24XZgLjK4--38BFTe1-uCORAe3acLOk1KSDlCOPpkgTxSBZWKPQpUlniUcnP7C-f7GENycmQYnSFvLdc7zQBk-hn1r4OxrlTxFjQY3ORKSHbUfcn3vuqTFm_MIyt4h3pX1zraTCA_0sX0b9ya5varxPB7DUKk0-wfC1HSh_PeJdFB7_Mc6sTKf--Hk2G96769lHhJrlW7xc1bJp538qGpQjJnVqhUhFia4ErfiROwhlIrxhiZmY8FFhAZUJmciogVYbHj8ulOb8YlKA8ZgKqIJd1pSLSXAVHkHdW9mh9t9YNMxZ)](https://mermaid-js.github.io/mermaid-live-editor/edit#pako:eNqVVMFuwjAM_ZUqJzaVw66IIU0D7bRd0G6VItO4LFvqoCRlqhj_PpeWASLduhyiqH7v-dlOsxO5VSgmIjfg_VzD2kGZUcKr3Z-Q0Ol8DgGegWCNLpl-jcfJEt1W57ig3NWbgGoZrONoS-oJXjBfCaPcPxI-ENkAQVvyF7RHS4VeVw5WBpea1gaDpZbZZ6eT_9VyzMK18_8oZeIuE8l4fMsn4tOQRvZm7FPra24XZgLjK4--38BFTe1-uCORAe3acLOk1KSDlCOPpkgTxSBZWKPQpUlniUcnP7C-f7GENycmQYnSFvLdc7zQBk-hn1r4OxrlTxFjQY3ORKSHbUfcn3vuqTFm_MIyt4h3pX1zraTCA_0sX0b9ya5varxPB7DUKk0-wfC1HSh_PeJdFB7_Mc6sTKf--Hk2G96769lHhJrlW7xc1bJp538qGpQjJnVqhUhFia4ErfiROwhlIrxhiZmY8FFhAZUJmciogVYbHj8ulOb8YlKA8ZgKqIJd1pSLSXAVHkHdW9mh9t9YNMxZ)
+[![](https://mermaid.ink/img/pako:eNqVVD1PwzAQ_SuRp4KSgTWCAakVEywVC4pkXeNLMTjnynaKotL_jpukTaLGEDxYdt579-4j8oHlWiBLWa7A2qWErYEyo8ivdn9CQiPzJTh4BoItmuj-O0miNZq9zHFFual3DsXaaePRVhQAR8pXwkntH4aPRNqBk5rsHMupENfOHWtWpIzdZSxKklt_In-a04igYyhaqDkd7AWeX1m04QRGNbV7M-OJBh9a-LQ4lyQd5wuLqogj4Um80EqgiaMuJd96_on1w4smvOmVBCVyXfAP6_FCKuyhSy3-Oyphe0RpEItBEG5h3wmPw5wDNU4lPkrZt8jvQlrYKOQCG_nAL6Ow2fWfNt2nhsyliKMvUArnhr8e8eE3emC8g5RsC_BNzU9l_8d5HCys7DNkMSvRlCCFfzsaXcbcO5aYsdQfBZjPjGV04lU7PxJcCem9WFqAshgzqJxe15Sz1JkKz6Tu_bmwdkBvWp_vxx_pzJiR)](https://mermaid-js.github.io/mermaid-live-editor/edit#pako:eNqVVD1PwzAQ_SuRp4KSgTWCAakVEywVC4pkXeNLMTjnynaKotL_jpukTaLGEDxYdt579-4j8oHlWiBLWa7A2qWErYEyo8ivdn9CQiPzJTh4BoItmuj-O0miNZq9zHFFual3DsXaaePRVhQAR8pXwkntH4aPRNqBk5rsHMupENfOHWtWpIzdZSxKklt_In-a04igYyhaqDkd7AWeX1m04QRGNbV7M-OJBh9a-LQ4lyQd5wuLqogj4Um80EqgiaMuJd96_on1w4smvOmVBCVyXfAP6_FCKuyhSy3-Oyphe0RpEItBEG5h3wmPw5wDNU4lPkrZt8jvQlrYKOQCG_nAL6Ow2fWfNt2nhsyliKMvUArnhr8e8eE3emC8g5RsC_BNzU9l_8d5HCys7DNkMSvRlCCFfzsaXcbcO5aYsdQfBZjPjGV04lU7PxJcCem9WFqAshgzqJxe15Sz1JkKz6Tu_bmwdkBvWp_vxx_pzJiR)
 
 ## Implementation Notes
 Efforts has been taken to provide `Service` data storage that is separate from existing data stores in order to keep those areas clean and simple. Where touchpoints are unavoidable, they are kept to the absolute bare minimum (e.g. `User.services` list in `users.json`, `Address.service_id` field).

docs/extensions/frontend-aspects.md

@@ -41,7 +41,7 @@ In your controller, the endpoint needs to be specified like this:
 ui = RubberduckService.blueprints["ui"]
 ```
 
-## Templates and Static Resources
+## Templates and static resources
 
 The minimal url routes for `Service` selection and management. As usualy in Flask, `templates` and `static` resources are in their respective subfolders. Please note that there is an additional directory with the id of the extension which looks redundant at first. This is due to the way blueprints are loading templates and ensures that there are no naming collisions. Maybe at a later stage, this can be used to let plugins override other plugin's templates.
 
@@ -66,8 +66,8 @@ You might have an extension which wants to inject e.g. JavaScript code into each
 
 For this to work, the extension needs to be activated for the user, though.
 
-### Extending dialogs
-You can extend the settings dialog or the wallet-dialog with your own templates. To do that, create a callback method in your service like:
+## Extending dialogs
+You can extend the settings dialog or the wallet dialog with your own templates. To do that, create a callback method in your service like:
 
 ```python
 from cryptoadvance.specter.services import callbacks
@@ -120,7 +120,7 @@ The `some_settingspage.jinja` should probably look exactly like all the other se
 		<input type="hidden" class="csrf-token" name="csrf_token" value="{{ csrf_token() }}"/>
 		<h1 id="title" class="settings-title">Settings</h1>
 		{% from 'settings/components/settings_menu.jinja' import settings_menu %}
-		{{ settings_menu('myext_something', current_user, setting_exts) }}
+		{{ settings_menu('myext_something', current_user, ext_settingstabs) }}
 		<div class="card" style="margin: 20px auto;">
 			<h1>{{ _("Something something") }} </h1>
 		</div>
@@ -146,3 +146,26 @@ A reasonable `mywalletdetails.jinja` would look like this:
 
 ![](./images/extensions/add_wallettabs.png)
 
+## Extending certain pages or complete endpoints
+
+For some endpoints, there is the possibility to extend/change parts of a page or the complete page. This works by declaring the `callback_adjust_view_model` method in your extension and modify the ViewModel which got passed into the callback. As there is only one callback for all types of ViewModels, you will need to check for the type that you're expecting and only adjust this type. Here is an example:
+
+```python
+from cryptoadvance.specter.server_endpoints.welcome.welcome_vm import WelcomeVm
+
+class ExtensionidService(Service):
+    [...}
+    def callback_adjust_view_model(self, view_model: WelcomeVm):
+        if type(view_model) == WelcomeVm:
+            # potentially, we could make a redirect here:
+            # view_model.about_redirect=url_for("spectrum_endpoint.some_enpoint_here")
+            # but we do it small here and only replace a specific component:
+            view_model.get_started_include = "spectrum/welcome/components/get_started.jinja"
+        return view_model
+```
+
+In this example, a certain part of the page gets replaced. As you can read in the comments, you could also trigger a complete redirect to a different endpoint.
+
+Currently, only two `ViewModels` are existing. Check them out. Don't hesitate to create an issue if you'd like to modify something where no ViewModel exists yet:
+- cryptoadvance.specter.server_endpoints.welcome.welcome_vm
+- cryptoadvance.specter.server_endpoints.wallets.wallets_vm

docs/extensions/nodes.md

@@ -0,0 +1,30 @@
+# Adding Nodes or NodeTypes
+
+The whole programming-model is based on the Bitcoin-Core API. So we need Bitcoin Core nodes or elements nodes or at least something which behaves like that. So for the Spectrum Integration, we made extending the node possible. This is a short description of how that has been done and which extensionpoints might be helpfull here.
+
+So to create your own Node, derive from `AbstractNode`:
+
+```
+from cryptoadvance.specter.node import AbstractNode
+
+class MyNode(AbstractNode):
+    # [...]
+    @classmethod
+    def from_json(cls, node_dict, *args, **kwargs):
+      [...]
+
+    def node_info_template(self):
+      return "spectrum/components/spectrum_info.jinja"
+```
+
+That class will need its own `fromJson` method. Overwrite the `node_info_template` method to specify your own template for the info-page which comes up if you click on a fully configured and functional node in the upper left corner. In order to smuggle your node into existence, you could potentially use the `callback_after_serverpy_init_app` callback. Have a look how the spectrum-extension did it [here](https://github.com/cryptoadvance/spectrum/pull/9/files#diff-82be7977bfa33bdbb0a448c7a03b43de90c4749565bef6737d6d516956ff0823R51-R62). Alternatively, you could create your own frontend in your controller and maybe additionally adjust the `WelcomeVm` model class as described in the frontend section.
+
+If the `node_settings` are clicked for that node, we also expect that you have a `node_settings` endpoint in your controller. Otherwise there will be errors. Something like:
+
+```
+@yourextension_endpoint.route("node/<node_alias>/", methods=["GET", "POST"])
+@login_required
+def node_settings(node_alias=None):
+    [...]
+    return render_template(...
+ ```

mkdocs.yml

@@ -44,6 +44,7 @@ nav:
       - extensions/data-storage.md
       - extensions/callbacks.md
       - extensions/devices.md
+      - extensions/nodes.md
 
 theme: 
   name: readthedocs

src/cryptoadvance/specter/helpers.py

@@ -121,8 +121,20 @@ def alias(name):
     return "".join(x for x in name if x.isalnum() or x == "_").lower()
 
 
+def fullpath(data_folder, name):
+    """Quick way to get a fullpath which usually"""
+    return os.path.join(data_folder, f"{alias(name)}.json")
+
+
+def calc_fullpath(data_folder, name):
+    """Get a fullpath for a Businessobject with a name quickly"""
+    return os.path.join(data_folder, f"{alias(name)}.json")
+
+
 def deep_update(d, u):
-    """updates the dict d with the dict u"""
+    """updates the dict d with the dict u
+    in short: second argument wins
+    """
     for k, v in six.iteritems(u):
         dv = d.get(k, {})
         if not isinstance(dv, collections.abc.Mapping):
@@ -151,6 +163,9 @@ def load_jsons(folder, key=None):
                 d["alias"] = fname[:-5]
                 dd[d[key]] = d
         except Exception as e:
+            logger.error(
+                f"Exception while loading json file {os.path.join(folder, fname)}"
+            )
             logger.exception(e)
     return dd
 

src/cryptoadvance/specter/internal_node.py

@@ -25,6 +25,7 @@ class InternalNode(Node):
     BROKEN = "Broken"
     DOWN = "Down"
     RUNNING = "Running"
+    external_node = False
 
     def __init__(
         self,
@@ -53,7 +54,6 @@ def __init__(
             port,
             host,
             protocol,
-            False,
             fullpath,
             "BTC",
             manager,
@@ -92,7 +92,6 @@ def from_json(cls, node_dict, manager, default_alias="", default_fullpath=""):
         port = node_dict.get("port", None)
         host = node_dict.get("host", "localhost")
         protocol = node_dict.get("protocol", "http")
-        external_node = node_dict.get("external_node", True)
         fullpath = node_dict.get("fullpath", default_fullpath)
         bitcoind_path = node_dict.get("bitcoind_path", "")
         bitcoind_network = node_dict.get("bitcoind_network", "main")

src/cryptoadvance/specter/managers/node_manager.py

@@ -1,14 +1,16 @@
+from gc import callbacks
 import os
 import logging
 import secrets
 import shutil
 
 from ..rpc import get_default_datadir, RPC_PORTS
-from ..specter_error import SpecterError
-from ..persistence import write_node, delete_file
-from ..helpers import alias, load_jsons
+from ..specter_error import SpecterError, SpecterInternalException
+from ..persistence import PersistentObject, write_node, delete_file
+from ..helpers import alias, calc_fullpath, load_jsons
 from ..node import Node
 from ..internal_node import InternalNode
+from ..services import callbacks
 from ..util.bitcoind_setup_tasks import setup_bitcoind_thread
 
 logger = logging.getLogger(__name__)
@@ -27,43 +29,45 @@ def __init__(
         internal_bitcoind_version="",
         data_folder="",
     ):
+        self.nodes = {}
         self.data_folder = data_folder
         self._active_node = active_node
         self.proxy_url = proxy_url
         self.only_tor = only_tor
         self.bitcoind_path = bitcoind_path
         self.internal_bitcoind_version = internal_bitcoind_version
-        self.update(data_folder)
+        self.load_from_disk(data_folder)
         internal_nodes = [
             node for node in self.nodes.values() if not node.external_node
         ]
         for node in internal_nodes:
             node.start()
 
-    def update(self, data_folder=None):
+    def load_from_disk(self, data_folder=None):
         if data_folder is not None:
             self.data_folder = data_folder
             if data_folder.startswith("~"):
                 data_folder = os.path.expanduser(data_folder)
             # creating folders if they don't exist
             if not os.path.isdir(data_folder):
                 os.mkdir(data_folder)
-        nodes = {}
         nodes_files = load_jsons(self.data_folder, key="name")
         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].get("external_node") else InternalNode
-            )
-            nodes[nodes_files[node_alias]["name"]] = node_class.from_json(
-                nodes_files[node_alias],
-                self,
-                default_alias=node_alias,
-                default_fullpath=fullpath,
-            )
-        if not nodes:
+            try:
+                self.nodes[
+                    nodes_files[node_alias]["name"]
+                ] = PersistentObject.from_json(
+                    nodes_files[node_alias],
+                    self,
+                    default_alias=node_alias,
+                    default_fullpath=calc_fullpath(self.data_folder, node_alias),
+                )
+            except SpecterInternalException as e:
+                logger.error(f"Skipping node {node_alias} due to {e}")
+
+        if not self.nodes:
             if os.environ.get("ELM_RPC_USER"):
-                self.add_node(
+                self.add_external_node(
                     node_type="ELM",
                     name="Blockstream Liquid",
                     autodetect=True,
@@ -73,10 +77,10 @@ def update(self, data_folder=None):
                     port=7041,
                     host="localhost",
                     protocol="http",
-                    external_node=True,
                     default_alias=self.DEFAULT_ALIAS,
                 )
-            self.add_node(
+            logger.info("Creating initial node-configuration")
+            self.add_external_node(
                 node_type="BTC",
                 name="Bitcoin Core",
                 autodetect=True,
@@ -86,11 +90,28 @@ def update(self, data_folder=None):
                 port=8332,
                 host="localhost",
                 protocol="http",
-                external_node=True,
                 default_alias=self.DEFAULT_ALIAS,
             )
-        else:
-            self.nodes = nodes
+
+        # Just to be sure here ....
+        has_default_node = False
+        for name, node in self.nodes.items():
+            if node.alias == self.DEFAULT_ALIAS:
+                return
+        # Make sure we always have a default node
+        # (needed for the rpc-as-pin-authentication, created and used for raspiblitz)
+        self.add_external_node(
+            node_type="BTC",
+            name="Bitcoin Core",
+            autodetect=True,
+            datadir=get_default_datadir(),
+            user="",
+            password="",
+            port=8332,
+            host="localhost",
+            protocol="http",
+            default_alias=self.DEFAULT_ALIAS,
+        )
 
     @property
     def active_node(self):
@@ -131,7 +152,7 @@ def update_bitcoind_version(self, specter, version):
         for node_alias in stopped_nodes:
             self.get_by_alias(node_alias).start(timeout=60)
 
-    def add_node(
+    def add_external_node(
         self,
         node_type,
         name,
@@ -142,12 +163,12 @@ def add_node(
         port,
         host,
         protocol,
-        external_node,
         default_alias=None,
     ):
         """Adding a node. Params:
         :param node_type: only valid for autodetect. Either BTC or ELM
-
+        This should only be used for an external node. Use add_internal_node for internal node
+        and if you have defined your own node-type, use save_node directly. to save the node (and create it yourself)
         """
         if not default_alias:
             node_alias = alias(name)
@@ -170,14 +191,22 @@ def add_node(
             port,
             host,
             protocol,
-            external_node,
             fullpath,
             node_type,
             self,
         )
-        logger.info(f"persisting {node} in add_node")
+        logger.info(f"persisting {node} in add_external_node")
+        self.nodes[name] = node
+        return self.save_node(node)
+
+    def save_node(self, node):
+        fullpath = (
+            node.fullpath
+            if hasattr(node, "fullpath")
+            else calc_fullpath(self.data_folder, node.name)
+        )
         write_node(node, fullpath)
-        self.update()  # reload files
+
         logger.info("Added new node {}".format(node.alias))
         return node
 
@@ -189,6 +218,10 @@ def add_internal_node(
         default_alias=None,
         datadir=None,
     ):
+        """Adding an internal node. Params:
+        This should only be used for internal nodes. Use add__External_node for external nodes
+        and if you have defined your own node-type, use save_node directly. to save the node (and create it yourself)
+        """
         if not default_alias:
             node_alias = alias(name)
         else:
@@ -218,11 +251,8 @@ def add_internal_node(
             network,
             self.internal_bitcoind_version,
         )
-        logger.info(f"persisting {node} in add_internal_node")
-        write_node(node, fullpath)
-        self.update()  # reload files
-        logger.info("Added new internal node {}".format(node.alias))
-        return node
+        self.nodes[name] = node
+        return self.save_node(node)
 
     def delete_node(self, node, specter):
         logger.info("Deleting {}".format(node.alias))
@@ -232,5 +262,4 @@ def delete_node(self, node, specter):
         if self._active_node == node.alias:
             specter.update_active_node(next(iter(self.nodes.values())).alias)
         del self.nodes[node.name]
-        self.update()
         logger.info("Node {} was deleted successfully".format(node.alias))