From 6ea32ba45fad71481320ef8d1a4c0dd311989592 Mon Sep 17 00:00:00 2001 From: DanielDerefaka Date: Mon, 22 Dec 2025 03:51:22 +0100 Subject: [PATCH 1/2] feat: Add standardized JSON output utilities (#635) This PR introduces a centralized json_utils module to standardize JSON output across all btcli commands, addressing the inconsistencies noted in issue #635. Changes: - Add `bittensor_cli/src/bittensor/json_utils.py` with standardized helpers: - `json_response()` - Base response formatter - `json_success()` - Success response helper - `json_error()` - Error response helper - `json_transaction()` - Transaction response helper - `serialize_balance()` - Consistent Balance serialization - Update `wallets.py` with sample usage (3 functions updated as POC) - Add comprehensive unit tests (21 tests) for schema compliance Standard Response Format: ```json { "success": bool, "data": {...}, // Optional: command-specific data "error": str // Optional: error message } ``` This establishes the foundation for full migration of all commands to use consistent JSON schemas. Additional commands can be migrated incrementally. Closes #635 --- bittensor_cli/src/bittensor/json_utils.py | 174 +++++++++++++++ bittensor_cli/src/commands/wallets.py | 91 +++----- tests/unit_tests/test_json_utils.py | 258 ++++++++++++++++++++++ 3 files changed, 463 insertions(+), 60 deletions(-) create mode 100644 bittensor_cli/src/bittensor/json_utils.py create mode 100644 tests/unit_tests/test_json_utils.py diff --git a/bittensor_cli/src/bittensor/json_utils.py b/bittensor_cli/src/bittensor/json_utils.py new file mode 100644 index 000000000..a58dd18dc --- /dev/null +++ b/bittensor_cli/src/bittensor/json_utils.py @@ -0,0 +1,174 @@ +""" +Standardized JSON output utilities for btcli. + +This module provides consistent JSON response formatting across all btcli commands. +All JSON outputs should use these utilities to ensure schema compliance. + +Standard Response Format: +{ + "success": bool, # Required: Whether the operation succeeded + "data": {...}, # Optional: Command-specific response data + "error": str # Optional: Error message if success=False +} + +For transaction responses, data should include: +{ + "extrinsic_hash": str, # The transaction hash + "block_hash": str # The block containing the transaction +} +""" + +import json +from typing import Any, Optional, Union +from rich.console import Console + +# JSON console for outputting JSON responses +json_console = Console() + + +def json_response( + success: bool, + data: Optional[Any] = None, + error: Optional[str] = None, +) -> str: + """ + Create a standardized JSON response string. + + Args: + success: Whether the operation succeeded + data: Optional response data (dict, list, or primitive) + error: Optional error message (typically used when success=False) + + Returns: + JSON string with standardized format + + Examples: + >>> json_response(True, {"balance": 100.5}) + '{"success": true, "data": {"balance": 100.5}}' + + >>> json_response(False, error="Wallet not found") + '{"success": false, "error": "Wallet not found"}' + """ + response: dict[str, Any] = {"success": success} + + if data is not None: + response["data"] = data + + if error is not None: + response["error"] = error + + return json.dumps(response) + + +def json_success(data: Any) -> str: + """ + Create a successful JSON response. + + Args: + data: Response data to include + + Returns: + JSON string with success=True and the provided data + """ + return json_response(success=True, data=data) + + +def json_error(error: str, data: Optional[Any] = None) -> str: + """ + Create an error JSON response. + + Args: + error: Error message describing what went wrong + data: Optional additional context data + + Returns: + JSON string with success=False and error message + """ + return json_response(success=False, data=data, error=error) + + +def json_transaction( + success: bool, + extrinsic_hash: Optional[str] = None, + block_hash: Optional[str] = None, + error: Optional[str] = None, + **extra_data: Any, +) -> str: + """ + Create a standardized transaction response. + + Args: + success: Whether the transaction succeeded + extrinsic_hash: The transaction/extrinsic hash + block_hash: The block hash containing the transaction + error: Error message if transaction failed + **extra_data: Additional transaction-specific data + + Returns: + JSON string with transaction details + """ + data: dict[str, Any] = {} + + if extrinsic_hash is not None: + data["extrinsic_hash"] = extrinsic_hash + + if block_hash is not None: + data["block_hash"] = block_hash + + # Add any extra data + data.update(extra_data) + + return json_response(success=success, data=data if data else None, error=error) + + +def print_json(response: str) -> None: + """ + Print a JSON response to the console. + + Args: + response: JSON string to print + """ + json_console.print(response) + + +def print_json_success(data: Any) -> None: + """ + Print a successful JSON response. + + Args: + data: Response data to include + """ + print_json(json_success(data)) + + +def print_json_error(error: str, data: Optional[Any] = None) -> None: + """ + Print an error JSON response. + + Args: + error: Error message + data: Optional additional context + """ + print_json(json_error(error, data)) + + +def serialize_balance(balance: Any) -> dict[str, Union[int, float]]: + """ + Serialize a Balance object to a consistent dictionary format. + + Args: + balance: A Balance object or numeric value + + Returns: + Dictionary with 'rao' (int) and 'tao' (float) keys + """ + if hasattr(balance, "rao") and hasattr(balance, "tao"): + return {"rao": int(balance.rao), "tao": float(balance.tao)} + elif isinstance(balance, (int, float)): + # Assume it's already in tao if float, rao if int + if isinstance(balance, float): + return {"rao": int(balance * 1e9), "tao": balance} + else: + return {"rao": balance, "tao": balance / 1e9} + else: + return {"rao": 0, "tao": 0.0} diff --git a/bittensor_cli/src/commands/wallets.py b/bittensor_cli/src/commands/wallets.py index 96c812be5..3f8e90f3f 100644 --- a/bittensor_cli/src/commands/wallets.py +++ b/bittensor_cli/src/commands/wallets.py @@ -54,6 +54,11 @@ get_hotkey_pub_ss58, print_extrinsic_id, ) +from bittensor_cli.src.bittensor.json_utils import ( + json_success, + json_error, + print_json, +) class SortByBalance(Enum): @@ -184,33 +189,21 @@ async def regen_coldkey( f"coldkey ss58: ({new_wallet.coldkeypub.ss58_address})", ) if json_output: - json_console.print( - json.dumps( - { - "success": True, - "data": { - "name": new_wallet.name, - "path": new_wallet.path, - "hotkey": new_wallet.hotkey_str, - "hotkey_ss58": get_hotkey_pub_ss58(new_wallet), - "coldkey_ss58": new_wallet.coldkeypub.ss58_address, - }, - "error": "", - } - ) - ) + print_json(json_success({ + "name": new_wallet.name, + "path": new_wallet.path, + "hotkey": new_wallet.hotkey_str, + "hotkey_ss58": get_hotkey_pub_ss58(new_wallet), + "coldkey_ss58": new_wallet.coldkeypub.ss58_address, + })) except ValueError: print_error("Mnemonic phrase is invalid") if json_output: - json_console.print( - '{"success": false, "error": "Mnemonic phrase is invalid", "data": null}' - ) + print_json(json_error("Mnemonic phrase is invalid")) except KeyFileError: print_error("KeyFileError: File is not writable") if json_output: - json_console.print( - '{"success": false, "error": "Keyfile is not writable", "data": null}' - ) + print_json(json_error("Keyfile is not writable")) async def regen_coldkey_pub( @@ -234,27 +227,17 @@ async def regen_coldkey_pub( f"coldkey ss58: ({new_coldkeypub.coldkeypub.ss58_address})", ) if json_output: - json_console.print( - json.dumps( - { - "success": True, - "data": { - "name": new_coldkeypub.name, - "path": new_coldkeypub.path, - "hotkey": new_coldkeypub.hotkey_str, - "hotkey_ss58": get_hotkey_pub_ss58(new_coldkeypub), - "coldkey_ss58": new_coldkeypub.coldkeypub.ss58_address, - }, - "error": "", - } - ) - ) + print_json(json_success({ + "name": new_coldkeypub.name, + "path": new_coldkeypub.path, + "hotkey": new_coldkeypub.hotkey_str, + "hotkey_ss58": get_hotkey_pub_ss58(new_coldkeypub), + "coldkey_ss58": new_coldkeypub.coldkeypub.ss58_address, + })) except KeyFileError: print_error("KeyFileError: File is not writable") if json_output: - json_console.print( - '{"success": false, "error": "Keyfile is not writable", "data": null}' - ) + print_json(json_error("Keyfile is not writable")) async def regen_hotkey( @@ -291,33 +274,21 @@ async def regen_hotkey( f"hotkey ss58: ({new_hotkey_.hotkeypub.ss58_address})", ) if json_output: - json_console.print( - json.dumps( - { - "success": True, - "data": { - "name": new_hotkey_.name, - "path": new_hotkey_.path, - "hotkey": new_hotkey_.hotkey_str, - "hotkey_ss58": new_hotkey_.hotkeypub.ss58_address, - "coldkey_ss58": new_hotkey_.coldkeypub.ss58_address, - }, - "error": "", - } - ) - ) + print_json(json_success({ + "name": new_hotkey_.name, + "path": new_hotkey_.path, + "hotkey": new_hotkey_.hotkey_str, + "hotkey_ss58": new_hotkey_.hotkeypub.ss58_address, + "coldkey_ss58": new_hotkey_.coldkeypub.ss58_address, + })) except ValueError: print_error("Mnemonic phrase is invalid") if json_output: - json_console.print( - '{"success": false, "error": "Mnemonic phrase is invalid", "data": null}' - ) + print_json(json_error("Mnemonic phrase is invalid")) except KeyFileError: print_error("KeyFileError: File is not writable") if json_output: - json_console.print( - '{"success": false, "error": "Keyfile is not writable", "data": null}' - ) + print_json(json_error("Keyfile is not writable")) async def regen_hotkey_pub( diff --git a/tests/unit_tests/test_json_utils.py b/tests/unit_tests/test_json_utils.py new file mode 100644 index 000000000..216244c79 --- /dev/null +++ b/tests/unit_tests/test_json_utils.py @@ -0,0 +1,258 @@ +""" +Unit tests for JSON output utilities. + +Tests the standardized JSON response formatting used across btcli commands. +""" + +import json +import pytest +from io import StringIO +from unittest.mock import patch + +from bittensor_cli.src.bittensor.json_utils import ( + json_response, + json_success, + json_error, + json_transaction, + serialize_balance, +) + + +class TestJsonResponse: + """Tests for the json_response function.""" + + def test_success_with_data(self): + """Test successful response with data.""" + result = json_response(success=True, data={"key": "value"}) + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"] == {"key": "value"} + assert "error" not in parsed + + def test_success_without_data(self): + """Test successful response without data.""" + result = json_response(success=True) + parsed = json.loads(result) + + assert parsed["success"] is True + assert "data" not in parsed + assert "error" not in parsed + + def test_error_response(self): + """Test error response.""" + result = json_response(success=False, error="Something went wrong") + parsed = json.loads(result) + + assert parsed["success"] is False + assert parsed["error"] == "Something went wrong" + + def test_error_with_data(self): + """Test error response with additional data.""" + result = json_response( + success=False, + data={"partial": "data"}, + error="Partial failure" + ) + parsed = json.loads(result) + + assert parsed["success"] is False + assert parsed["data"] == {"partial": "data"} + assert parsed["error"] == "Partial failure" + + def test_nested_data(self): + """Test response with nested data structures.""" + data = { + "wallet": { + "name": "test", + "hotkeys": ["hk1", "hk2"], + "balance": {"rao": 1000000000, "tao": 1.0} + } + } + result = json_response(success=True, data=data) + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"]["wallet"]["name"] == "test" + assert len(parsed["data"]["wallet"]["hotkeys"]) == 2 + + +class TestJsonSuccess: + """Tests for the json_success helper.""" + + def test_simple_data(self): + """Test success with simple data.""" + result = json_success({"status": "ok"}) + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"]["status"] == "ok" + + def test_list_data(self): + """Test success with list data.""" + result = json_success([1, 2, 3]) + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"] == [1, 2, 3] + + def test_primitive_data(self): + """Test success with primitive data.""" + result = json_success("simple string") + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"] == "simple string" + + +class TestJsonError: + """Tests for the json_error helper.""" + + def test_simple_error(self): + """Test simple error message.""" + result = json_error("Connection failed") + parsed = json.loads(result) + + assert parsed["success"] is False + assert parsed["error"] == "Connection failed" + assert "data" not in parsed + + def test_error_with_context(self): + """Test error with additional context data.""" + result = json_error("Validation failed", data={"field": "amount"}) + parsed = json.loads(result) + + assert parsed["success"] is False + assert parsed["error"] == "Validation failed" + assert parsed["data"]["field"] == "amount" + + +class TestJsonTransaction: + """Tests for the json_transaction helper.""" + + def test_successful_transaction(self): + """Test successful transaction response.""" + result = json_transaction( + success=True, + extrinsic_hash="0x123abc", + block_hash="0x456def" + ) + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"]["extrinsic_hash"] == "0x123abc" + assert parsed["data"]["block_hash"] == "0x456def" + + def test_failed_transaction(self): + """Test failed transaction response.""" + result = json_transaction( + success=False, + error="Insufficient balance" + ) + parsed = json.loads(result) + + assert parsed["success"] is False + assert parsed["error"] == "Insufficient balance" + + def test_transaction_with_extra_data(self): + """Test transaction with extra data fields.""" + result = json_transaction( + success=True, + extrinsic_hash="0x123", + amount=100.5, + recipient="5xyz..." + ) + parsed = json.loads(result) + + assert parsed["success"] is True + assert parsed["data"]["extrinsic_hash"] == "0x123" + assert parsed["data"]["amount"] == 100.5 + assert parsed["data"]["recipient"] == "5xyz..." + + +class TestSerializeBalance: + """Tests for balance serialization.""" + + def test_balance_object(self): + """Test serializing a Balance-like object.""" + class MockBalance: + rao = 1000000000 + tao = 1.0 + + result = serialize_balance(MockBalance()) + + assert result["rao"] == 1000000000 + assert result["tao"] == 1.0 + + def test_float_value(self): + """Test serializing a float (assumes TAO).""" + result = serialize_balance(2.5) + + assert result["tao"] == 2.5 + assert result["rao"] == 2500000000 + + def test_int_value(self): + """Test serializing an int (assumes RAO).""" + result = serialize_balance(5000000000) + + assert result["rao"] == 5000000000 + assert result["tao"] == 5.0 + + def test_unknown_type(self): + """Test serializing unknown type returns zeros.""" + result = serialize_balance("invalid") + + assert result["rao"] == 0 + assert result["tao"] == 0.0 + + +class TestJsonOutputConsistency: + """Tests to verify JSON output schema consistency.""" + + def test_success_always_has_success_field(self): + """Verify success responses always include 'success' field.""" + responses = [ + json_success({}), + json_success([]), + json_success("test"), + json_success(None), + ] + + for response in responses: + parsed = json.loads(response) + assert "success" in parsed + assert parsed["success"] is True + + def test_error_always_has_success_and_error_fields(self): + """Verify error responses always include required fields.""" + responses = [ + json_error("error1"), + json_error("error2", data={}), + ] + + for response in responses: + parsed = json.loads(response) + assert "success" in parsed + assert "error" in parsed + assert parsed["success"] is False + + def test_no_null_data_in_success(self): + """Verify success responses don't include null data.""" + result = json_success({"key": "value"}) + parsed = json.loads(result) + + # Data should be present, not null + assert parsed.get("data") is not None + + def test_json_is_valid(self): + """Verify all outputs are valid JSON.""" + test_cases = [ + json_response(True, {"test": "data"}), + json_success({"nested": {"deep": True}}), + json_error("test error"), + json_transaction(True, "0x123", "0x456"), + ] + + for output in test_cases: + # Should not raise + json.loads(output) From 10b292d639afc9a6b752463eff24452a418bbadd Mon Sep 17 00:00:00 2001 From: DanielDerefaka Date: Mon, 22 Dec 2025 16:31:22 +0100 Subject: [PATCH 2/2] feat: Expand JSON utils with TransactionResult classes and extrinsic_identifier - Add TransactionResult and MultiTransactionResult classes for consistent transaction responses across all commands - Add print_transaction_response() helper function - Update schema to use {success, message, extrinsic_identifier} format - Migrate wallets.py: transfer, swap_hotkey, set_id - Migrate sudo.py: trim command - Migrate stake/add.py and stake/remove.py - Migrate liquidity.py: add_liquidity, remove_liquidity, modify_liquidity - Update tests for new transaction response utilities (25 tests passing) This addresses feedback from @thewhaleking on PR #781 to apply standardized JSON output to all json_console usages with extrinsic_identifier support. Closes #635 --- bittensor_cli/src/bittensor/json_utils.py | 171 +++++++++++++----- .../src/commands/liquidity/liquidity.py | 39 ++-- bittensor_cli/src/commands/stake/add.py | 13 +- bittensor_cli/src/commands/stake/remove.py | 5 +- bittensor_cli/src/commands/sudo.py | 15 +- bittensor_cli/src/commands/wallets.py | 19 +- tests/unit_tests/test_json_utils.py | 123 +++++++++---- 7 files changed, 251 insertions(+), 134 deletions(-) diff --git a/bittensor_cli/src/bittensor/json_utils.py b/bittensor_cli/src/bittensor/json_utils.py index a58dd18dc..e094fa88b 100644 --- a/bittensor_cli/src/bittensor/json_utils.py +++ b/bittensor_cli/src/bittensor/json_utils.py @@ -4,17 +4,18 @@ This module provides consistent JSON response formatting across all btcli commands. All JSON outputs should use these utilities to ensure schema compliance. -Standard Response Format: +Standard Transaction Response Format: { - "success": bool, # Required: Whether the operation succeeded - "data": {...}, # Optional: Command-specific response data - "error": str # Optional: Error message if success=False + "success": bool, # Required: Whether the operation succeeded + "message": str | None, # Optional: Human-readable message + "extrinsic_identifier": str | None # Optional: Block-extrinsic ID (e.g., "12345-2") } -For transaction responses, data should include: +Standard Data Response Format: { - "extrinsic_hash": str, # The transaction hash - "block_hash": str # The block containing the transaction + "success": bool, # Required: Whether the operation succeeded + "data": {...}, # Optional: Command-specific response data + "error": str # Optional: Error message if success=False } """ @@ -22,17 +23,119 @@ from typing import Any, Optional, Union from rich.console import Console -# JSON console for outputting JSON responses json_console = Console() +def transaction_response( + success: bool, + message: Optional[str] = None, + extrinsic_identifier: Optional[str] = None, +) -> dict[str, Any]: + """ + Create a standardized transaction response dictionary. + + Args: + success: Whether the transaction succeeded + message: Human-readable status message + extrinsic_identifier: The extrinsic ID (e.g., "12345678-2") + + Returns: + Dictionary with standardized transaction format + """ + return { + "success": success, + "message": message, + "extrinsic_identifier": extrinsic_identifier, + } + + +def print_transaction_response( + success: bool, + message: Optional[str] = None, + extrinsic_identifier: Optional[str] = None, +) -> None: + """ + Print a standardized transaction response as JSON. + + Args: + success: Whether the transaction succeeded + message: Human-readable status message + extrinsic_identifier: The extrinsic ID (e.g., "12345678-2") + """ + json_console.print_json(data=transaction_response(success, message, extrinsic_identifier)) + + +class TransactionResult: + """ + Helper class for building transaction responses. + + Provides a clean interface for transaction commands that need to + build up response data before printing. + """ + + def __init__( + self, + success: bool, + message: Optional[str] = None, + extrinsic_identifier: Optional[str] = None, + ): + self.success = success + self.message = message + self.extrinsic_identifier = extrinsic_identifier + + def as_dict(self) -> dict[str, Any]: + """Return the response as a dictionary.""" + return transaction_response( + self.success, + self.message, + self.extrinsic_identifier, + ) + + def print(self) -> None: + """Print the response as JSON.""" + json_console.print_json(data=self.as_dict()) + + +class MultiTransactionResult: + """ + Helper class for commands that process multiple transactions. + + Builds a keyed dictionary of transaction results. + """ + + def __init__(self): + self._results: dict[str, TransactionResult] = {} + + def add( + self, + key: str, + success: bool, + message: Optional[str] = None, + extrinsic_identifier: Optional[str] = None, + ) -> None: + """Add a transaction result with the given key.""" + self._results[key] = TransactionResult(success, message, extrinsic_identifier) + + def add_result(self, key: str, result: TransactionResult) -> None: + """Add an existing TransactionResult with the given key.""" + self._results[key] = result + + def as_dict(self) -> dict[str, dict[str, Any]]: + """Return all results as a dictionary.""" + return {k: v.as_dict() for k, v in self._results.items()} + + def print(self) -> None: + """Print all results as JSON.""" + json_console.print_json(data=self.as_dict()) + + def json_response( success: bool, data: Optional[Any] = None, error: Optional[str] = None, ) -> str: """ - Create a standardized JSON response string. + Create a standardized JSON response string for data queries. Args: success: Whether the operation succeeded @@ -62,7 +165,7 @@ def json_response( def json_success(data: Any) -> str: """ - Create a successful JSON response. + Create a successful JSON response string. Args: data: Response data to include @@ -75,7 +178,7 @@ def json_success(data: Any) -> str: def json_error(error: str, data: Optional[Any] = None) -> str: """ - Create an error JSON response. + Create an error JSON response string. Args: error: Error message describing what went wrong @@ -87,43 +190,9 @@ def json_error(error: str, data: Optional[Any] = None) -> str: return json_response(success=False, data=data, error=error) -def json_transaction( - success: bool, - extrinsic_hash: Optional[str] = None, - block_hash: Optional[str] = None, - error: Optional[str] = None, - **extra_data: Any, -) -> str: - """ - Create a standardized transaction response. - - Args: - success: Whether the transaction succeeded - extrinsic_hash: The transaction/extrinsic hash - block_hash: The block hash containing the transaction - error: Error message if transaction failed - **extra_data: Additional transaction-specific data - - Returns: - JSON string with transaction details - """ - data: dict[str, Any] = {} - - if extrinsic_hash is not None: - data["extrinsic_hash"] = extrinsic_hash - - if block_hash is not None: - data["block_hash"] = block_hash - - # Add any extra data - data.update(extra_data) - - return json_response(success=success, data=data if data else None, error=error) - - def print_json(response: str) -> None: """ - Print a JSON response to the console. + Print a JSON string response to the console. Args: response: JSON string to print @@ -152,6 +221,16 @@ def print_json_error(error: str, data: Optional[Any] = None) -> None: print_json(json_error(error, data)) +def print_json_data(data: Any) -> None: + """ + Print data directly as JSON (for simple data responses). + + Args: + data: Data to print as JSON + """ + json_console.print_json(data=data) + + def serialize_balance(balance: Any) -> dict[str, Union[int, float]]: """ Serialize a Balance object to a consistent dictionary format. diff --git a/bittensor_cli/src/commands/liquidity/liquidity.py b/bittensor_cli/src/commands/liquidity/liquidity.py index 7997afc5f..6ad87ef13 100644 --- a/bittensor_cli/src/commands/liquidity/liquidity.py +++ b/bittensor_cli/src/commands/liquidity/liquidity.py @@ -14,6 +14,12 @@ json_console, print_extrinsic_id, ) +from bittensor_cli.src.bittensor.json_utils import ( + print_transaction_response, + print_json_data, + TransactionResult, + MultiTransactionResult, +) from bittensor_cli.src.bittensor.balances import Balance, fixed_to_float from bittensor_cli.src.commands.liquidity.utils import ( LiquidityPosition, @@ -295,13 +301,7 @@ async def add_liquidity( else: ext_id = None if json_output: - json_console.print_json( - data={ - "success": success, - "message": message, - "extrinsic_identifier": ext_id, - } - ) + print_transaction_response(success, message, ext_id) else: if success: console.print( @@ -558,9 +558,7 @@ async def show_liquidity_list( if not json_output: console.print(liquidity_table) else: - json_console.print( - json.dumps({"success": True, "err_msg": "", "positions": json_table}) - ) + print_json_data({"success": True, "err_msg": "", "positions": json_table}) async def remove_liquidity( @@ -584,9 +582,7 @@ async def remove_liquidity( success, msg, positions = await get_liquidity_list(subtensor, wallet, netuid) if not success: if json_output: - json_console.print_json( - data={"success": False, "err_msg": msg, "positions": positions} - ) + print_json_data({"success": False, "err_msg": msg, "positions": positions}) else: return print_error(f"Error: {msg}") return None @@ -629,14 +625,11 @@ async def remove_liquidity( else: print_error(f"Error removing {posid}: {msg}") else: - json_table = {} + json_results = MultiTransactionResult() for (success, msg, ext_receipt), posid in zip(results, position_ids): - json_table[posid] = { - "success": success, - "err_msg": msg, - "extrinsic_identifier": await ext_receipt.get_extrinsic_identifier(), - } - json_console.print_json(data=json_table) + ext_id = await ext_receipt.get_extrinsic_identifier() if ext_receipt else None + json_results.add(str(posid), success, msg, ext_id) + json_results.print() return None @@ -657,7 +650,7 @@ async def modify_liquidity( if not await subtensor.subnet_exists(netuid=netuid): err_msg = f"Subnet with netuid: {netuid} does not exist in {subtensor}." if json_output: - json_console.print(json.dumps({"success": False, "err_msg": err_msg})) + print_transaction_response(False, err_msg, None) else: print_error(err_msg) return False @@ -687,9 +680,7 @@ async def modify_liquidity( ) if json_output: ext_id = await ext_receipt.get_extrinsic_identifier() if success else None - json_console.print_json( - data={"success": success, "err_msg": msg, "extrinsic_identifier": ext_id} - ) + print_transaction_response(success, msg, ext_id) else: if success: await print_extrinsic_id(ext_receipt) diff --git a/bittensor_cli/src/commands/stake/add.py b/bittensor_cli/src/commands/stake/add.py index 18c6578eb..f967d08f9 100644 --- a/bittensor_cli/src/commands/stake/add.py +++ b/bittensor_cli/src/commands/stake/add.py @@ -26,6 +26,7 @@ get_hotkey_pub_ss58, print_extrinsic_id, ) +from bittensor_cli.src.bittensor.json_utils import print_json_data from bittensor_wallet import Wallet if TYPE_CHECKING: @@ -528,13 +529,11 @@ async def stake_extrinsic( staking_address ] = await ext_receipt.get_extrinsic_identifier() if json_output: - json_console.print_json( - data={ - "staking_success": successes, - "error_messages": error_messages, - "extrinsic_ids": extrinsic_ids, - } - ) + print_json_data({ + "staking_success": successes, + "error_messages": error_messages, + "extrinsic_ids": extrinsic_ids, + }) # Helper functions diff --git a/bittensor_cli/src/commands/stake/remove.py b/bittensor_cli/src/commands/stake/remove.py index 1cc7116a5..90c8de22f 100644 --- a/bittensor_cli/src/commands/stake/remove.py +++ b/bittensor_cli/src/commands/stake/remove.py @@ -29,6 +29,7 @@ get_hotkey_pub_ss58, print_extrinsic_id, ) +from bittensor_cli.src.bittensor.json_utils import print_json_data if TYPE_CHECKING: from bittensor_cli.src.bittensor.subtensor_interface import SubtensorInterface @@ -370,7 +371,7 @@ async def unstake( f"[{COLOR_PALETTE['STAKE']['STAKE_AMOUNT']}]Unstaking operations completed." ) if json_output: - json_console.print_json(data=successes) + print_json_data(successes) return True @@ -580,7 +581,7 @@ async def unstake_all( "extrinsic_identifier": ext_id, } if json_output: - json_console.print(json.dumps({"success": successes})) + print_json_data({"success": successes}) # Extrinsics diff --git a/bittensor_cli/src/commands/sudo.py b/bittensor_cli/src/commands/sudo.py index b039ea4f2..24240d8e1 100644 --- a/bittensor_cli/src/commands/sudo.py +++ b/bittensor_cli/src/commands/sudo.py @@ -33,6 +33,7 @@ get_hotkey_pub_ss58, print_extrinsic_id, ) +from bittensor_cli.src.bittensor.json_utils import print_transaction_response if TYPE_CHECKING: from bittensor_cli.src.bittensor.subtensor_interface import ( @@ -1252,7 +1253,7 @@ async def trim( if subnet_owner != wallet.coldkeypub.ss58_address: err_msg = "This wallet doesn't own the specified subnet." if json_output: - json_console.print_json(data={"success": False, "message": err_msg}) + print_transaction_response(False, err_msg, None) else: print_error(err_msg) return False @@ -1274,13 +1275,7 @@ async def trim( ) if not success: if json_output: - json_console.print_json( - data={ - "success": False, - "message": err_msg, - "extrinsic_identifier": None, - } - ) + print_transaction_response(False, err_msg, None) else: print_error(err_msg) return False @@ -1288,9 +1283,7 @@ async def trim( ext_id = await ext_receipt.get_extrinsic_identifier() msg = f"Successfully trimmed UIDs on SN{netuid} to {max_n}" if json_output: - json_console.print_json( - data={"success": True, "message": msg, "extrinsic_identifier": ext_id} - ) + print_transaction_response(True, msg, ext_id) else: await print_extrinsic_id(ext_receipt) console.print( diff --git a/bittensor_cli/src/commands/wallets.py b/bittensor_cli/src/commands/wallets.py index 3f8e90f3f..596c0d927 100644 --- a/bittensor_cli/src/commands/wallets.py +++ b/bittensor_cli/src/commands/wallets.py @@ -58,6 +58,8 @@ json_success, json_error, print_json, + print_json_data, + print_transaction_response, ) @@ -1525,9 +1527,8 @@ async def transfer( ) ext_id = (await ext_receipt.get_extrinsic_identifier()) if result else None if json_output: - json_console.print( - json.dumps({"success": result, "extrinsic_identifier": ext_id}) - ) + msg = "Transfer successful" if result else "Transfer failed" + print_transaction_response(result, msg, ext_id) else: await print_extrinsic_id(ext_receipt) return result @@ -1733,9 +1734,8 @@ async def swap_hotkey( else: ext_id = None if json_output: - json_console.print( - json.dumps({"success": result, "extrinsic_identifier": ext_id}) - ) + msg = "Hotkey swap successful" if result else "Hotkey swap failed" + print_transaction_response(result, msg, ext_id) else: await print_extrinsic_id(ext_receipt) return result @@ -1811,23 +1811,20 @@ async def set_id( print_error(f"Failed! {err_msg}") output_dict["error"] = err_msg if json_output: - json_console.print(json.dumps(output_dict)) + print_transaction_response(False, err_msg, None) return False else: console.print(":white_heavy_check_mark: [dark_sea_green3]Success!") ext_id = await ext_receipt.get_extrinsic_identifier() await print_extrinsic_id(ext_receipt) - output_dict["success"] = True identity = await subtensor.query_identity(wallet.coldkeypub.ss58_address) table = create_identity_table(title="New on-chain Identity") table.add_row("Address", wallet.coldkeypub.ss58_address) for key, value in identity.items(): table.add_row(key, str(value) if value else "~") - output_dict["identity"] = identity - output_dict["extrinsic_identifier"] = ext_id if json_output: - json_console.print(json.dumps(output_dict)) + print_transaction_response(True, "Identity set successfully", ext_id) else: console.print(table) return True diff --git a/tests/unit_tests/test_json_utils.py b/tests/unit_tests/test_json_utils.py index 216244c79..277ef69a8 100644 --- a/tests/unit_tests/test_json_utils.py +++ b/tests/unit_tests/test_json_utils.py @@ -13,8 +13,10 @@ json_response, json_success, json_error, - json_transaction, serialize_balance, + transaction_response, + TransactionResult, + MultiTransactionResult, ) @@ -127,47 +129,100 @@ def test_error_with_context(self): assert parsed["data"]["field"] == "amount" -class TestJsonTransaction: - """Tests for the json_transaction helper.""" +class TestTransactionResponse: + """Tests for the transaction_response helper.""" def test_successful_transaction(self): """Test successful transaction response.""" - result = json_transaction( + result = transaction_response( success=True, - extrinsic_hash="0x123abc", - block_hash="0x456def" + message="Transfer successful", + extrinsic_identifier="12345678-2" ) - parsed = json.loads(result) - assert parsed["success"] is True - assert parsed["data"]["extrinsic_hash"] == "0x123abc" - assert parsed["data"]["block_hash"] == "0x456def" + assert result["success"] is True + assert result["message"] == "Transfer successful" + assert result["extrinsic_identifier"] == "12345678-2" def test_failed_transaction(self): """Test failed transaction response.""" - result = json_transaction( + result = transaction_response( success=False, - error="Insufficient balance" + message="Insufficient balance" ) - parsed = json.loads(result) - assert parsed["success"] is False - assert parsed["error"] == "Insufficient balance" + assert result["success"] is False + assert result["message"] == "Insufficient balance" + assert result["extrinsic_identifier"] is None - def test_transaction_with_extra_data(self): - """Test transaction with extra data fields.""" - result = json_transaction( + def test_transaction_without_message(self): + """Test transaction without message.""" + result = transaction_response( success=True, - extrinsic_hash="0x123", - amount=100.5, - recipient="5xyz..." + extrinsic_identifier="12345678-3" ) - parsed = json.loads(result) - assert parsed["success"] is True - assert parsed["data"]["extrinsic_hash"] == "0x123" - assert parsed["data"]["amount"] == 100.5 - assert parsed["data"]["recipient"] == "5xyz..." + assert result["success"] is True + assert result["message"] is None + assert result["extrinsic_identifier"] == "12345678-3" + + +class TestTransactionResult: + """Tests for the TransactionResult class.""" + + def test_as_dict(self): + """Test conversion to dictionary.""" + result = TransactionResult( + success=True, + message="Success", + extrinsic_identifier="12345-1" + ) + d = result.as_dict() + + assert d["success"] is True + assert d["message"] == "Success" + assert d["extrinsic_identifier"] == "12345-1" + + def test_failed_result(self): + """Test failed transaction result.""" + result = TransactionResult( + success=False, + message="Error occurred" + ) + d = result.as_dict() + + assert d["success"] is False + assert d["message"] == "Error occurred" + assert d["extrinsic_identifier"] is None + + +class TestMultiTransactionResult: + """Tests for the MultiTransactionResult class.""" + + def test_add_results(self): + """Test adding multiple results.""" + multi = MultiTransactionResult() + multi.add("pos1", True, "Success 1", "12345-1") + multi.add("pos2", False, "Failed 2", None) + + d = multi.as_dict() + + assert "pos1" in d + assert "pos2" in d + assert d["pos1"]["success"] is True + assert d["pos1"]["extrinsic_identifier"] == "12345-1" + assert d["pos2"]["success"] is False + assert d["pos2"]["extrinsic_identifier"] is None + + def test_add_result_object(self): + """Test adding TransactionResult objects.""" + multi = MultiTransactionResult() + result = TransactionResult(True, "Test", "123-1") + multi.add_result("key1", result) + + d = multi.as_dict() + assert d["key1"]["success"] is True + assert d["key1"]["message"] == "Test" class TestSerializeBalance: @@ -236,13 +291,16 @@ def test_error_always_has_success_and_error_fields(self): assert "error" in parsed assert parsed["success"] is False - def test_no_null_data_in_success(self): - """Verify success responses don't include null data.""" - result = json_success({"key": "value"}) - parsed = json.loads(result) + def test_transaction_response_schema(self): + """Verify transaction responses have consistent schema.""" + success_result = transaction_response(True, "OK", "123-1") + fail_result = transaction_response(False, "Failed", None) - # Data should be present, not null - assert parsed.get("data") is not None + # Both should have all three keys + for result in [success_result, fail_result]: + assert "success" in result + assert "message" in result + assert "extrinsic_identifier" in result def test_json_is_valid(self): """Verify all outputs are valid JSON.""" @@ -250,7 +308,6 @@ def test_json_is_valid(self): json_response(True, {"test": "data"}), json_success({"nested": {"deep": True}}), json_error("test error"), - json_transaction(True, "0x123", "0x456"), ] for output in test_cases: