cryptoadvance / cryptoadvance/specter-desktop
Active Node is not User/Session Aware
- Dominant language
- Python
- Stars
- 847
- Forks
- 259
- Avg merge
- 6d 18h
- Merged PRs (30d)
- 2
Description
Currently, the active node which is managed by the [NodeManager](https://github.com/cryptoadvance/specter-desktop/blob/master/src/cryptoadvance/specter/managers/node_manager.py#L96-L97) and it's effective for all users. This means that using more than one node and more than one user at a time is practically not usable.
We need to make the node-configuration an attribute of the session. Making that needs careful consideration which Managers are User/Session aware and which are not. As we'll see below, the `WalletManager` is almost the only one which needs bigger changes.
On top of that, the rest-interface though, should not have session-state at all and also shouldn't rely on any kind of state. But this problem comes after solving the user/session-awareness.
# Status Quo
The current node is quite deeply embedded in a lot of objects. The method to switch the node is in the specter-object [update_active_node](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/specter.py#L261-L265):
```
def update_active_node(self, node_alias):
"""update the current active node to use"""
self.config_manager.update_active_node(node_alias)
self.node_manager.switch_node(node_alias)
self.check()
```
The config-manager and the nodemanager are effectively only changing its internal data. The `ConfigManager` is already user-aware in some methods and this could also easily changed.
The `NodeManager` is storing the active Node and can probably also easily return data from a session (or not called at all any longer)
The problem is the [check-method](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/specter.py#L153-L182) which i'd consider to use an antipattern. Simplified, it does this:
* [ConfigManager.check_config()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/managers/config_manager.py#L77-L99)
* this is not problematic at all. There is no impact for the node and it's questionable whether this call is needed at all
* [Node(the activeOne).update_rpc()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/node.py#L201-L247)
* this is also not that problematic as this call is not called with any relevant parameter. However, it's calling check_info at the end, which might be relevant:
* [Node.check_info()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/node.py#L256-L322)
* This call is some sort of health-check for the node which also fills the node-object with the current state of the real node. It's not problematic in terms of state as long as this method is called regulary or if in doubt that the node is fully functional.
* [User.check()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/user.py#L252-L254) which itself calls:
* [DeviceManager.update()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/node.py#L256-L322)
* is simply reloading the devices from disk and therefore also not problematic. Devices are not node-relevant, anyway,
* [User.check_wallet_manager()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/user.py#L256-L283) which in turn calls [WalletManager.update()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/managers/wallet_manager.py#L52-L108)
* this is where more changes are needed. The signature of that message is: `def update(self, data_folder=None, rpc=None, chain=None, use_threading=True)`
* The update-method is flipping the WalletManager (and its managed wallets) upside down if `chain`, `rpc` or `data_folder` changes.
## WalletManager
Let's investigate [WalletManager.update()](https://github.com/cryptoadvance/specter-desktop/blob/e2ad1eaea13e5c67ea761e267227c27e58461056/src/cryptoadvance/specter/managers/wallet_manager.py#L52-L108) more closely.
# Considering the options
The main issue is on the `WalletManager`. Currently, changing the `rpc`,`chain` or `data_folder` effectively reconstructs the WalletManager-instance
# ToDos:
- [x] Refactor WalletManager #1667
- [ ] Figure out what the next step is
Contributor guide
Assessment
This issue has not been assessed yet.