Conversation
WalkthroughIntroduces KDF migration: adds activation payload builder from local Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as activation._cli
participant Act as activation.build_activate_command
participant Cfg as COINS_CONFIG_FILE (local)
participant Net as Legacy/Remote (optional)
participant KDF as KDF RPC (kdf_proxy)
User->>CLI: request payload for TICKER
CLI->>Act: build_activate_command(TICKER)
Act->>Cfg: CoinsConfigLoader.load() (cached)
alt UTXO & missing electrum
Act->>Net: try legacy electrum merge (best-effort)
end
Act-->>CLI: JSON RPC payload {mmrpc, method, params, id}
CLI->>KDF: kdf_proxy(payload)
KDF-->>CLI: result / error
CLI-->>User: print JSON / exit
sequenceDiagram
autonumber
participant App as helpers.get_kdf
participant GH as GitHub Releases API
participant FS as Local FS
App->>GH: fetch latest KDF release (cached)
GH-->>App: release info or error
alt success
App->>FS: download asset -> SCRIPT_PATH/kdf
App->>FS: extract & rename binary to kdf/kdf(.exe)
App->>FS: chmod +x (non-Windows)
else fallback
App->>FS: use fallback/default metadata
end
App-->>App: return path to KDF binary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
const.py (1)
66-76: Don’t hard-exit on coins download; add status check, headers, and fallback to cached file.Current code exits the app offline/ratelimited. Use raise_for_status, a UA header, and atomic write with fallback.
-try: - print("Updating coins file...") - coins = requests.get(COINS_URL, timeout=5).json() - with open(COINS_FILE, "w", encoding="utf-8") as f: - json.dump(coins, f, ensure_ascii=False, indent=4) -except: - print( - f"Unable to load {COINS_FILE}, please check your internet connection, or report this to smk on Discord." - ) - sys.exit() +try: + print("Updating coins file...") + r = requests.get( + COINS_URL, + timeout=(3, 15), + headers={"User-Agent": "pytomicDEX-makerbot/0.2 (+https://github.com/smk762/pytomicDEX_makerbot)"}, + ) + r.raise_for_status() + coins = r.json() + tmp = f"{COINS_FILE}.tmp" + with open(tmp, "w", encoding="utf-8") as f: + json.dump(coins, f, ensure_ascii=False, indent=4) + os.replace(tmp, COINS_FILE) +except (requests.RequestException, ValueError) as e: + if os.path.exists(COINS_FILE): + print(f"Warning: coins update failed, using cached {COINS_FILE}: {e}") + else: + print(f"Unable to fetch coins and no cache present: {e}") + sys.exit(1)scan_seeds.py (1)
62-69: Fix inverted message and ensure file-exists logic is correct.Message claims the file was found in the else branch.
- if os.path.exists(TEMP_MM2_JSON_FILE): + if os.path.exists(TEMP_MM2_JSON_FILE): with open(TEMP_MM2_JSON_FILE, "r") as f: MM2_JSON = json.load(f) if "passphrase" in MM2_JSON: kdf_seed_phrase = MM2_JSON["passphrase"] else: - print(f"{TEMP_MM2_JSON_FILE} found, exiting...") + print(f"{TEMP_MM2_JSON_FILE} not found, exiting...") sys.exit()helpers.py (1)
108-123: Asset selection bug: for-else always prints “Release not found!” and may use undefined varsBreak on match and guard against missing assets.
-def get_kdf(branch=None): - assets = get_latest_kdf_release_info().get("assets", []) - for asset in assets: - print(asset) - if OP_SYS.lower() in asset["browser_download_url"].lower(): - asset_url = asset["browser_download_url"] - asset_name = asset["name"] - else: - error_print("Release not found!") +def get_kdf(branch=None): + assets = get_latest_kdf_release_info().get("assets", []) + asset_url = None + asset_name = None + for asset in assets: + # debug print removed + url = asset.get("browser_download_url", "") + if isinstance(url, str) and OP_SYS.lower() in url.lower(): + asset_url = url + asset_name = asset.get("name") + break + if not asset_url or not asset_name: + error_print("Release not found for current OS!") + return Nonemodels.py (5)
82-107: kdf_proxy has dead code, undefined variable ‘r’, and inconsistent RPC usageSimplify: call pykomodefi with v2=True, parse .json(), retry once on network failure.
- def kdf_proxy(self, params): - try: - resp = self.api.rpc(params["method"], params) - try: - pass - # resp = r.json() - except Exception as e: - print(f"Error getting response: {e}") - if "NewConnectionError" in str(r): - self.start() - r = self.api.rpc(params["method"], params) - resp = r.json() - else: - print(f"Response: {r}") - resp = r - except requests.exceptions.RequestException as e: - self.start() - resp = self.api.rpc(params["method"], params) - # resp = r.json() - if "error" in resp: - if resp["error"].find("Userpass is invalid"): - error_print( - "MM2 is rejecting your rpc_password. Please check you are not running additional instances of mm2 on the same port, and confirm your rpc_password conforms to constraints in https://developers.komodoplatform.com/basic-docs/atomicdex/atomicdex-setup/configure-mm2-json.html#mm2-json" - ) - sys.exit() - return resp + def kdf_proxy(self, params): + method = params.get("method") + if not method: + return {"error": "missing method"} + # Accept both mmrpc-style and flat dicts + call_params = params.get("params") + if call_params is None: + call_params = {k: v for k, v in params.items() if k not in {"mmrpc", "method", "id"}} + try: + r = self.api.rpc(method, call_params, v2=True) + resp = r.json() if hasattr(r, "json") else r + except requests.exceptions.RequestException: + self.start() + r = self.api.rpc(method, call_params, v2=True) + resp = r.json() if hasattr(r, "json") else r + if isinstance(resp, dict) and "error" in resp and "Userpass is invalid" in str(resp.get("error")): + error_print( + "KDF is rejecting your rpc_password. Ensure no other instance is running and verify your rpc_password." + ) + sys.exit() + return resp
109-139: Activation response handling uses wrong keys and may log incorrect balanceBalance is under result; also ensure status_print message uses that.
- if "result" in resp: - if "balance" in resp["result"]: - status_print(f"{coin} activated. Balance: {resp['balance']}") + if "result" in resp: + if isinstance(resp["result"], dict) and "balance" in resp["result"]: + status_print(f"{coin} activated. Balance: {resp['result']['balance']}")
283-292: cancel_all_orders calls missing v2 envelopeUnify structure for both branches.
if coin: params = { - "method": "cancel_all_orders", - "cancel_by": {"type": "Coin", "data": {"ticker": coin}}, + "mmrpc": "2.0", + "method": "cancel_all_orders", + "params": {"cancel_by": {"type": "Coin", "data": {"ticker": coin}}}, + "id": 0, } return self.kdf_proxy(params) - params = {"method": "cancel_all_orders", "cancel_by": {"type": "All"}} - return self.kdf_proxy(params) + return self.kdf_proxy({ + "mmrpc": "2.0", + "method": "cancel_all_orders", + "params": {"cancel_by": {"type": "All"}}, + "id": 0, + })
743-756: Undefined name ‘dex’ and malformed RPC in loop_viewsUse self.dex and a single RPC call.
- self.swaps_summary( - self.dex.api.rpc(dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) - ) + self.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}, v2=True).json())
1197-1204: Attribute mismatch and RPC shape
- enabled_coins_list doesn’t exist; use enabled_coins.
- Unify swaps view call as in Table.loop_views.
- self.table.balances(self.dex.enabled_coins_list) + self.table.balances(self.dex.enabled_coins) @@ - self.table.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) + self.table.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}, v2=True).json())
🧹 Nitpick comments (13)
kdf/.gitignore (1)
14-14: Also ignore the Windows binary.Add kdf.exe to avoid accidental commits on Windows.
kdf +kdf.exebalances.py (1)
8-8: Switch to Dex.enabled_coins — OK; minor readability nit.Use truthiness for empty lists.
-if len(coins_list) == 0: +if not coins_list:makerbot.py (1)
9-21: Avoid double-colorizing the header.Header already embeds ANSI sequences; wrapping it in colorize() may produce unexpected colors. Consider printing as-is.
- print(colorize(header, "lightgreen")) + print(header)scan_seeds.py (3)
35-45: Guard response shape before accessing address/balance.Avoid KeyError when fields are missing or nested.
- if "balance" in resp: - if float(resp["balance"]) > 0: + if isinstance(resp, dict) and "balance" in resp: + try: + bal = float(resp["balance"]) + except (TypeError, ValueError): + bal = 0.0 + if bal > 0: balance_found = True - seed_phrases[seed_phrase].update( + seed_phrases[seed_phrase].update( { - coin: { - "address": resp["address"], - "balance": resp["balance"], + coin: { + "address": resp.get("address"), + "balance": resp.get("balance"), } } )
50-53: Avoid bareexcept Exception; narrow the errors and include coin context once.Prevents swallowing unrelated bugs.
- except Exception as e: - print("---------------------------") - print(f"{coin}: {e}") - print("---------------------------") + except (KeyError, TypeError) as e: + print(f"{coin}: malformed response: {e}") + except Exception as e: + print(f"{coin}: unexpected error: {e}")
16-22: Function names still reference MM2; rename for clarity.update_MM2json now writes KDF config; consider renaming and switching to plain "w" mode.
-def update_MM2json(seed_phrase): +def update_kdf_json(seed_phrase): @@ - with open(TEMP_MM2_JSON_FILE, "w+") as f: + with open(TEMP_MM2_JSON_FILE, "w", encoding="utf-8") as f: json.dump(kdf_conf, f, indent=4)activation.py (2)
1-1: Shebang present but file may not be executableEither make the file executable in the repo or drop the shebang to silence EXE001.
6-6: Add ClassVar and logging import for cache + diagnosticsPrepare for proper class attribute typing and minimal logging.
-from typing import Any, Dict, Optional, Tuple, List +from typing import Any, Dict, Optional, Tuple, List, ClassVar +import logginghelpers.py (2)
79-106: Latest-release fetch: narrow exceptions and simpler endpointCatching all Exceptions hides actionable errors; also GitHub supports /releases/latest which reduces payload.
- releases_url = "https://api.github.com/repos/KomodoPlatform/komodo-defi-framework/releases" + releases_url = "https://api.github.com/repos/KomodoPlatform/komodo-defi-framework/releases/latest" @@ - r = requests.get(releases_url, timeout=8) + r = requests.get(releases_url, timeout=8) r.raise_for_status() - data = r.json() - if isinstance(data, list) and len(data) > 0: - latest = data[0] - _LATEST_KDF_CACHE.update({"ts": now, "data": latest}) - return latest - except Exception: - pass + latest = r.json() + if isinstance(latest, dict): + _LATEST_KDF_CACHE.update({"ts": now, "data": latest}) + return latest + except (requests.Timeout, requests.ConnectionError, requests.HTTPError, ValueError) as e: + status_print(f"Warning: failed to fetch latest KDF release info: {e}")If pre-release filtering is needed, we can extend this to list releases and pick the first non-prerelease.
231-237: Add timeout and basic diagnostics to price fetchAvoid hanging indefinitely and surface errors once.
- for i in PRICE_URLS: + for i in PRICE_URLS: try: - return requests.get(i).json() - except Exception as e: - print(f"Price service at {i} is not responding!") + return requests.get(i, timeout=8).json() + except requests.RequestException: + status_print(f"Price service at {i} is not responding!") return {}models.py (3)
1233-1236: Minor fixes: extraneous f-strings and time delta parens
- Remove f-prefix where no interpolation.
- Ensure milliseconds-to-seconds subtraction is parenthesized.
- address = color_input(f"Enter the destination address: ") + address = color_input("Enter the destination address: ") @@ - address = color_input(f"Enter the destination address: ") + address = color_input("Enter the destination address: ")- since = sec_to_hms(int(time.time()) - int(updated) / 1000) + since = sec_to_hms(int(time.time() - (int(updated) / 1000)))Also applies to: 656-656
600-607: User-facing typos: “seperated” -> “separated”Tweak prompts for polish.
- "Enter tickers of coins you want to sell, seperated by a space (default: KMD LTC):\n", + "Enter tickers of coins you want to sell, separated by a space (default: KMD LTC):\n", @@ - "Enter tickers of coins you want to buy, seperated by a space (press enter to use same coins as above):\n", + "Enter tickers of coins you want to buy, separated by a space (press enter to use same coins as above):\n",Also applies to: 826-851
41-52: Leverage helpers in status for consistency and reduce broad exceptsThe multiple broad try/except blocks in status make debugging hard. Consider centralizing error handling and logging minimal diagnostics (counts fallback to 0 is fine).
Would you like a follow-up patch to normalize status collection with small helpers and narrower exceptions?
Also applies to: 197-202
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
activation.py(1 hunks)balances.py(1 hunks)const.py(2 hunks)helpers.py(6 hunks)kdf/.gitignore(1 hunks)kdf/update_coins.sh(1 hunks)kdf/update_kdf.sh(1 hunks)makerbot.py(1 hunks)mm2/update_coins.sh(0 hunks)mm2/update_mm2.sh(0 hunks)models.py(20 hunks)requirements.txt(1 hunks)scan_seeds.py(3 hunks)
💤 Files with no reviewable changes (2)
- mm2/update_coins.sh
- mm2/update_mm2.sh
🧰 Additional context used
🧬 Code graph analysis (3)
helpers.py (1)
activation.py (2)
build_activate_command(147-230)load(17-40)
scan_seeds.py (2)
activation.py (3)
get_all_coins_from_config(55-59)build_activate_command(147-230)load(17-40)models.py (3)
Dex(44-345)kdf_proxy(82-107)disable_coin(298-299)
models.py (2)
helpers.py (14)
get_kdf(108-147)get_price(34-42)get_order_count(159-173)generate_rpc_pass(238-253)get_prices(230-236)compute_kdf_version_suffix(292-309)center_visible(281-289)get_seednodes_list(318-349)error_print(223-224)preexec(155-156)success_print(219-220)sleep_message(150-152)color_input(203-204)status_print(215-216)activation.py (2)
build_activate_command(147-230)load(17-40)
🪛 Shellcheck (0.10.0)
kdf/update_kdf.sh
[warning] 7-7: Quote this to prevent word splitting.
(SC2046)
🪛 Ruff (0.12.2)
activation.py
1-1: Shebang is present but file is not executable
(EXE001)
14-14: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
38-39: try-except-pass detected, consider logging the exception
(S110)
38-38: Do not catch blind exception: Exception
(BLE001)
106-107: try-except-pass detected, consider logging the exception
(S110)
106-106: Do not catch blind exception: Exception
(BLE001)
helpers.py
96-97: try-except-pass detected, consider logging the exception
(S110)
96-96: Do not catch blind exception: Exception
(BLE001)
108-108: Unused function argument: branch
(ARG001)
141-141: Do not catch blind exception: Exception
(BLE001)
308-308: Do not catch blind exception: Exception
(BLE001)
330-330: Do not catch blind exception: Exception
(BLE001)
330-330: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
345-345: Consider moving this statement to an else block
(TRY300)
346-346: Do not catch blind exception: Exception
(BLE001)
scan_seeds.py
50-50: Do not catch blind exception: Exception
(BLE001)
models.py
67-67: subprocess call: check for execution of untrusted input
(S603)
88-88: Do not catch blind exception: Exception
(BLE001)
90-90: Undefined name r
(F821)
97-97: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
164-164: Do not catch blind exception: Exception
(BLE001)
170-170: Do not catch blind exception: Exception
(BLE001)
171-171: Local variable maker_orders is assigned to but never used
Remove assignment to unused variable maker_orders
(F841)
171-171: Local variable taker_orders is assigned to but never used
Remove assignment to unused variable taker_orders
(F841)
177-177: Do not catch blind exception: Exception
(BLE001)
183-183: Do not catch blind exception: Exception
(BLE001)
188-188: Do not catch blind exception: Exception
(BLE001)
194-194: Do not catch blind exception: Exception
(BLE001)
229-229: Do not catch blind exception: Exception
(BLE001)
755-755: Undefined name dex
(F821)
1233-1233: f-string without any placeholders
Remove extraneous f prefix
(F541)
1236-1236: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (5)
requirements.txt (1)
2-3: Confirm/update pinned packages in requirements.txt
- requests is pinned to 2.32.0 but PyPI latest is 2.32.5 — either bump to 2.32.5 or confirm 2.32.0 is intentional and has no compatibility/security reasons.
- pykomodefi==0.2.6 is already the latest.
File: requirements.txt (lines 2–3).
const.py (1)
25-27: KDF path/log/config constants — LGTM.OS switch, KDFBIN, KDF_LOG_FILE, and COINS_CONFIG_FILE are consistent with the migration.
Also applies to: 54-54, 60-61
scan_seeds.py (1)
28-35: Verify request envelope matches kdf_proxy expectations.build_activate_command returns a full MMRPC envelope; models.Dex.kdf_proxy appears to call api.rpc(method, params). Ensure it unpacks the envelope (method + params) or adapt here.
Would you like me to generate a patch for models.Dex.kdf_proxy to accept full envelopes?
activation.py (2)
214-228: UTXO: empty Electrum servers produce invalid activationIf no servers are found, KDF may reject the payload.
Consider failing fast with a clear error or skipping activation when electrum_servers is empty. If KDF supports bootstrapping without servers, ignore this.
90-108: Keep legacycoinsfile path; log the exception instead of swallowing itactivation.py (lines 90–108): the legacy file is intentionally SCRIPT_PATH/coins (no extension) — const.py defines COINS_FILE and kdf/update_coins.sh downloads the upstream
coinsfile. Do not change toconfig/coins.json. Replace the bareexcept Exception: passwith a logged debug message (e.g.logging.debug("Legacy coins fallback failed: %s", e)) so failures aren't silently swallowed.Likely an incorrect or invalid review comment.
| _cache: Dict[str, Any] = {"ts": 0, "data": None} | ||
|
|
||
| @classmethod | ||
| def load(cls) -> Dict[str, Any]: | ||
| try: | ||
| if cls._cache.get("data"): | ||
| return cls._cache["data"] | ||
| if os.path.exists(COINS_CONFIG_FILE): |
There was a problem hiding this comment.
Cache TTL unused and blind-except; annotate cache, add TTL and logging
Current code caches forever and swallows errors. Use a small TTL and log once.
-class CoinsConfigLoader:
+class CoinsConfigLoader:
@@
- _cache: Dict[str, Any] = {"ts": 0, "data": None}
+ _cache: ClassVar[Dict[str, Any]] = {"ts": 0, "data": None}
+ _ttl_seconds: ClassVar[int] = 300
@@
- try:
- if cls._cache.get("data"):
- return cls._cache["data"]
+ try:
+ if cls._cache.get("data") and (time.time() - (cls._cache.get("ts") or 0) < cls._ttl_seconds):
+ return cls._cache["data"]
@@
- except Exception:
- pass
+ except Exception as e:
+ logging.debug(f"CoinsConfigLoader.load() failed: {e}")
return {}Also applies to: 36-40
🧰 Tools
🪛 Ruff (0.12.2)
14-14: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
🤖 Prompt for AI Agents
In activation.py around lines 14-21 (and similarly apply to lines 36-40), the
cache currently lives forever and exceptions are being swallowed; add a TTL
check and proper logging: annotate the cache more clearly (keep _cache:
Dict[str, Any] but ensure "ts" is float and "data" optional), define a small TTL
constant (e.g. TTL_SECONDS) and, in load(), check if time.time() -
cls._cache["ts"] < TTL_SECONDS before returning cached data; if expired or
missing, proceed to load and update cls._cache["ts"] and cls._cache["data"].
Replace the bare except with a narrow exception (e.g. Exception as e) and call
the module logger once to record the error before re-raising or returning a safe
default.
| nodes = _normalize_eth_nodes(pdata.get("nodes") or pdata.get("rpc")) | ||
| if nodes: | ||
| params["nodes"] = nodes | ||
| sc = pdata.get("swap_contract_address") or pdata.get("swap_contract") or pdata.get("swap") or pdata.get("swap_contracts") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
ETH nodes normalization misses rpc_urls
If nodes/rpc aren’t present but rpc_urls is, ETH activation will omit nodes.
- nodes = _normalize_eth_nodes(pdata.get("nodes") or pdata.get("rpc"))
+ nodes = _normalize_eth_nodes(pdata.get("nodes") or pdata.get("rpc") or pdata.get("rpc_urls"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| nodes = _normalize_eth_nodes(pdata.get("nodes") or pdata.get("rpc")) | |
| if nodes: | |
| params["nodes"] = nodes | |
| sc = pdata.get("swap_contract_address") or pdata.get("swap_contract") or pdata.get("swap") or pdata.get("swap_contracts") | |
| nodes = _normalize_eth_nodes(pdata.get("nodes") or pdata.get("rpc") or pdata.get("rpc_urls")) | |
| if nodes: | |
| params["nodes"] = nodes | |
| sc = pdata.get("swap_contract_address") or pdata.get("swap_contract") or pdata.get("swap") or pdata.get("swap_contracts") |
🤖 Prompt for AI Agents
In activation.py around lines 160 to 163, the code only checks pdata["nodes"] or
pdata["rpc"] before normalizing ETH nodes, so when those keys are absent but
pdata["rpc_urls"] exists the nodes list is omitted; update the conditional to
fall back to pdata.get("rpc_urls") (and/or pdata.get("rpcUrl") if used
elsewhere) so _normalize_eth_nodes receives pdata.get("nodes") or
pdata.get("rpc") or pdata.get("rpc_urls"); ensure the resulting value is passed
through _normalize_eth_nodes and, if non-empty, assigned to params["nodes"]
exactly as the original flow.
| target_dir = f"{SCRIPT_PATH}/kdf" | ||
| if not os.path.isdir(target_dir): | ||
| os.makedirs(target_dir, exist_ok=True) | ||
| download_path = f"{target_dir}/{asset_name}" | ||
| if not os.path.exists(download_path): |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden download/extract + correct permissions
- Ensure target file vars exist (after guarding above).
- Set executable to 0o755 (not only X bit), otherwise chmod resets perms to execute only.
download_path = f"{target_dir}/{asset_name}"
@@
- if OP_SYS.lower() != "windows":
- print("setting perms")
- os.chmod(new_path, stat.S_IEXEC)
+ if OP_SYS.lower() != "windows":
+ # ensure rwxr-xr-x
+ os.chmod(new_path, 0o755)Also applies to: 129-146
🤖 Prompt for AI Agents
helpers.py lines 119-123 (also apply same changes to 129-146): after creating
target_dir, ensure download_path and any other target file variables are defined
only after the dir-exists guard; when setting permissions after download/extract
use an explicit mode of 0o755 (not toggling only the execute bit) so
owner/group/other read+execute and owner write are preserved; also harden the
download/extract sequence by validating the downloaded file exists and is fully
written before extraction, and use atomic moves (download to a temp file then
rename) to avoid partial files.
| def get_seednodes_list(): | ||
| """Return a list of host values from seed-nodes.json. | ||
|
|
||
| Prefers local config/seed-nodes.json. If missing, attempts to download. | ||
| Returns [] on failure. | ||
| """ | ||
| try: | ||
| print(f"Getting seed nodes list from {_SEED_NODES_PATH}...") | ||
| if os.path.exists(_SEED_NODES_PATH): | ||
| with open(_SEED_NODES_PATH, "r", encoding="utf-8") as f: | ||
| try: | ||
| data = json.load(f) | ||
| except Exception as e: | ||
| os.remove(_SEED_NODES_PATH) | ||
| return get_seednodes_list() | ||
| else: | ||
| r = requests.get(_SEED_NODES_URL, timeout=8) | ||
| r.raise_for_status() | ||
| data = r.json() | ||
| os.makedirs(os.path.dirname(_SEED_NODES_PATH), exist_ok=True) | ||
| with open(_SEED_NODES_PATH, "w", encoding="utf-8") as f: | ||
| json.dump(data, f, ensure_ascii=False, indent=2) | ||
| hosts = [] | ||
| if isinstance(data, list): | ||
| for entry in data: | ||
| if isinstance(entry, dict) and entry.get("host"): | ||
| hosts.append(entry.get("host")) | ||
| return hosts | ||
| except Exception as e: | ||
| print(f"Error getting seed nodes list: {e}") | ||
| print(f"Please update the seed nodes list in your MM2.json file {MM2_JSON_FILE}") | ||
| sys.exit() |
There was a problem hiding this comment.
Do not sys.exit() in a library helper; return [] on failure (docstring promises this)
Exiting here kills the whole TUI unexpectedly.
def get_seednodes_list():
@@
- except Exception as e:
- print(f"Error getting seed nodes list: {e}")
- print(f"Please update the seed nodes list in your MM2.json file {MM2_JSON_FILE}")
- sys.exit()
+ except Exception as e:
+ error_print(f"Error getting seed nodes list: {e}")
+ status_print(f"Please update seed nodes in {MM2_JSON_FILE} or ensure {_SEED_NODES_URL} is reachable.")
+ return []📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_seednodes_list(): | |
| """Return a list of host values from seed-nodes.json. | |
| Prefers local config/seed-nodes.json. If missing, attempts to download. | |
| Returns [] on failure. | |
| """ | |
| try: | |
| print(f"Getting seed nodes list from {_SEED_NODES_PATH}...") | |
| if os.path.exists(_SEED_NODES_PATH): | |
| with open(_SEED_NODES_PATH, "r", encoding="utf-8") as f: | |
| try: | |
| data = json.load(f) | |
| except Exception as e: | |
| os.remove(_SEED_NODES_PATH) | |
| return get_seednodes_list() | |
| else: | |
| r = requests.get(_SEED_NODES_URL, timeout=8) | |
| r.raise_for_status() | |
| data = r.json() | |
| os.makedirs(os.path.dirname(_SEED_NODES_PATH), exist_ok=True) | |
| with open(_SEED_NODES_PATH, "w", encoding="utf-8") as f: | |
| json.dump(data, f, ensure_ascii=False, indent=2) | |
| hosts = [] | |
| if isinstance(data, list): | |
| for entry in data: | |
| if isinstance(entry, dict) and entry.get("host"): | |
| hosts.append(entry.get("host")) | |
| return hosts | |
| except Exception as e: | |
| print(f"Error getting seed nodes list: {e}") | |
| print(f"Please update the seed nodes list in your MM2.json file {MM2_JSON_FILE}") | |
| sys.exit() | |
| def get_seednodes_list(): | |
| """Return a list of host values from seed-nodes.json. | |
| Prefers local config/seed-nodes.json. If missing, attempts to download. | |
| Returns [] on failure. | |
| """ | |
| try: | |
| print(f"Getting seed nodes list from {_SEED_NODES_PATH}...") | |
| if os.path.exists(_SEED_NODES_PATH): | |
| with open(_SEED_NODES_PATH, "r", encoding="utf-8") as f: | |
| try: | |
| data = json.load(f) | |
| except Exception as e: | |
| os.remove(_SEED_NODES_PATH) | |
| return get_seednodes_list() | |
| else: | |
| r = requests.get(_SEED_NODES_URL, timeout=8) | |
| r.raise_for_status() | |
| data = r.json() | |
| os.makedirs(os.path.dirname(_SEED_NODES_PATH), exist_ok=True) | |
| with open(_SEED_NODES_PATH, "w", encoding="utf-8") as f: | |
| json.dump(data, f, ensure_ascii=False, indent=2) | |
| hosts = [] | |
| if isinstance(data, list): | |
| for entry in data: | |
| if isinstance(entry, dict) and entry.get("host"): | |
| hosts.append(entry.get("host")) | |
| return hosts | |
| except Exception as e: | |
| error_print(f"Error getting seed nodes list: {e}") | |
| status_print(f"Please update seed nodes in {MM2_JSON_FILE} or ensure {_SEED_NODES_URL} is reachable.") | |
| return [] |
🧰 Tools
🪛 Ruff (0.12.2)
330-330: Do not catch blind exception: Exception
(BLE001)
330-330: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
345-345: Consider moving this statement to an else block
(TRY300)
346-346: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In helpers.py around lines 318 to 349, the top-level except block calls
sys.exit() which violates the function docstring and kills the TUI; change that
block to log/print the error and then return an empty list instead of exiting
the process so callers can handle failure; ensure any other early-failure paths
also return [] (not raise or exit), and keep the existing prints for visibility.
| #!/bin/bash | ||
| wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/coins -O coins | ||
| mkdir -p ../config | ||
| wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/utils/coins_config.json -O ../config/coins_config.json | ||
| wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/seed-nodes.json -O ../config/seed-nodes.json |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add fail-fast and atomic writes for reliability.
Prevents partial/corrupt files and surfaces errors clearly.
-#!/bin/bash
-wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/coins -O coins
-mkdir -p ../config
-wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/utils/coins_config.json -O ../config/coins_config.json
-wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/seed-nodes.json -O ../config/seed-nodes.json
+#!/bin/bash
+set -euo pipefail
+BASE_URL="https://raw.githubusercontent.com/KomodoPlatform/coins/master"
+mkdir -p ../config
+wget -qO coins.tmp "$BASE_URL/coins"
+mv -f coins.tmp coins
+wget -qO ../config/coins_config.json.tmp "$BASE_URL/utils/coins_config.json"
+mv -f ../config/coins_config.json.tmp ../config/coins_config.json
+wget -qO ../config/seed-nodes.json.tmp "$BASE_URL/seed-nodes.json"
+mv -f ../config/seed-nodes.json.tmp ../config/seed-nodes.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/bin/bash | |
| wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/coins -O coins | |
| mkdir -p ../config | |
| wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/utils/coins_config.json -O ../config/coins_config.json | |
| wget https://raw.githubusercontent.com/KomodoPlatform/coins/master/seed-nodes.json -O ../config/seed-nodes.json | |
| #!/bin/bash | |
| set -euo pipefail | |
| BASE_URL="https://raw.githubusercontent.com/KomodoPlatform/coins/master" | |
| mkdir -p ../config | |
| wget -qO coins.tmp "$BASE_URL/coins" | |
| mv -f coins.tmp coins | |
| wget -qO ../config/coins_config.json.tmp "$BASE_URL/utils/coins_config.json" | |
| mv -f ../config/coins_config.json.tmp ../config/coins_config.json | |
| wget -qO ../config/seed-nodes.json.tmp "$BASE_URL/seed-nodes.json" | |
| mv -f ../config/seed-nodes.json.tmp ../config/seed-nodes.json |
🤖 Prompt for AI Agents
In kdf/update_coins.sh lines 1-5, the script currently downloads files directly
which can leave partial/corrupt files and hides failures; make it fail-fast and
perform atomic writes by enabling strict bash options (set -euo pipefail),
download each remote file to a secure temporary file (mktemp), verify wget/curl
exit status, and then move the temp file into place with mv (which is atomic on
same filesystem); also register a trap to remove temp files on exit/interrupt
and ensure parent directories exist before moving.
| def get_recent_swaps(self, limit=100): | ||
| return self.mm2_proxy({"method": "my_recent_swaps", "limit": limit}) | ||
| return self.kdf_proxy({"method": "my_recent_swaps", "limit": limit}) | ||
|
|
There was a problem hiding this comment.
my_recent_swaps call missing mmrpc envelope and params
This will likely be rejected by KDF v2.
- return self.kdf_proxy({"method": "my_recent_swaps", "limit": limit})
+ return self.kdf_proxy({"mmrpc": "2.0", "method": "my_recent_swaps", "params": {"limit": limit}, "id": 0})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_recent_swaps(self, limit=100): | |
| return self.mm2_proxy({"method": "my_recent_swaps", "limit": limit}) | |
| return self.kdf_proxy({"method": "my_recent_swaps", "limit": limit}) | |
| def get_recent_swaps(self, limit=100): | |
| return self.kdf_proxy({"mmrpc": "2.0", "method": "my_recent_swaps", "params": {"limit": limit}, "id": 0}) |
🤖 Prompt for AI Agents
In models.py around lines 279 to 281, the direct call self.kdf_proxy({"method":
"my_recent_swaps", "limit": limit}) lacks the required mmrpc envelope and nested
params for KDF v2; wrap the request in the mmrpc JSON-RPC envelope and pass the
actual method and its parameters under params (e.g. construct a payload with
"method": "mmrpc", and "params": {"method":"my_recent_swaps", "params":
{"limit": limit}}), then send that payload to self.kdf_proxy so KDF v2 accepts
the call.
| def get_balance(self, coin): | ||
| return self.mm2_proxy({"method": "my_balance", "coin": coin}) | ||
| return self.kdf_proxy({"method": "my_balance", "coin": coin}) | ||
|
|
||
| # https://developers.komodoplatform.com/basic-docs/atomicdex-api-legacy/my_balance.html | ||
| def disable_coin(self, coin): | ||
| return self.mm2_proxy({"method": "disable_coin", "coin": coin}) | ||
| return self.kdf_proxy({"method": "disable_coin", "coin": coin}) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
get_balance/disable_coin missing v2 envelope
These currently send flat payloads.
- return self.kdf_proxy({"method": "my_balance", "coin": coin})
+ return self.kdf_proxy({"mmrpc": "2.0", "method": "my_balance", "params": {"coin": coin}, "id": 0})
@@
- return self.kdf_proxy({"method": "disable_coin", "coin": coin})
+ return self.kdf_proxy({"mmrpc": "2.0", "method": "disable_coin", "params": {"coin": coin}, "id": 0})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_balance(self, coin): | |
| return self.mm2_proxy({"method": "my_balance", "coin": coin}) | |
| return self.kdf_proxy({"method": "my_balance", "coin": coin}) | |
| # https://developers.komodoplatform.com/basic-docs/atomicdex-api-legacy/my_balance.html | |
| def disable_coin(self, coin): | |
| return self.mm2_proxy({"method": "disable_coin", "coin": coin}) | |
| return self.kdf_proxy({"method": "disable_coin", "coin": coin}) | |
| def get_balance(self, coin): | |
| return self.kdf_proxy({"mmrpc": "2.0", "method": "my_balance", "params": {"coin": coin}, "id": 0}) | |
| # https://developers.komodoplatform.com/basic-docs/atomicdex-api-legacy/my_balance.html | |
| def disable_coin(self, coin): | |
| return self.kdf_proxy({"mmrpc": "2.0", "method": "disable_coin", "params": {"coin": coin}, "id": 0}) |
🤖 Prompt for AI Agents
models.py around lines 294–300: both get_balance and disable_coin are sending
flat payloads; change them to send a JSON‑RPC v2 envelope instead (include
"jsonrpc": "2.0", a unique "id", the "method" string, and pass coin inside a
"params" object) so the payload becomes a proper v2 request with method and
params rather than a flat map.
| def send_raw_tx(self, coin, tx_hex): | ||
| return self.mm2_proxy( | ||
| return self.kdf_proxy( | ||
| {"method": "send_raw_transaction", "coin": coin, "tx_hex": tx_hex} | ||
| ) | ||
|
|
||
| # https://developers.komodoplatform.com/basic-docs/atomicdex-api-legacy/validateaddress.html | ||
| def validate_address(self, coin, address): | ||
| return self.mm2_proxy( | ||
| return self.kdf_proxy( | ||
| {"method": "validateaddress", "coin": coin, "address": address} | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
send_raw_transaction and validateaddress missing v2 envelope
Align to mmrpc 2.0.
- return self.kdf_proxy(
- {"method": "send_raw_transaction", "coin": coin, "tx_hex": tx_hex}
- )
+ return self.kdf_proxy({"mmrpc": "2.0", "method": "send_raw_transaction", "params": {"coin": coin, "tx_hex": tx_hex}, "id": 0})
@@
- return self.kdf_proxy(
- {"method": "validateaddress", "coin": coin, "address": address}
- )
+ return self.kdf_proxy({"mmrpc": "2.0", "method": "validateaddress", "params": {"coin": coin, "address": address}, "id": 0})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def send_raw_tx(self, coin, tx_hex): | |
| return self.mm2_proxy( | |
| return self.kdf_proxy( | |
| {"method": "send_raw_transaction", "coin": coin, "tx_hex": tx_hex} | |
| ) | |
| # https://developers.komodoplatform.com/basic-docs/atomicdex-api-legacy/validateaddress.html | |
| def validate_address(self, coin, address): | |
| return self.mm2_proxy( | |
| return self.kdf_proxy( | |
| {"method": "validateaddress", "coin": coin, "address": address} | |
| ) | |
| def send_raw_tx(self, coin, tx_hex): | |
| return self.kdf_proxy({"mmrpc": "2.0", "method": "send_raw_transaction", "params": {"coin": coin, "tx_hex": tx_hex}, "id": 0}) | |
| # https://developers.komodoplatform.com/basic-docs/atomicdex-api-legacy/validateaddress.html | |
| def validate_address(self, coin, address): | |
| return self.kdf_proxy({"mmrpc": "2.0", "method": "validateaddress", "params": {"coin": coin, "address": address}, "id": 0}) |
🤖 Prompt for AI Agents
In models.py around lines 323 to 332, the payloads for send_raw_tx and
validate_address call kdf_proxy with legacy flat fields rather than the mmrpc v2
envelope; update both to send a JSON-RPC 2.0 envelope: include "jsonrpc": "2.0",
an "id" (reuse the same id pattern used elsewhere in this file), set "method" to
the RPC name and move the current coin/tx_hex or coin/address into a "params"
object; follow the same envelope structure used by other v2 calls in this module
to ensure compatibility.
| time.sleep(0.1) | ||
| dex.disable_coin(coin) | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
disable_coin call likely needs a proper params envelope.
Align with other KDF calls to avoid RPC schema mismatch.
- dex.disable_coin(coin)
+ dex.kdf_proxy({
+ "mmrpc": "2.0",
+ "method": "disable_coin",
+ "params": { "ticker": coin },
+ "id": 0
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| time.sleep(0.1) | |
| dex.disable_coin(coin) | |
| time.sleep(0.1) | |
| dex.kdf_proxy({ | |
| "mmrpc": "2.0", | |
| "method": "disable_coin", | |
| "params": { "ticker": coin }, | |
| "id": 0 | |
| }) |
🤖 Prompt for AI Agents
In scan_seeds.py around lines 47 to 49, the call dex.disable_coin(coin) sends
the raw value instead of the RPC "params" envelope used by other KDF calls; wrap
the argument in the same RPC envelope to avoid schema mismatch — e.g., change
the call to pass a params object (match the pattern used elsewhere, such as
dex.disable_coin({'params': [coin]}) or the exact named params used in your
codebase) so the RPC receives a properly structured payload.
| if kdf_seed_phrase not in seed_phrases["seed_phrases"]: | ||
| seed_phrases["seed_phrases"].update({kdf_seed_phrase: {}}) | ||
| with open(SEEDS_FILE, "w", encoding="utf-8") as f: |
There was a problem hiding this comment.
Possible NameError: kdf_seed_phrase may be undefined.
Initialize it and guard use when missing.
- if kdf_seed_phrase not in seed_phrases["seed_phrases"]:
+ if 'kdf_seed_phrase' in locals() and kdf_seed_phrase and kdf_seed_phrase not in seed_phrases.get("seed_phrases", {}):
seed_phrases["seed_phrases"].update({kdf_seed_phrase: {}})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scan_seeds.py around lines 79 to 81, kdf_seed_phrase may be referenced before
being defined; ensure it is initialized and guarded: compute or assign
kdf_seed_phrase before this if (e.g., derive from current seed entry or set to
None), then check for None or falsy value and skip/update accordingly (avoid
NameError), and only call seed_phrases["seed_phrases"].update(...) when
kdf_seed_phrase is valid; if derivation can fail, catch the error and handle it
(log/continue) rather than using an undefined variable.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
models.py (5)
84-109: kdf_proxy is broken (undefined r, wrong v2 routing, inconsistent return type)Route v2 payloads via api.rpc(..., v2=True), always return JSON dict, and fix userpass check. This currently raises NameError (r), and many callers expect a dict.
- def kdf_proxy(self, params): - try: - resp = self.api.rpc(params["method"], params) - try: - pass - # resp = r.json() - except Exception as e: - print(f"Error getting response: {e}") - if "NewConnectionError" in str(r): - self.start() - r = self.api.rpc(params["method"], params) - resp = r.json() - else: - print(f"Response: {r}") - resp = r - except requests.exceptions.RequestException as e: - self.start() - resp = self.api.rpc(params["method"], params) - # resp = r.json() - if "error" in resp: - if resp["error"].find("Userpass is invalid"): + def kdf_proxy(self, payload): + """ + Accepts either: + - v2 envelope: {"mmrpc":"2.0","method":"X","params":{...},"id":0} + - legacy shape: {"method":"X", ...} + Always returns parsed JSON (dict). + """ + def _call(method, pargs, v2=False): + resp = self.api.rpc(method, pargs, v2=v2) + return resp.json() if hasattr(resp, "json") else resp + try: + if isinstance(payload, dict) and payload.get("mmrpc") == "2.0": + method = payload["method"] + params = payload.get("params", {}) or {} + resp = _call(method, params, v2=True) + else: + method = payload["method"] + params = {k: v for k, v in payload.items() if k != "method"} + resp = _call(method, params, v2=False) + except requests.exceptions.RequestException: + self.start() + # retry once + if isinstance(payload, dict) and payload.get("mmrpc") == "2.0": + resp = _call(payload["method"], payload.get("params", {}) or {}, v2=True) + else: + resp = _call(payload["method"], {k: v for k, v in payload.items() if k != "method"}, v2=False) + if isinstance(resp, dict) and "error" in resp: + if "Userpass is invalid" in str(resp.get("error", "")): error_print( "MM2 is rejecting your rpc_password. Please check you are not running additional instances of mm2 on the same port, and confirm your rpc_password conforms to constraints in https://developers.komodoplatform.com/basic-docs/atomicdex/atomicdex-setup/configure-mm2-json.html#mm2-json" ) sys.exit() return resp
115-140: Activation response parsing bug and error checkBalance is under result; also use substring test for “already initialized”.
- if "result" in resp: - if "balance" in resp["result"]: - status_print(f"{coin} activated. Balance: {resp['balance']}") + if "result" in resp: + if "balance" in resp["result"]: + status_print(f"{coin} activated. Balance: {resp['result']['balance']}") @@ - elif "error" in resp: - if resp["error"].find("already initialized") >= 0: + elif "error" in resp: + if "already initialized" in str(resp.get("error", "")): status_print(f"{coin} was already activated.")Consider persisting task_ids keyed by coin for non-Z coins to avoid "unknown".
206-220: get_total_balance_usd: handle v2 shapeAccess fields from resp["result"] to avoid false negatives.
- for coin in self.enabled_coins: - resp = self.get_balance(coin) - if "balance" in resp: - price = get_price(coin, current_prices) - coin_balance = float(resp["balance"]) + float( - resp["unspendable_balance"] - ) + for coin in self.enabled_coins: + resp = self.get_balance(coin) + res = resp.get("result", resp) if isinstance(resp, dict) else {} + if {"balance", "unspendable_balance"} <= set(res.keys()): + price = get_price(coin, current_prices) + coin_balance = float(res["balance"]) + float(res["unspendable_balance"])
285-294: cancel_all_orders: add v2 envelopeLegacy payloads won’t work with KDF v2.
if coin: - params = { - "method": "cancel_all_orders", - "cancel_by": {"type": "Coin", "data": {"ticker": coin}}, - } - return self.kdf_proxy(params) - params = {"method": "cancel_all_orders", "cancel_by": {"type": "All"}} - return self.kdf_proxy(params) + return self.kdf_proxy({ + "mmrpc": "2.0", + "method": "cancel_all_orders", + "params": {"cancel_by": {"type": "Coin", "data": {"ticker": coin}}}, + "id": 0 + }) + return self.kdf_proxy({ + "mmrpc": "2.0", + "method": "cancel_all_orders", + "params": {"cancel_by": {"type": "All"}}, + "id": 0 + })
1248-1257: Adjust send_raw_tx handling to v2 resultAfter converting send_raw_tx to v2, read tx_hash from result.
- resp = self.dex.withdraw(coin, amount, address) + resp = self.dex.withdraw(coin, amount, address) if "error" in resp: error_print(resp) elif "result" in resp: if "tx_hex" in resp["result"]: - send_resp = self.dex.send_raw_tx(coin, resp["result"]["tx_hex"]) - if "tx_hash" in send_resp: + send_resp = self.dex.send_raw_tx(coin, resp["result"]["tx_hex"]) + tx_hash = send_resp.get("result", {}).get("tx_hash") + if tx_hash: success_print( - f"{amount} {coin} sent to {address}. TXID: {send_resp['tx_hash']}" + f"{amount} {coin} sent to {address}. TXID: {tx_hash}" ) else: error_print(send_resp) else: error_print(resp) else: error_print(resp)
♻️ Duplicate comments (5)
models.py (5)
159-160: Quit/stop must use v2 envelope via kdf_proxyLegacy flat payload will be rejected by KDF v2. This was flagged earlier and still applies.
- resp = self.kdf_proxy({"method": "stop"}) + resp = self.kdf_proxy({"mmrpc": "2.0", "method": "stop", "params": {}, "id": 0})
282-283: my_recent_swaps: missing v2 envelopeStill using legacy shape; align to v2.
- return self.kdf_proxy({"method": "my_recent_swaps", "limit": limit}) + return self.kdf_proxy({"mmrpc": "2.0", "method": "my_recent_swaps", "params": {"limit": limit}, "id": 0})
296-302: get_balance/disable_coin: add v2 envelopeBoth are still flat payloads.
- return self.kdf_proxy({"method": "my_balance", "coin": coin}) + return self.kdf_proxy({"mmrpc": "2.0", "method": "my_balance", "params": {"coin": coin}, "id": 0}) @@ - return self.kdf_proxy({"method": "disable_coin", "coin": coin}) + return self.kdf_proxy({"mmrpc": "2.0", "method": "disable_coin", "params": {"coin": coin}, "id": 0})
326-334: send_raw_tx/validate_address need v2; is_address_valid assumes v2 resultUpdate envelopes and keep is_address_valid as-is.
- return self.kdf_proxy( - {"method": "send_raw_transaction", "coin": coin, "tx_hex": tx_hex} - ) + return self.kdf_proxy({"mmrpc": "2.0", "method": "send_raw_transaction", "params": {"coin": coin, "tx_hex": tx_hex}, "id": 0}) @@ - return self.kdf_proxy( - {"method": "validateaddress", "coin": coin, "address": address} - ) + return self.kdf_proxy({"mmrpc": "2.0", "method": "validateaddress", "params": {"coin": coin, "address": address}, "id": 0})Also applies to: 337-339
1213-1214: view_swaps: set v2=TrueEnsure v2 routing.
- self.table.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) + self.table.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}, v2=True).json())
🧹 Nitpick comments (5)
models.py (5)
56-60: Harden is_running: avoid substring check on unknown typeGuard against non-dict returns and exceptions.
- is_running = self.api.version - if "error" in is_running: - return False - else: - return True + try: + v = self.api.version + if isinstance(v, dict) and "error" in v: + return False + return bool(v) + except Exception: + return False
64-73: Start: improve robustness (bin fetch, log path, subprocess I/O)
- Ensure log dir exists and open file safely; keep v2 env var set; preserve current behavior.
if not self.is_running: os.environ["MM_CONF_PATH"] = self.kdf_config if not os.path.isfile(KDFBIN): error_print(f"kdf binary not found in {KDFBIN}!") get_kdf() - kdf_output = open(self.kdf_log, "w+") + os.makedirs(os.path.dirname(self.kdf_log), exist_ok=True) + kdf_output = open(self.kdf_log, "a", buffering=1) subprocess.Popen( [KDFBIN], stdout=kdf_output, stderr=kdf_output, universal_newlines=True, preexec_fn=preexec, )Please confirm KDFBIN is always an absolute path in const to avoid cwd surprises.
Also applies to: 69-75, 80-81
165-204: status(): narrow broad excepts and avoid unused vars
- Replace blind excepts; ignore unused maker/taker.
- try: - orders_obj = self.api.orders - maker_orders, taker_orders, order_count = get_order_count(orders_obj) - except Exception: - maker_orders, taker_orders, order_count = {}, {}, 0 + try: + orders_obj = self.api.orders + _, _, order_count = get_order_count(orders_obj) + except Exception: + order_count = 0
913-914: Validate entered BIP39 mnemonicAdd a quick validity check to prevent malformed seeds.
- new_seed = color_input("[E]nter BIP39 mnemonic manually or [G]enerate one? [E/G]: ") + new_seed = color_input("[E]nter BIP39 mnemonic manually or [G]enerate one? [E/G]: ") @@ - if new_seed in ["E", "e"]: - passphrase = color_input("Enter a seed phrase: ") + if new_seed in ["E", "e"]: + passphrase = color_input("Enter a seed phrase: ") + if not mnemonic.Mnemonic('english').check(passphrase): + error_print("Invalid BIP39 mnemonic; please try again.") + return self.init_MM2_json()
1233-1242: Remove f-prefix from constant stringsFixes Ruff F541.
- address = color_input(f"Enter the destination address: ") + address = color_input("Enter the destination address: ") @@ - address = color_input(f"Enter the destination address: ") + address = color_input("Enter the destination address: ")Also applies to: 1243-1246
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
makerbot.py(2 hunks)models.py(20 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- makerbot.py
🧰 Additional context used
🧬 Code graph analysis (1)
models.py (2)
helpers.py (15)
get_kdf(108-147)get_price(34-42)get_order_count(159-173)sec_to_hms(15-19)generate_rpc_pass(238-253)get_prices(230-236)compute_kdf_version_suffix(292-309)center_visible(281-289)get_seednodes_list(318-349)wait_continue(227-228)error_print(223-224)preexec(155-156)success_print(219-220)color_input(203-204)status_print(215-216)activation.py (2)
build_activate_command(147-230)load(17-40)
🪛 Ruff (0.12.2)
models.py
69-69: subprocess call: check for execution of untrusted input
(S603)
90-90: Do not catch blind exception: Exception
(BLE001)
92-92: Undefined name r
(F821)
99-99: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
166-166: Do not catch blind exception: Exception
(BLE001)
172-172: Do not catch blind exception: Exception
(BLE001)
173-173: Local variable maker_orders is assigned to but never used
Remove assignment to unused variable maker_orders
(F841)
173-173: Local variable taker_orders is assigned to but never used
Remove assignment to unused variable taker_orders
(F841)
179-179: Do not catch blind exception: Exception
(BLE001)
185-185: Do not catch blind exception: Exception
(BLE001)
190-190: Do not catch blind exception: Exception
(BLE001)
196-196: Do not catch blind exception: Exception
(BLE001)
231-231: Do not catch blind exception: Exception
(BLE001)
757-757: Undefined name dex
(F821)
1243-1243: f-string without any placeholders
Remove extraneous f prefix
(F541)
1246-1246: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (12)
models.py (12)
144-145: Use of activation builder looks goodDelegation keeps models.py slimmer.
147-153: get_task: LGTMCorrect v2 envelope and routing through kdf_proxy.
306-322: withdraw (v2) looks correctMAX path and amount path use the right v2 envelope.
351-357: MakerBot init: LGTMPassing kdf_config through Dex improves testability.
358-367: activate_bot_coins: LGTMMinimal diff and clearer messaging.
369-376: Makerbot start: v2 flag is correctGood use of v2=True here.
901-911: KDF config defaults: LGTMHD enabled and seednodes from repo are sensible defaults.
939-957: Config migration: LGTMIdempotent updates and minimal writes look good.
1150-1155: TUI paths: LGTMClear startup info for users.
1206-1211: Docs screen: LGTMGood addition with wait prompt.
1219-1225: Withdraw: good UX on enabled coinsMinor nit: message builds fine.
Also applies to: 1223-1225
1272-1277: Exit: depends on quit fixThis will work once quit() sends a v2 envelope.
After applying the quit() diff above, please test exit flow to ensure KDF stops cleanly.
| error_print(f"kdf binary not found in {KDFBIN}!") | ||
| get_kdf() | ||
| kdf_output = open(self.kdf_log, "w+") | ||
| subprocess.Popen( |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Address Ruff findings in one pass
- Undefined name r; remove dead code in kdf_proxy (fixed above).
- Blind excepts: narrow or log (partially addressed above).
- Extraneous f-strings at Lines 1243/1246 (fixed above).
- Undefined name dex at Line 757 (fixed above).
Run:
Also applies to: 92-92, 99-99, 166-166, 172-173, 179-179, 185-185, 190-190, 196-196, 231-231, 757-757, 1243-1246
🏁 Script executed:
#!/bin/bash
ruff check models.py || true
rg -nP 'kdf_proxy\(\{\s*"method"' models.py
rg -nP 'api\.rpc\(\s*"my_recent_swaps"' models.py -C1Length of output: 20491
Address Ruff findings in one pass — fix these remaining issues
Ruff reports 36 errors (9 auto-fixable). Actionable items:
- Undefined name
r— line 92: replacestr(r)with the actual exception/response variable (use the caughte) or remove dead reference. - Undefined name
dex— line 757: useself.dex(e.g. callself.dex.api.rpc("my_recent_swaps", {"limit":1000}).json()) and pass result toswaps_summary. - subprocess security S603 — line 69: validate/whitelist
KDFBIN(useshutil.which, absolute path, or ensure it’s constant/not user-controlled); avoid shell injection. - Blind/bare excepts — lines 88–92, 166, 172–173, 179, 185, 190, 196, 231, 344–346, 771–774, 778–781, 1188–1191: catch specific exceptions (e.g., requests.exceptions.RequestException, ValueError, FileNotFoundError, json.JSONDecodeError) and log the exception (
except Exception as e:then loge) instead of swallowing. - Bare
except(E722) — lines 346, 773, 780: replace with specific exception types. - Extraneous f-strings (F541) — lines 387, 476, 723–727, 1187–1191, 1243, 1246: remove
fprefix from literal strings. - Mutable default args (B006) — line 1110: replace
default=list()/valid_options=set()withNoneand initialize inside the function. - Unused args/vars (ARG002 / F841) — line 488 (
limit), lines 99/1190/173/790 (unusede,maker_orders,taker_orders,use_bidirectional_threshold): remove or use them. - TRY301 / TRY300 (raising inside try) — lines ~1072, 1096, 1122, 1126: move
raiseout of the try or restructure to avoid catching your own raised exceptions. - Run:
ruff --fix models.py(applies 9 fixes) then re-runruff check models.pyand run tests / smoke checks.
🧰 Tools
🪛 Ruff (0.12.2)
69-69: subprocess call: check for execution of untrusted input
(S603)
🤖 Prompt for AI Agents
In models.py (review span roughly lines 69–1246) address the Ruff findings by:
1) For the subprocess at line 69 validate/whitelist KDFBIN using shutil.which or
use an absolute constant path and avoid shell=True to prevent S603; 2) Replace
undefined names (e.g., change str(r) at ~line 92 to use the caught exception
variable e, and use self.dex instead of dex at ~line 757) and remove dead
references; 3) Replace all bare/blank excepts and overly broad excepts (lines
noted in the comment) with specific exception types
(requests.exceptions.RequestException, ValueError, FileNotFoundError,
json.JSONDecodeError, etc.) and log the exception via except Exception as e:
logger.exception(e) or processLogger.error(e); 4) Remove extraneous f-strings on
plain literals and unused variables/args (drop unused params like limit or use
them), change mutable default args to None and initialize inside the function,
and restructure try/raise blocks to avoid catching your own raises (move raise
out of try); finally run ruff --fix models.py, re-run ruff check models.py and
run tests/smoke checks to verify.
| coins_list = self.dex.enabled_coins | ||
| sleep_message(msg, 10) | ||
| self.balances(coins_list) | ||
| sleep_message(msg, 10) | ||
| self.orders(Dex().api.orders) | ||
| sleep_message(msg, 10) | ||
| self.swaps_summary( | ||
| Dex().api.rpc(dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) | ||
| self.dex.api.rpc(dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) | ||
| ) |
There was a problem hiding this comment.
Fix undefined name and double-rpc in loop_views
Uses undefined dex and nests rpc calls incorrectly. Call once with v2 and pass JSON.
- self.swaps_summary(
- self.dex.api.rpc(dex.api.rpc("my_recent_swaps", {"limit": 1000}).json())
- )
+ self.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}, v2=True).json())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| coins_list = self.dex.enabled_coins | |
| sleep_message(msg, 10) | |
| self.balances(coins_list) | |
| sleep_message(msg, 10) | |
| self.orders(Dex().api.orders) | |
| sleep_message(msg, 10) | |
| self.swaps_summary( | |
| Dex().api.rpc(dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) | |
| self.dex.api.rpc(dex.api.rpc("my_recent_swaps", {"limit": 1000}).json()) | |
| ) | |
| coins_list = self.dex.enabled_coins | |
| sleep_message(msg, 10) | |
| self.balances(coins_list) | |
| sleep_message(msg, 10) | |
| self.orders(Dex().api.orders) | |
| sleep_message(msg, 10) | |
| self.swaps_summary(self.dex.api.rpc("my_recent_swaps", {"limit": 1000}, v2=True).json()) |
🧰 Tools
🪛 Ruff (0.12.2)
757-757: Undefined name dex
(F821)
🤖 Prompt for AI Agents
In models.py around lines 750-758, the loop_views code uses an undefined local
name dex and incorrectly nests rpc calls; change Dex() usage to the instance
self.dex (use self.dex.api.orders instead of Dex().api.orders) and call the RPC
only once with v2, e.g. call self.dex.api.rpc("my_recent_swaps", {"limit":
1000}, version="v2").json() and pass that JSON result into self.swaps_summary;
remove the nested rpc(...) wrapper so you pass the already-parsed JSON and
ensure all API calls use the self.dex instance.
| self.table.balances(self.dex.enabled_coins_list) | ||
|
|
There was a problem hiding this comment.
Fix attribute: enabled_coins_list → enabled_coins
Property doesn’t exist after refactor.
- self.table.balances(self.dex.enabled_coins_list)
+ self.table.balances(self.dex.enabled_coins)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self.table.balances(self.dex.enabled_coins_list) | |
| self.table.balances(self.dex.enabled_coins) |
🤖 Prompt for AI Agents
In models.py around lines 1201-1202, the call uses the removed property
enabled_coins_list; update it to use the refactored property enabled_coins
instead by replacing self.dex.enabled_coins_list with self.dex.enabled_coins so
the balances method receives the correct attribute.
1 participant
mm2-> `kdfTODO:
Summary by CodeRabbit
New Features
Refactor
Chores