[Node] multi-tenant wallet architecture by Herklos · Pull Request #3429 · Drakkar-Software/OctoBot

Herklos · 2026-04-30T10:51:15Z

WARNING: contains breaking changes on wallets management without backward compat

GuillaumeDSM

great work ! this is a great feature!

GuillaumeDSM · 2026-05-01T08:02:29Z

+    def list_wallets(self) -> list:
+        """Return public wallet info (no key material)."""
+        return [
+            {"address": e["address"], "name": e.get("name"), "is_admin": e.get("is_admin", False)}


we should type this wallet info to benefit from type checking

GuillaumeDSM · 2026-05-01T08:04:26Z

+            raise ValueError("Passphrase must be at least 8 characters")
+        normalized = address.lower()
+        with self._wallet_lock:
+            node_wallets = self._get_node_wallets_list()
+            if any(e.get("address", "") == normalized for e in node_wallets):
+                raise ValueError(f"Wallet {address} already exists")
+            if is_admin and any(e.get("is_admin") for e in node_wallets):
+                raise ValueError("An admin wallet already exists")


can we use dedicated errors (at least one dedicated to WalletErrors) ? It will make error management easier

GuillaumeDSM · 2026-05-01T08:06:15Z

+            if fcntl is not None:
+                fcntl.flock(lock_fd.fileno(), fcntl.LOCK_EX)
+                lock_acquired = True


should we implement a windows version or is the lock need posix only?

GuillaumeDSM · 2026-05-01T08:07:34Z

+    def load(self) -> list:
+        wallets = self._sync_storage.get_item(constants.CONFIG_COMMUNITY_WALLETS) or {}
+        result = wallets.get(constants.CHAIN_TYPE, {}).get(constants.CHAIN_NETWORK, [])
+        if "iv" in result and "data" in result:


can wee use constants ? I don't understand what "iv" represents here

GuillaumeDSM · 2026-05-01T08:09:01Z

+def build_wallet_storage(sync_storage) -> WalletStorage:
+    """Factory: select the wallet storage backend from OCTOBOT_WALLET_STORAGE_BACKEND."""
+    backend = constants.WALLET_STORAGE_BACKEND.lower()
+    if backend in ("config", ""):


I think we should use an enum to identify backends

GuillaumeDSM · 2026-05-01T08:34:50Z

+# so direct imports always resolve to the correct copy (source or installed).
 try:
+    from api.route_provider import register_all_provider_routes
+    from api.routes import login, nodes, users, tasks, logs, setup, exchanges, wallets


I think we should prioritize tentacles.Services.Interfaces.node_api_interface imports as they are the target import (and not the fallback)

GuillaumeDSM · 2026-05-01T08:35:32Z

+
+def test_delete_tasks_requires_auth(client, mock_auth):
+    resp = client.delete(f"/api/v1/tasks/?taskIds={TENANT_TASK_ID}")
+    assert resp.status_code == 401


great tests !

GuillaumeDSM · 2026-05-01T08:36:09Z

-except ImportError:
    import utils
-    import api.main
+    import api.main as _api_main


same here, we should prioritize tentacles.Services.Interfaces.node_api_interface imports

GuillaumeDSM · 2026-05-01T08:40:02Z

is adding this file an error?

- Add WalletError hierarchy (errors.py): typed subclasses preserve ValueError/KeyError compat - Add WalletInfo / WalletEntry TypedDicts for type-safe wallet dicts - Add WalletStorageBackend enum (octobot/enums.py); use in build_wallet_storage factory - Replace raw "iv"/"data" string literals with _WALLET_BLOB_IV_KEY/_WALLET_BLOB_DATA_KEY constants - Replace fcntl-based file locking with filelock (cross-platform, pythonic); add to requirements.txt - Upgrade init_sync_client_for_wallet log from info → error for KeyError/ValueError; re-raise on Exception - Upgrade auto_init_sync_client ValueError log from info → warning with explicit message - Move _filter_by_wallet to workflows_util.filter_by_wallet; reuse in scheduler.get_pending_tasks/get_results - Replace ValueError message-string inspection in setup.py with typed WalletError except clauses - Flip import priority in main.py, node_api.py, tasks.py, users.py: tentacles-first, bare fallback - Drop stray test_core.py (simple_market_making) from this PR (unrelated to wallet architecture) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Patterns: enums in enums.py, constants in constants.py, typed error hierarchies, TypedDicts for structured dicts, filelock for cross-platform locking, tentacle import priority (tentacles-first), log level guidelines, shared filter helpers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Herklos · 2026-05-01T12:36:17Z

Thanks! It's up

GuillaumeDSM · 2026-05-01T14:17:00Z

+class WalletAlreadyExistsError(WalletError, ValueError):
+    pass
+
+
+class AdminWalletAlreadyExistsError(WalletError, ValueError):
+    pass
+
+
+class WalletNotFoundError(WalletError, KeyError):
+    pass
+
+
+class InvalidPassphraseError(WalletError, ValueError):
+    pass
+
+
+class CannotRemoveLastWalletError(WalletError, ValueError):
+    pass
+
+
+class CannotRemoveAdminWalletError(WalletError, ValueError):
+    pass
+
+
+class InvalidPrivateKeyError(WalletError, ValueError):
+    pass
+
+
+class PassphraseTooShortError(WalletError, ValueError):
+    pass


Why extending KeyError / ValueError ?

GuillaumeDSM · 2026-05-01T14:18:52Z

+class WalletInfo(typing.TypedDict):
+    address: str
+    name: typing.Optional[str]
+    is_admin: bool


GuillaumeDSM · 2026-05-01T14:20:02Z

+        # Addresses are stored lowercase; normalize input to match
+        normalized = address.lower()
+        for entry in self._get_node_wallets_list():
+            if entry.get("address", "") == normalized:


If we use a typeddict we should not need to use get() with a "magic"

GuillaumeDSM · 2026-05-01T14:20:16Z

+    def list_wallets(self) -> list[WalletInfo]:
+        """Return public wallet info (no key material)."""
+        return [
+            WalletInfo(address=e["address"], name=e.get("name"), is_admin=e.get("is_admin", False))


same here and in other places in this file

GuillaumeDSM · 2026-05-01T14:21:09Z

+    def decrypt_wallet_by_address(self, address: str, passphrase: str) -> sync_chain.Wallet:
+        entry = self._find_wallet_entry(address)
+        if entry is None:
+            raise WalletNotFoundError(f"Wallet {address} not found")


GuillaumeDSM · 2026-05-01T14:24:22Z

+                    replica_write_mode=constants.REPLICA_WRITE_MODE,
+                    replica_sync_interval_ms=constants.REPLICA_SYNC_INTERVAL_MS,
+                )
+            except (KeyError, ValueError) as e:


we should catch the custom errors here

GuillaumeDSM · 2026-05-01T14:25:05Z

+                    )
+                )
+            return True
+        except ValueError as e:


we should catch custom errors

GuillaumeDSM · 2026-05-01T14:28:27Z

+                passphrase=body.passphrase,
+                is_admin=True,
+            )
+    except (wallet_backend.WalletAlreadyExistsError, wallet_backend.AdminWalletAlreadyExistsError) as err:


GuillaumeDSM · 2026-05-01T14:29:06Z

-    wallet = auth.decrypt_node_wallet(credentials.password)
+    try:
+        wallet = auth.decrypt_wallet_by_address(current_user.email, credentials.password)
+    except KeyError:


should we use WalletNotFoundError?

GuillaumeDSM · 2026-05-01T14:30:58Z

+# sys.path insertion enables direct imports as a build-time fallback.
+_pkg_dir = str(pathlib.Path(__file__).resolve().parent)
+if _pkg_dir not in sys.path:
+    sys.path.insert(0, _pkg_dir)
+# Prefer the installed tentacles package (production target); fall back to
+# direct imports when running from source (build time, tests).


I don't think we should do this in the common code

Right, thanks

GuillaumeDSM

great work !

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Herklos self-assigned this Apr 30, 2026

Herklos requested a review from GuillaumeDSM as a code owner April 30, 2026 10:51

Herklos force-pushed the feature/add-multi-wallets-node branch 10 times, most recently from 3d47820 to 1ca74ca Compare April 30, 2026 17:03

GuillaumeDSM reviewed May 1, 2026

View reviewed changes

Herklos force-pushed the feature/add-multi-wallets-node branch from 1ca74ca to aaf094f Compare May 1, 2026 12:33

Herklos force-pushed the feature/add-multi-wallets-node branch from a5ae938 to b135c59 Compare May 1, 2026 12:35

GuillaumeDSM reviewed May 1, 2026

View reviewed changes

Herklos force-pushed the feature/add-multi-wallets-node branch 2 times, most recently from 8dc9ff4 to 7b8db2c Compare May 1, 2026 15:35

GuillaumeDSM approved these changes May 1, 2026

View reviewed changes

Herklos force-pushed the feature/add-multi-wallets-node branch from 7b8db2c to 6ee1a1b Compare May 1, 2026 17:10

Herklos enabled auto-merge (rebase) May 1, 2026 17:11

Herklos force-pushed the feature/add-multi-wallets-node branch from 6ee1a1b to 6a7a232 Compare May 1, 2026 17:55

[Node] multi-tenant wallet architecture

d101cbf

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Herklos force-pushed the feature/add-multi-wallets-node branch from 6a7a232 to d101cbf Compare May 1, 2026 18:42

Herklos merged commit ebe1b2e into dev May 1, 2026
21 checks passed

Herklos deleted the feature/add-multi-wallets-node branch May 1, 2026 18:47

Uh oh!

Conversation

Herklos commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GuillaumeDSM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeDSM May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Herklos commented May 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeDSM left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Herklos commented Apr 30, 2026 •

edited

Loading

GuillaumeDSM May 1, 2026 •

edited

Loading