Skip to content

Commit aaf094f

Browse files
Herklosclaude
andcommitted
[Node] address PR #3429 review comments
- 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>
1 parent 5832cc5 commit aaf094f

16 files changed

Lines changed: 237 additions & 132 deletions

File tree

octobot/community/authentication.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -697,9 +697,10 @@ def init_sync_client_for_wallet(self, address: str) -> None:
697697
replica_sync_interval_ms=constants.REPLICA_SYNC_INTERVAL_MS,
698698
)
699699
except (KeyError, ValueError) as e:
700-
self.logger.info(f"Sync client init skipped for {address}: {e}")
700+
self.logger.error(f"Sync client init skipped for {address}: {e}")
701701
except Exception as e:
702702
self.logger.exception(e, True, f"Failed to initialize sync client for wallet {address}: {e}")
703+
raise
703704

704705
def init_sync_client_with_passphrase(self, passphrase: str) -> None:
705706
"""Initialize the sync client using the admin wallet passphrase."""
@@ -752,7 +753,7 @@ def auto_init_sync_client(self) -> bool:
752753
)
753754
return True
754755
except ValueError as e:
755-
self.logger.info(f"Bot auto-unlock not available: {e}")
756+
self.logger.warning(f"Bot auto-unlock skipped — wallet unreachable or misconfigured: {e}")
756757
return False
757758
except Exception as e:
758759
self.logger.exception(e, True, f"Failed to auto-initialize sync client: {e}")

octobot/community/wallet_backend/__init__.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,18 @@
1818
from octobot.community.wallet_backend.community_wallet import (
1919
WalletBackend,
2020
)
21+
from octobot.community.wallet_backend import errors
22+
from octobot.community.wallet_backend.errors import (
23+
WalletError,
24+
WalletAlreadyExistsError,
25+
AdminWalletAlreadyExistsError,
26+
WalletNotFoundError,
27+
InvalidPassphraseError,
28+
CannotRemoveLastWalletError,
29+
CannotRemoveAdminWalletError,
30+
InvalidPrivateKeyError,
31+
PassphraseTooShortError,
32+
)
2133
from octobot.community.wallet_backend import wallet_storage
2234
from octobot.community.wallet_backend.wallet_storage import (
2335
WalletStorage,
@@ -29,6 +41,15 @@
2941

3042
__all__ = [
3143
"WalletBackend",
44+
"WalletError",
45+
"WalletAlreadyExistsError",
46+
"AdminWalletAlreadyExistsError",
47+
"WalletNotFoundError",
48+
"InvalidPassphraseError",
49+
"CannotRemoveLastWalletError",
50+
"CannotRemoveAdminWalletError",
51+
"InvalidPrivateKeyError",
52+
"PassphraseTooShortError",
3253
"WalletStorage",
3354
"ConfigJsonWalletStorage",
3455
"DedicatedFileWalletStorage",

octobot/community/wallet_backend/community_wallet.py

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@
2121
import typing
2222

2323
import octobot_sync.chain as sync_chain
24+
from octobot.community.wallet_backend.errors import (
25+
AdminWalletAlreadyExistsError,
26+
CannotRemoveAdminWalletError,
27+
CannotRemoveLastWalletError,
28+
InvalidPassphraseError,
29+
InvalidPrivateKeyError,
30+
PassphraseTooShortError,
31+
WalletAlreadyExistsError,
32+
WalletNotFoundError,
33+
)
2434
from octobot.community.wallet_backend.wallet_storage import (
2535
WalletStorage,
2636
build_wallet_storage,
@@ -30,6 +40,20 @@
3040
_PBKDF2_ALG = "sha256"
3141

3242

43+
class WalletInfo(typing.TypedDict):
44+
address: str
45+
name: typing.Optional[str]
46+
is_admin: bool
47+
48+
49+
class WalletEntry(typing.TypedDict):
50+
address: str
51+
name: typing.Optional[str]
52+
is_admin: bool
53+
private_key: str
54+
passphrase_hash: str
55+
56+
3357
def _hash_passphrase(passphrase: str) -> str:
3458
"""Hash a passphrase with PBKDF2-HMAC-SHA256 for storage. Format: salt_b64:key_b64."""
3559
salt = secrets.token_bytes(32)
@@ -60,33 +84,33 @@ def __init__(self, sync_storage, logger, storage: typing.Optional[WalletStorage]
6084
self._wallet_lock = threading.Lock()
6185
self._storage: WalletStorage = storage if storage is not None else build_wallet_storage(sync_storage)
6286

63-
def _get_node_wallets_list(self) -> list:
87+
def _get_node_wallets_list(self) -> list[WalletEntry]:
6488
return self._storage.load()
6589

66-
def _save_node_wallets_list(self, node_wallets: list) -> None:
90+
def _save_node_wallets_list(self, node_wallets: list[WalletEntry]) -> None:
6791
self._storage.save(node_wallets)
6892

69-
def _build_entry_fields(self, private_key_hex: str, passphrase: str) -> dict:
93+
def _build_entry_fields(self, private_key_hex: str, passphrase: str) -> dict[str, str]:
7094
return {
7195
"private_key": private_key_hex,
7296
"passphrase_hash": _hash_passphrase(passphrase),
7397
}
7498

75-
def _wallet_from_entry(self, entry: dict) -> sync_chain.Wallet:
99+
def _wallet_from_entry(self, entry: WalletEntry) -> sync_chain.Wallet:
76100
return sync_chain.Wallet(private_key=entry["private_key"], address=entry["address"])
77101

78-
def _find_wallet_entry(self, address: str) -> typing.Optional[dict]:
102+
def _find_wallet_entry(self, address: str) -> typing.Optional[WalletEntry]:
79103
# Addresses are stored lowercase; normalize input to match
80104
normalized = address.lower()
81105
for entry in self._get_node_wallets_list():
82106
if entry.get("address", "") == normalized:
83107
return entry
84108
return None
85109

86-
def list_wallets(self) -> list:
110+
def list_wallets(self) -> list[WalletInfo]:
87111
"""Return public wallet info (no key material)."""
88112
return [
89-
{"address": e["address"], "name": e.get("name"), "is_admin": e.get("is_admin", False)}
113+
WalletInfo(address=e["address"], name=e.get("name"), is_admin=e.get("is_admin", False))
90114
for e in self._get_node_wallets_list()
91115
]
92116

@@ -109,7 +133,7 @@ def import_wallet(
109133
try:
110134
address = sync_chain.address_from_evm_key(private_key)
111135
except Exception as err:
112-
raise ValueError(f"Invalid EVM private key: {err}") from err
136+
raise InvalidPrivateKeyError(f"Invalid EVM private key: {err}") from err
113137
return self._add_wallet_entry(private_key, address, name, passphrase, is_admin)
114138

115139
def _add_wallet_entry(
@@ -121,15 +145,15 @@ def _add_wallet_entry(
121145
is_admin: bool,
122146
) -> sync_chain.Wallet:
123147
if len(passphrase) < 8:
124-
raise ValueError("Passphrase must be at least 8 characters")
148+
raise PassphraseTooShortError("Passphrase must be at least 8 characters")
125149
normalized = address.lower()
126150
with self._wallet_lock:
127151
node_wallets = self._get_node_wallets_list()
128152
if any(e.get("address", "") == normalized for e in node_wallets):
129-
raise ValueError(f"Wallet {address} already exists")
153+
raise WalletAlreadyExistsError(f"Wallet {address} already exists")
130154
if is_admin and any(e.get("is_admin") for e in node_wallets):
131-
raise ValueError("An admin wallet already exists")
132-
entry = {
155+
raise AdminWalletAlreadyExistsError("An admin wallet already exists")
156+
entry: WalletEntry = {
133157
"address": normalized, # always store lowercase
134158
"name": name or None,
135159
"is_admin": is_admin,
@@ -139,52 +163,52 @@ def _add_wallet_entry(
139163
self._save_node_wallets_list(node_wallets)
140164
return sync_chain.Wallet(private_key=private_key, address=address)
141165

142-
def authenticate(self, address: str, passphrase: str) -> dict:
166+
def authenticate(self, address: str, passphrase: str) -> WalletInfo:
143167
"""Verify passphrase and return wallet metadata in a single storage read.
144168
145-
Returns {"is_admin": bool, "name": Optional[str]}.
146-
Raises KeyError if wallet not found, ValueError if passphrase incorrect.
169+
Returns WalletInfo with is_admin and name.
170+
Raises WalletNotFoundError if wallet not found, InvalidPassphraseError if passphrase incorrect.
147171
"""
148172
entry = self._find_wallet_entry(address)
149173
if entry is None:
150-
raise KeyError(f"Wallet {address} not found")
174+
raise WalletNotFoundError(f"Wallet {address} not found")
151175
if not _verify_passphrase_hash(passphrase, entry.get("passphrase_hash", "")):
152-
raise ValueError("Invalid passphrase")
153-
return {"is_admin": entry.get("is_admin", False), "name": entry.get("name")}
176+
raise InvalidPassphraseError("Invalid passphrase")
177+
return WalletInfo(is_admin=entry.get("is_admin", False), name=entry.get("name"), address=entry["address"])
154178

155179
def verify_wallet_passphrase(self, address: str, passphrase: str) -> bool:
156180
try:
157181
self.authenticate(address, passphrase)
158182
return True
159-
except (KeyError, ValueError):
183+
except (WalletNotFoundError, InvalidPassphraseError):
160184
return False
161185

162186
def decrypt_wallet_by_address(self, address: str, passphrase: str) -> sync_chain.Wallet:
163187
entry = self._find_wallet_entry(address)
164188
if entry is None:
165-
raise KeyError(f"Wallet {address} not found")
189+
raise WalletNotFoundError(f"Wallet {address} not found")
166190
if not _verify_passphrase_hash(passphrase, entry.get("passphrase_hash", "")):
167-
raise ValueError("Invalid passphrase")
191+
raise InvalidPassphraseError("Invalid passphrase")
168192
return self._wallet_from_entry(entry)
169193

170194
def get_wallet_for_bot(self, address: str) -> sync_chain.Wallet:
171195
"""Return wallet without passphrase verification — for bot auto-unlock at startup."""
172196
entry = self._find_wallet_entry(address)
173197
if entry is None:
174-
raise KeyError(f"Wallet {address} not found")
198+
raise WalletNotFoundError(f"Wallet {address} not found")
175199
return self._wallet_from_entry(entry)
176200

177201
def remove_wallet(self, address: str) -> None:
178202
normalized = address.lower()
179203
with self._wallet_lock:
180204
node_wallets = self._get_node_wallets_list()
181205
if len(node_wallets) <= 1:
182-
raise ValueError("Cannot remove the last wallet")
206+
raise CannotRemoveLastWalletError("Cannot remove the last wallet")
183207
entry = next((e for e in node_wallets if e.get("address", "") == normalized), None)
184208
if entry is None:
185-
raise KeyError(f"Wallet {address} not found")
209+
raise WalletNotFoundError(f"Wallet {address} not found")
186210
if entry.get("is_admin"):
187-
raise ValueError("Cannot remove the admin wallet")
211+
raise CannotRemoveAdminWalletError("Cannot remove the admin wallet")
188212
self._save_node_wallets_list([e for e in node_wallets if e.get("address", "") != normalized])
189213

190214
def rename_wallet(self, address: str, name: typing.Optional[str]) -> None:
@@ -196,7 +220,7 @@ def rename_wallet(self, address: str, name: typing.Optional[str]) -> None:
196220
entry["name"] = name or None
197221
self._save_node_wallets_list(node_wallets)
198222
return
199-
raise KeyError(f"Wallet {address} not found")
223+
raise WalletNotFoundError(f"Wallet {address} not found")
200224

201225
def is_admin_wallet(self, address: str) -> bool:
202226
entry = self._find_wallet_entry(address)
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
# This file is part of OctoBot (https://github.com/Drakkar-Software/OctoBot)
2+
# Copyright (c) 2025 Drakkar-Software, All rights reserved.
3+
#
4+
# OctoBot is free software; you can redistribute it and/or
5+
# modify it under the terms of the GNU General Public License
6+
# as published by the Free Software Foundation; either
7+
# version 3.0 of the License, or (at your option) any later version.
8+
#
9+
# OctoBot is distributed in the hope that it will be useful,
10+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
12+
# General Public License for more details.
13+
#
14+
# You should have received a copy of the GNU General Public
15+
# License along with OctoBot. If not, see <https://www.gnu.org/licenses/>.
16+
17+
18+
class WalletError(Exception):
19+
pass
20+
21+
22+
class WalletAlreadyExistsError(WalletError, ValueError):
23+
pass
24+
25+
26+
class AdminWalletAlreadyExistsError(WalletError, ValueError):
27+
pass
28+
29+
30+
class WalletNotFoundError(WalletError, KeyError):
31+
pass
32+
33+
34+
class InvalidPassphraseError(WalletError, ValueError):
35+
pass
36+
37+
38+
class CannotRemoveLastWalletError(WalletError, ValueError):
39+
pass
40+
41+
42+
class CannotRemoveAdminWalletError(WalletError, ValueError):
43+
pass
44+
45+
46+
class InvalidPrivateKeyError(WalletError, ValueError):
47+
pass
48+
49+
50+
class PassphraseTooShortError(WalletError, ValueError):
51+
pass

0 commit comments

Comments
 (0)