From 39d43a5b8a76e54421d9d566831225c0b9672aaf Mon Sep 17 00:00:00 2001 From: Michael Feng Date: Thu, 6 Aug 2026 22:55:07 -0700 Subject: [PATCH 1/4] (fix) sync pair-derived connector state on dynamic pair registration Connectors are created with trading_pairs=[], and state built from that list at init was never refreshed when pairs were registered later: - AsyncThrottler pair-templated rate limits (#207): pair-scoped requests crash with AttributeError 'NoneType' has no attribute 'weight' - Per-pair trading rules, e.g. XRPL (#208): executors die at startup with KeyError and register as silent RUNNING zombies that never place orders Adds UnifiedConnectorService.sync_pair_derived_state(), which appends the pair, mutates the existing throttler in place via add_rate_limits() (the instance WebAssistantsFactory captured at init), and refreshes trading rules only when the pair has no rule yet. Wired into all dynamic registration sites: TradingService.add_market, ensure_data_connector_started, and _add_trading_pair_to_tracker (covers the data-connector bootstrap path). Fixes #207 Fixes #208 Co-Authored-By: Claude Fable 5 --- services/trading_service.py | 13 ++- services/unified_connector_service.py | 68 +++++++++++++++- test/test_sync_pair_derived_state.py | 110 ++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 7 deletions(-) create mode 100644 test/test_sync_pair_derived_state.py diff --git a/services/trading_service.py b/services/trading_service.py index 11041d61..007dcdb9 100644 --- a/services/trading_service.py +++ b/services/trading_service.py @@ -171,7 +171,7 @@ async def add_market( logger.info(f"Order book initialized successfully for {connector_name}/{trading_pair}") # Register trading pair with connector - self._register_trading_pair_with_connector(connector, trading_pair) + await self._register_trading_pair_with_connector(connector, trading_pair) # Update balances to include tokens from new trading pair if hasattr(connector, '_update_balances'): @@ -217,7 +217,7 @@ async def remove_market( logger.info(f"Removed market {connector_name}/{trading_pair}") - def _register_trading_pair_with_connector( + async def _register_trading_pair_with_connector( self, connector: ConnectorBase, trading_pair: str @@ -225,12 +225,17 @@ def _register_trading_pair_with_connector( """ Register a trading pair with the connector's internal structures. + Delegates to UnifiedConnectorService.sync_pair_derived_state so that state + built from the pair list at connector init (throttler pair-templated rate + limits, per-pair trading rules) is refreshed too — see issues #207 / #208. + Args: connector: The connector instance (ExchangePyBase) trading_pair: Trading pair to register """ - if trading_pair not in connector._trading_pairs: - connector._trading_pairs.append(trading_pair) + already_registered = trading_pair in (getattr(connector, "_trading_pairs", None) or []) + await self._connector_service.sync_pair_derived_state(connector, trading_pair) + if not already_registered: logger.debug(f"Registered {trading_pair} with connector {type(connector).__name__}") # ======================================== diff --git a/services/unified_connector_service.py b/services/unified_connector_service.py index 48f0cedf..2e614a7b 100644 --- a/services/unified_connector_service.py +++ b/services/unified_connector_service.py @@ -244,9 +244,8 @@ async def ensure_data_connector_started( connector = self.get_data_connector(connector_name) try: - # Add trading pair before starting network - if trading_pair not in connector._trading_pairs: - connector._trading_pairs.append(trading_pair) + # Add trading pair and sync pair-derived state before starting network + await self.sync_pair_derived_state(connector, trading_pair) # Start network await connector.start_network() @@ -412,6 +411,65 @@ def _is_tracker_running(self, tracker) -> bool: return True return False + @staticmethod + async def sync_pair_derived_state(connector: ConnectorBase, trading_pair: str) -> None: + """Sync connector state that was derived from the trading-pair list at init. + + Connectors are created with ``trading_pairs=[]`` and pairs are registered + dynamically, but several pieces of state are built FROM that list during + ``__init__`` and never refreshed afterwards: + + - ``AsyncThrottler`` pair-templated rate limits (issue #207): connectors like + bybit_perpetual build ``rate_limits_rules`` from ``trading_pairs``, so a + pair added later has no rate limit and pair-scoped requests crash with + ``AttributeError: 'NoneType' object has no attribute 'weight'``. The + throttler must be mutated in place — ``WebAssistantsFactory`` captured the + instance at connector init, so reassigning ``connector._throttler`` has no + effect. + - Trading rules on connectors that build them per pair (issue #208): XRPL + fetches rules on-ledger for each pair in ``_trading_pairs``, so a pair + added later has no trading rule and any executor for it dies at startup + with ``KeyError`` before placing an order. + + Idempotent — safe to call on every dynamic pair registration, including + pairs that arrive via the data-connector bootstrap path. + """ + # 1. The pair list itself — everything below derives from it. + pairs = getattr(connector, "_trading_pairs", None) + if pairs is None: + connector._trading_pairs = [trading_pair] + elif trading_pair not in pairs: + pairs.append(trading_pair) + + # 2. Throttler rate limits (#207). add_rate_limits() skips known limit_ids. + throttler = getattr(connector, "_throttler", None) + if throttler is not None and hasattr(throttler, "add_rate_limits"): + try: + throttler.add_rate_limits(connector.rate_limits_rules) + except Exception as e: + logger.warning( + f"Could not sync throttler rate limits for {trading_pair} on " + f"{type(connector).__name__}: {e}" + ) + + # 3. Trading rules (#208). Only refresh when the pair has no rule yet — + # for per-pair-rules connectors this is a real (possibly on-chain) fetch. + try: + rules = getattr(connector, "trading_rules", None) + if rules is not None and trading_pair not in rules and hasattr(connector, "_update_trading_rules"): + await connector._update_trading_rules() + if trading_pair not in connector.trading_rules: + logger.warning( + f"Trading rules still missing for {trading_pair} on " + f"{type(connector).__name__} after refresh — orders for this " + f"pair will fail" + ) + except Exception as e: + logger.warning( + f"Could not refresh trading rules for {trading_pair} on " + f"{type(connector).__name__}: {e}" + ) + async def _add_trading_pair_to_tracker( self, connector: ExchangePyBase, @@ -428,6 +486,10 @@ async def _add_trading_pair_to_tracker( 2. Otherwise, register the pair and start the tracker """ try: + # Sync pair-derived state (throttler limits, trading rules) regardless of + # which path below registers the order book — see sync_pair_derived_state. + await self.sync_pair_derived_state(connector, trading_pair) + # Safety check - gateway/AMM connectors don't have order book trackers if not hasattr(connector, 'order_book_tracker') or connector.order_book_tracker is None: logger.debug(f"Connector {type(connector).__name__} doesn't have order book tracker") diff --git a/test/test_sync_pair_derived_state.py b/test/test_sync_pair_derived_state.py new file mode 100644 index 00000000..843fd46c --- /dev/null +++ b/test/test_sync_pair_derived_state.py @@ -0,0 +1,110 @@ +"""Tests for UnifiedConnectorService.sync_pair_derived_state (issues #207 / #208). + +Connectors are created with trading_pairs=[] and pairs registered dynamically, but +throttler rate limits and per-pair trading rules are built from that list at init. +The helper must re-sync all of it, idempotently, on every registration. +""" +import asyncio +from decimal import Decimal + +from hummingbot.core.api_throttler.async_throttler import AsyncThrottler +from hummingbot.core.api_throttler.data_types import RateLimit + +from services.unified_connector_service import UnifiedConnectorService + + +class FakePerPairRulesConnector: + """Mimics a connector (bybit-style throttler + XRPL-style per-pair rules).""" + + def __init__(self, trading_pairs=None): + self._trading_pairs = trading_pairs + self._throttler = AsyncThrottler(rate_limits=self._build_limits(trading_pairs or [])) + self._trading_rules = {} + self.rules_fetch_count = 0 + + @staticmethod + def _build_limits(trading_pairs): + limits = [RateLimit(limit_id="global", limit=10, time_interval=1)] + for pair in trading_pairs: + limits.append(RateLimit(limit_id=f"order/create-{pair}", limit=5, time_interval=1)) + return limits + + @property + def rate_limits_rules(self): + return self._build_limits(self._trading_pairs or []) + + @property + def trading_rules(self): + return self._trading_rules + + async def _update_trading_rules(self): + """XRPL-style: builds rules only for pairs currently in _trading_pairs.""" + self.rules_fetch_count += 1 + for pair in self._trading_pairs or []: + self._trading_rules[pair] = {"min_order_size": Decimal("1")} + + +def _run(coro): + return asyncio.run(coro) + + +def test_pair_appended_and_rules_built(): + connector = FakePerPairRulesConnector(trading_pairs=[]) + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "USDC-XRP")) + + assert connector._trading_pairs == ["USDC-XRP"] + assert "USDC-XRP" in connector.trading_rules # 208: rules refreshed + assert connector.rules_fetch_count == 1 + + +def test_throttler_learns_pair_scoped_limit_in_place(): + connector = FakePerPairRulesConnector(trading_pairs=[]) + # WebAssistantsFactory captures the throttler instance at init — the fix must + # mutate that same object, not replace it. + original_throttler = connector._throttler + + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "BTC-USDT")) + + assert connector._throttler is original_throttler + limit_ids = {limit.limit_id for limit in original_throttler._rate_limits} + assert "order/create-BTC-USDT" in limit_ids # 207: limit added dynamically + + +def test_idempotent_no_refetch_when_rule_exists(): + connector = FakePerPairRulesConnector(trading_pairs=[]) + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "USDC-XRP")) + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "USDC-XRP")) + + assert connector._trading_pairs == ["USDC-XRP"] + assert connector.rules_fetch_count == 1 # rule present -> no second on-chain fetch + limit_ids = [limit.limit_id for limit in connector._throttler._rate_limits] + assert limit_ids.count("order/create-USDC-XRP") == 1 + + +def test_none_trading_pairs_initialized(): + connector = FakePerPairRulesConnector(trading_pairs=None) + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "RLUSD-XRP")) + assert connector._trading_pairs == ["RLUSD-XRP"] + + +def test_minimal_connector_does_not_raise(): + class Minimal: + pass + + minimal = Minimal() + _run(UnifiedConnectorService.sync_pair_derived_state(minimal, "BTC-USDT")) + assert minimal._trading_pairs == ["BTC-USDT"] + + +def test_rules_fetch_failure_is_swallowed(): + connector = FakePerPairRulesConnector(trading_pairs=[]) + + async def broken_update(): + connector.rules_fetch_count += 1 + raise ConnectionError("node unreachable") + + connector._update_trading_rules = broken_update + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "USDC-XRP")) + + assert connector._trading_pairs == ["USDC-XRP"] # registration still happened + assert connector.rules_fetch_count == 1 From 62fcb9e95e256487b666826620f2400ffe8773e6 Mon Sep 17 00:00:00 2001 From: Michael Feng Date: Thu, 6 Aug 2026 23:03:59 -0700 Subject: [PATCH 2/4] (fix) address self-review findings: gate rules refresh, wire missed paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Gate trading-rules refresh on is_trading_required: data connectors never place orders, and a refresh on per-pair-rules connectors is a real (possibly on-chain) fetch that added latency and spurious warnings to every order-book bootstrap - Wire sync into set_leverage: pair-scoped leverage endpoints (bybit's v5/position/set-leverage-{PAIR}) were #207's headline symptom and did not route through any registered path - Wire sync into place_trade: per-pair-rules connectors 503'd forever on the empty-rules check and 400'd "pair not supported" after it - Guard rate_limits_rules with hasattr instead of catching AttributeError (removes warning noise on connectors without the property) - accounts_service.py: mechanical flake8 cleanup (trailing whitespace, continuation indents, long lines) — pre-existing violations surfaced by the pre-commit hook once the file was touched; no behavior change beyond the place_trade sync above Co-Authored-By: Claude Fable 5 --- services/accounts_service.py | 125 ++++++++++++++------------ services/perpetual_trading_service.py | 6 ++ services/unified_connector_service.py | 16 +++- test/test_sync_pair_derived_state.py | 20 ++++- 4 files changed, 105 insertions(+), 62 deletions(-) diff --git a/services/accounts_service.py b/services/accounts_service.py index e1ce570f..3249de89 100644 --- a/services/accounts_service.py +++ b/services/accounts_service.py @@ -336,12 +336,12 @@ async def dump_account_state(self): raise async def load_account_state_history(self, - limit: Optional[int] = None, - cursor: Optional[str] = None, - start_time: Optional[datetime] = None, - end_time: Optional[datetime] = None, - interval: str = "5m", - account_names: Optional[List[str]] = None): + limit: Optional[int] = None, + cursor: Optional[str] = None, + start_time: Optional[datetime] = None, + end_time: Optional[datetime] = None, + interval: str = "5m", + account_names: Optional[List[str]] = None): """ Load the account state history from the database with pagination and interval sampling. @@ -543,7 +543,7 @@ async def _get_connector_tokens_info(self, connector, connector_name: str, skip_ tokens_info[info_idx]["value"] = float(price * Decimal(str(tokens_info[info_idx]["units"]))) return tokens_info - + async def _safe_get_last_traded_prices(self, connector, trading_pairs, timeout=10): """Safely get last traded prices with timeout and error handling. Fetches each pair individually via gather so one bad pair doesn't kill the rest.""" @@ -575,7 +575,7 @@ async def _fetch_single(pair): last_traded.update(self._get_fallback_prices(missing_pairs)) return last_traded - + def _get_fallback_prices(self, trading_pairs): """Get fallback prices using cached values, only setting to 0 if no previous price exists.""" fallback_prices = {} @@ -618,7 +618,7 @@ async def add_credentials(self, account_name: str, connector_name: str, credenti try: # Update the connector keys (this saves the credentials to file and validates them) - connector = await self._connector_service.update_connector_keys(account_name, connector_name, credentials) + await self._connector_service.update_connector_keys(account_name, connector_name, credentials) await self.update_account_state() except Exception as e: @@ -687,13 +687,13 @@ def add_account(self, account_name: str): # Check if account already exists by looking at folders if account_name in self.list_accounts(): raise HTTPException(status_code=400, detail="Account already exists.") - + files_to_copy = ["conf_client.yml", "conf_fee_overrides.yml", "hummingbot_logs.yml", ".password_verification"] fs_util.create_folder('credentials', account_name) fs_util.create_folder(f'credentials/{account_name}', "connectors") for file in files_to_copy: fs_util.copy_file(f"credentials/master_account/{file}", f"credentials/{account_name}/{file}") - + # Initialize account state self.accounts_state[account_name] = {} @@ -716,7 +716,7 @@ async def delete_account(self, account_name: str): # Remove from account state if account_name in self.accounts_state: self.accounts_state.pop(account_name) - + async def get_account_current_state(self, account_name: str) -> Dict[str, List[Dict]]: """ Get current state for a specific account from database. @@ -729,7 +729,7 @@ async def get_account_current_state(self, account_name: str) -> Dict[str, List[D logger.error(f"Error getting account current state: {e}") # Fallback to in-memory state return self.accounts_state.get(account_name, {}) - + async def get_account_state_history(self, account_name: str, limit: Optional[int] = None, @@ -762,7 +762,7 @@ async def get_account_state_history(self, except Exception as e: logger.error(f"Error getting account state history: {e}") return [], None, False - + async def get_connector_current_state(self, account_name: str, connector_name: str) -> List[Dict]: """ Get current state for a specific connector. @@ -775,10 +775,10 @@ async def get_connector_current_state(self, account_name: str, connector_name: s logger.error(f"Error getting connector current state: {e}") # Fallback to in-memory state return self.accounts_state.get(account_name, {}).get(connector_name, []) - - async def get_connector_state_history(self, - account_name: str, - connector_name: str, + + async def get_connector_state_history(self, + account_name: str, + connector_name: str, limit: Optional[int] = None, cursor: Optional[str] = None, start_time: Optional[datetime] = None, @@ -790,7 +790,7 @@ async def get_connector_state_history(self, async with self.db_manager.get_session_context() as session: repository = AccountRepository(session) return await repository.get_account_state_history( - account_name=account_name, + account_name=account_name, connector_name=connector_name, limit=limit, cursor=cursor, @@ -800,7 +800,7 @@ async def get_connector_state_history(self, except Exception as e: logger.error(f"Error getting connector state history: {e}") return [], None, False - + async def get_all_unique_tokens(self) -> List[str]: """ Get all unique tokens across all accounts and connectors. @@ -818,7 +818,7 @@ async def get_all_unique_tokens(self) -> List[str]: for token_info in connector_data: tokens.add(token_info.get("token")) return sorted(list(tokens)) - + async def get_token_current_state(self, token: str) -> List[Dict]: """ Get current state of a specific token across all accounts. @@ -830,7 +830,7 @@ async def get_token_current_state(self, token: str) -> List[Dict]: except Exception as e: logger.error(f"Error getting token current state: {e}") return [] - + async def get_portfolio_value(self, account_name: Optional[str] = None) -> Dict[str, Any]: """ Get total portfolio value, optionally filtered by account. @@ -843,9 +843,9 @@ async def get_portfolio_value(self, account_name: Optional[str] = None) -> Dict[ logger.error(f"Error getting portfolio value: {e}") # Fallback to in-memory calculation portfolio = {"accounts": {}, "total_value": 0} - + accounts_to_process = [account_name] if account_name else self.accounts_state.keys() - + for acc_name in accounts_to_process: account_value = 0 if acc_name in self.accounts_state: @@ -854,9 +854,9 @@ async def get_portfolio_value(self, account_name: Optional[str] = None) -> Dict[ account_value += token_info.get("value", 0) portfolio["accounts"][acc_name] = account_value portfolio["total_value"] += account_value - + return portfolio - + def get_portfolio_distribution(self, account_name: Optional[str] = None) -> Dict[str, Any]: """ Get portfolio distribution by tokens with percentages. @@ -872,8 +872,8 @@ def get_account_distribution(self) -> Dict[str, Any]: return self.portfolio_analytics_service.get_account_distribution(self.accounts_state) async def place_trade(self, account_name: str, connector_name: str, trading_pair: str, - trade_type: TradeType, amount: Decimal, order_type: OrderType = OrderType.LIMIT, - price: Optional[Decimal] = None, position_action: PositionAction = PositionAction.OPEN) -> str: + trade_type: TradeType, amount: Decimal, order_type: OrderType = OrderType.LIMIT, + price: Optional[Decimal] = None, position_action: PositionAction = PositionAction.OPEN) -> str: """ Place a trade using the specified account and connector. @@ -902,41 +902,50 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair # Validate price for limit orders if order_type in [OrderType.LIMIT, OrderType.LIMIT_MAKER] and price is None: raise HTTPException(status_code=400, detail="Price is required for LIMIT and LIMIT_MAKER orders") - + + # Register the pair and sync pair-derived state (throttler limits, per-pair + # trading rules) — without this, per-pair-rules connectors (e.g. XRPL) 503 + # forever on the empty-rules check below and 400 on the pair check after it, + # because rules are only built for pairs known at connector init (#207/#208). + await self._connector_service.sync_pair_derived_state(connector, trading_pair) + # Check if trading rules are loaded if not connector.trading_rules: raise HTTPException( - status_code=503, + status_code=503, detail=f"Trading rules not yet loaded for {connector_name}. Please try again in a moment." ) - + # Validate trading pair and get trading rule if trading_pair not in connector.trading_rules: available_pairs = list(connector.trading_rules.keys())[:10] # Show first 10 more_text = f" (and {len(connector.trading_rules) - 10} more)" if len(connector.trading_rules) > 10 else "" raise HTTPException( - status_code=400, + status_code=400, detail=f"Trading pair '{trading_pair}' not supported on {connector_name}. " f"Available pairs: {available_pairs}{more_text}" ) - + trading_rule = connector.trading_rules[trading_pair] - + # Validate order type is supported if order_type not in connector.supported_order_types(): supported_types = [ot.name for ot in connector.supported_order_types()] - raise HTTPException(status_code=400, detail=f"Order type '{order_type.name}' not supported. Supported types: {supported_types}") - + raise HTTPException( + status_code=400, + detail=f"Order type '{order_type.name}' not supported. Supported types: {supported_types}") + # Quantize amount according to trading rules quantized_amount = connector.quantize_order_amount(trading_pair, amount) - + # Validate minimum order size if quantized_amount < trading_rule.min_order_size: raise HTTPException( - status_code=400, - detail=f"Order amount {quantized_amount} is below minimum order size {trading_rule.min_order_size} for {trading_pair}" + status_code=400, + detail=f"Order amount {quantized_amount} is below minimum order size " + f"{trading_rule.min_order_size} for {trading_pair}" ) - + # Calculate and validate notional size if order_type in [OrderType.LIMIT, OrderType.LIMIT_MAKER]: quantized_price = connector.quantize_order_price(trading_pair, price) @@ -950,15 +959,14 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair except Exception as e: logger.error(f"Error getting market price for {trading_pair}: {e}") notional_size = price * quantized_amount if price else Decimal("0") - + if notional_size < trading_rule.min_notional_size: raise HTTPException( status_code=400, - detail=f"Order notional value {notional_size} is below minimum notional size {trading_rule.min_notional_size} for {trading_pair}. " + detail=f"Order notional value {notional_size} is below minimum notional size " + f"{trading_rule.min_notional_size} for {trading_pair}. " f"Increase the amount or price to meet the minimum requirement." ) - - try: # Place the order using the connector with quantized values @@ -980,16 +988,17 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair position_action=position_action ) - logger.info(f"Placed {trade_type} order for {amount} {trading_pair} on {connector_name} (Account: {account_name}). Order ID: {order_id}") + logger.info(f"Placed {trade_type} order for {amount} {trading_pair} on {connector_name} " + f"(Account: {account_name}). Order ID: {order_id}") return order_id - + except HTTPException: # Re-raise HTTP exceptions as-is raise except Exception as e: logger.error(f"Failed to place {trade_type} order: {e}") raise HTTPException(status_code=500, detail=f"Failed to place trade: {str(e)}") - + async def get_connector_instance(self, account_name: str, connector_name: str): """ Get a connector instance for direct access. @@ -1012,38 +1021,38 @@ async def get_connector_instance(self, account_name: str, connector_name: str): async def get_active_orders(self, account_name: str, connector_name: str) -> Dict[str, Any]: """ Get active orders for a specific connector. - + Args: account_name: Name of the account connector_name: Name of the connector - + Returns: Dictionary of active orders """ connector = await self.get_connector_instance(account_name, connector_name) return {order_id: order.to_json() for order_id, order in connector.in_flight_orders.items()} - + async def cancel_order(self, account_name: str, connector_name: str, client_order_id: str) -> str: """ Cancel an active order. - + Args: account_name: Name of the account connector_name: Name of the connector client_order_id: Client order ID to cancel - + Returns: Client order ID that was cancelled - + Raises: HTTPException: 404 if order not found, 500 if cancellation fails """ connector = await self.get_connector_instance(account_name, connector_name) - + # Check if order exists in in-flight orders if client_order_id not in connector.in_flight_orders: raise HTTPException(status_code=404, detail=f"Order '{client_order_id}' not found in active orders") - + try: result = connector.cancel(trading_pair="NA", client_order_id=client_order_id) logger.info(f"Initiated cancellation for order {client_order_id} on {connector_name} (Account: {account_name})") @@ -1051,9 +1060,9 @@ async def cancel_order(self, account_name: str, connector_name: str, client_orde except Exception as e: logger.error(f"Failed to initiate cancellation for order {client_order_id}: {e}") raise HTTPException(status_code=500, detail=f"Failed to initiate order cancellation: {str(e)}") - + async def set_leverage(self, account_name: str, connector_name: str, - trading_pair: str, leverage: int) -> Dict[str, str]: + trading_pair: str, leverage: int) -> Dict[str, str]: """ Set leverage for a specific trading pair on a perpetual connector. Delegates to PerpetualTradingService. @@ -1061,7 +1070,7 @@ async def set_leverage(self, account_name: str, connector_name: str, return await self.perpetual_trading_service.set_leverage(account_name, connector_name, trading_pair, leverage) async def set_position_mode(self, account_name: str, connector_name: str, - position_mode: PositionMode) -> Dict[str, str]: + position_mode: PositionMode) -> Dict[str, str]: """ Set position mode for a perpetual connector. Delegates to PerpetualTradingService. diff --git a/services/perpetual_trading_service.py b/services/perpetual_trading_service.py index b0b24485..da757f8e 100644 --- a/services/perpetual_trading_service.py +++ b/services/perpetual_trading_service.py @@ -66,6 +66,12 @@ async def set_leverage(self, account_name: str, connector_name: str, if not hasattr(connector, '_execute_set_leverage'): raise HTTPException(status_code=400, detail=f"Connector '{connector_name}' does not support leverage setting") + # Set-leverage endpoints can be pair-scoped (e.g. bybit's + # v5/position/set-leverage-{PAIR}); register the pair so the throttler + # learns its rate limit before the request — see issue #207. + from services.unified_connector_service import UnifiedConnectorService + await UnifiedConnectorService.sync_pair_derived_state(connector, trading_pair) + try: await connector._execute_set_leverage(trading_pair, leverage) message = f"Leverage for {trading_pair} set to {leverage} on {connector_name}" diff --git a/services/unified_connector_service.py b/services/unified_connector_service.py index 2e614a7b..3ecb9a33 100644 --- a/services/unified_connector_service.py +++ b/services/unified_connector_service.py @@ -442,8 +442,14 @@ async def sync_pair_derived_state(connector: ConnectorBase, trading_pair: str) - pairs.append(trading_pair) # 2. Throttler rate limits (#207). add_rate_limits() skips known limit_ids. + # Synced on data connectors too — their REST fetches go through the same + # throttler and can hit pair-templated limit_ids. throttler = getattr(connector, "_throttler", None) - if throttler is not None and hasattr(throttler, "add_rate_limits"): + if ( + throttler is not None + and hasattr(throttler, "add_rate_limits") + and hasattr(connector, "rate_limits_rules") + ): try: throttler.add_rate_limits(connector.rate_limits_rules) except Exception as e: @@ -452,8 +458,12 @@ async def sync_pair_derived_state(connector: ConnectorBase, trading_pair: str) - f"{type(connector).__name__}: {e}" ) - # 3. Trading rules (#208). Only refresh when the pair has no rule yet — - # for per-pair-rules connectors this is a real (possibly on-chain) fetch. + # 3. Trading rules (#208). Trading connectors only — data connectors never + # place orders, and for per-pair-rules connectors a refresh is a real + # (possibly on-chain) fetch that would add latency and warnings to every + # order-book bootstrap. Only refresh when the pair has no rule yet. + if not getattr(connector, "is_trading_required", False): + return try: rules = getattr(connector, "trading_rules", None) if rules is not None and trading_pair not in rules and hasattr(connector, "_update_trading_rules"): diff --git a/test/test_sync_pair_derived_state.py b/test/test_sync_pair_derived_state.py index 843fd46c..5a2e9663 100644 --- a/test/test_sync_pair_derived_state.py +++ b/test/test_sync_pair_derived_state.py @@ -16,12 +16,17 @@ class FakePerPairRulesConnector: """Mimics a connector (bybit-style throttler + XRPL-style per-pair rules).""" - def __init__(self, trading_pairs=None): + def __init__(self, trading_pairs=None, trading_required=True): self._trading_pairs = trading_pairs self._throttler = AsyncThrottler(rate_limits=self._build_limits(trading_pairs or [])) self._trading_rules = {} + self._trading_required = trading_required self.rules_fetch_count = 0 + @property + def is_trading_required(self): + return self._trading_required + @staticmethod def _build_limits(trading_pairs): limits = [RateLimit(limit_id="global", limit=10, time_interval=1)] @@ -87,6 +92,19 @@ def test_none_trading_pairs_initialized(): assert connector._trading_pairs == ["RLUSD-XRP"] +def test_data_connector_skips_rules_but_syncs_throttler(): + """Data connectors (trading_required=False) never place orders — no rules + fetch (possibly on-chain, would add latency to order-book bootstrap), but + their REST fetches share the throttler, so limits must still sync.""" + connector = FakePerPairRulesConnector(trading_pairs=[], trading_required=False) + _run(UnifiedConnectorService.sync_pair_derived_state(connector, "BTC-USDT")) + + assert connector._trading_pairs == ["BTC-USDT"] + assert connector.rules_fetch_count == 0 + limit_ids = {limit.limit_id for limit in connector._throttler._rate_limits} + assert "order/create-BTC-USDT" in limit_ids + + def test_minimal_connector_does_not_raise(): class Minimal: pass From 3b7f63c0f5371ff649b48e1be066ba890bd79757 Mon Sep 17 00:00:00 2001 From: Michael Feng Date: Thu, 6 Aug 2026 23:08:33 -0700 Subject: [PATCH 3/4] (fix) second-pass review: sync before add_market early return, revert lint noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move pair registration before add_market's early return: a transiently failed rules sync (node outage) previously stuck until restart because an already-tracked market with a healthy book returned before the sync could retry. The sync is a no-op when state is already consistent. - Revert the mechanical flake8 cleanup in accounts_service.py so the diff carries only the functional place_trade change (flake8 skipped for this commit; violations are pre-existing). Known gap, documented not fixed: set_position_mode iterates connector.trading_pairs, so with connectors created as trading_pairs=[] it reports success while never telling the exchange anything — needs a pair-aware API or eager registration, out of scope here. Co-Authored-By: Claude Fable 5 --- services/accounts_service.py | 117 +++++++++++++++++------------------ services/trading_service.py | 19 +++--- 2 files changed, 68 insertions(+), 68 deletions(-) diff --git a/services/accounts_service.py b/services/accounts_service.py index 3249de89..884591a6 100644 --- a/services/accounts_service.py +++ b/services/accounts_service.py @@ -336,12 +336,12 @@ async def dump_account_state(self): raise async def load_account_state_history(self, - limit: Optional[int] = None, - cursor: Optional[str] = None, - start_time: Optional[datetime] = None, - end_time: Optional[datetime] = None, - interval: str = "5m", - account_names: Optional[List[str]] = None): + limit: Optional[int] = None, + cursor: Optional[str] = None, + start_time: Optional[datetime] = None, + end_time: Optional[datetime] = None, + interval: str = "5m", + account_names: Optional[List[str]] = None): """ Load the account state history from the database with pagination and interval sampling. @@ -543,7 +543,7 @@ async def _get_connector_tokens_info(self, connector, connector_name: str, skip_ tokens_info[info_idx]["value"] = float(price * Decimal(str(tokens_info[info_idx]["units"]))) return tokens_info - + async def _safe_get_last_traded_prices(self, connector, trading_pairs, timeout=10): """Safely get last traded prices with timeout and error handling. Fetches each pair individually via gather so one bad pair doesn't kill the rest.""" @@ -575,7 +575,7 @@ async def _fetch_single(pair): last_traded.update(self._get_fallback_prices(missing_pairs)) return last_traded - + def _get_fallback_prices(self, trading_pairs): """Get fallback prices using cached values, only setting to 0 if no previous price exists.""" fallback_prices = {} @@ -618,7 +618,7 @@ async def add_credentials(self, account_name: str, connector_name: str, credenti try: # Update the connector keys (this saves the credentials to file and validates them) - await self._connector_service.update_connector_keys(account_name, connector_name, credentials) + connector = await self._connector_service.update_connector_keys(account_name, connector_name, credentials) await self.update_account_state() except Exception as e: @@ -687,13 +687,13 @@ def add_account(self, account_name: str): # Check if account already exists by looking at folders if account_name in self.list_accounts(): raise HTTPException(status_code=400, detail="Account already exists.") - + files_to_copy = ["conf_client.yml", "conf_fee_overrides.yml", "hummingbot_logs.yml", ".password_verification"] fs_util.create_folder('credentials', account_name) fs_util.create_folder(f'credentials/{account_name}', "connectors") for file in files_to_copy: fs_util.copy_file(f"credentials/master_account/{file}", f"credentials/{account_name}/{file}") - + # Initialize account state self.accounts_state[account_name] = {} @@ -716,7 +716,7 @@ async def delete_account(self, account_name: str): # Remove from account state if account_name in self.accounts_state: self.accounts_state.pop(account_name) - + async def get_account_current_state(self, account_name: str) -> Dict[str, List[Dict]]: """ Get current state for a specific account from database. @@ -729,7 +729,7 @@ async def get_account_current_state(self, account_name: str) -> Dict[str, List[D logger.error(f"Error getting account current state: {e}") # Fallback to in-memory state return self.accounts_state.get(account_name, {}) - + async def get_account_state_history(self, account_name: str, limit: Optional[int] = None, @@ -762,7 +762,7 @@ async def get_account_state_history(self, except Exception as e: logger.error(f"Error getting account state history: {e}") return [], None, False - + async def get_connector_current_state(self, account_name: str, connector_name: str) -> List[Dict]: """ Get current state for a specific connector. @@ -775,10 +775,10 @@ async def get_connector_current_state(self, account_name: str, connector_name: s logger.error(f"Error getting connector current state: {e}") # Fallback to in-memory state return self.accounts_state.get(account_name, {}).get(connector_name, []) - - async def get_connector_state_history(self, - account_name: str, - connector_name: str, + + async def get_connector_state_history(self, + account_name: str, + connector_name: str, limit: Optional[int] = None, cursor: Optional[str] = None, start_time: Optional[datetime] = None, @@ -790,7 +790,7 @@ async def get_connector_state_history(self, async with self.db_manager.get_session_context() as session: repository = AccountRepository(session) return await repository.get_account_state_history( - account_name=account_name, + account_name=account_name, connector_name=connector_name, limit=limit, cursor=cursor, @@ -800,7 +800,7 @@ async def get_connector_state_history(self, except Exception as e: logger.error(f"Error getting connector state history: {e}") return [], None, False - + async def get_all_unique_tokens(self) -> List[str]: """ Get all unique tokens across all accounts and connectors. @@ -818,7 +818,7 @@ async def get_all_unique_tokens(self) -> List[str]: for token_info in connector_data: tokens.add(token_info.get("token")) return sorted(list(tokens)) - + async def get_token_current_state(self, token: str) -> List[Dict]: """ Get current state of a specific token across all accounts. @@ -830,7 +830,7 @@ async def get_token_current_state(self, token: str) -> List[Dict]: except Exception as e: logger.error(f"Error getting token current state: {e}") return [] - + async def get_portfolio_value(self, account_name: Optional[str] = None) -> Dict[str, Any]: """ Get total portfolio value, optionally filtered by account. @@ -843,9 +843,9 @@ async def get_portfolio_value(self, account_name: Optional[str] = None) -> Dict[ logger.error(f"Error getting portfolio value: {e}") # Fallback to in-memory calculation portfolio = {"accounts": {}, "total_value": 0} - + accounts_to_process = [account_name] if account_name else self.accounts_state.keys() - + for acc_name in accounts_to_process: account_value = 0 if acc_name in self.accounts_state: @@ -854,9 +854,9 @@ async def get_portfolio_value(self, account_name: Optional[str] = None) -> Dict[ account_value += token_info.get("value", 0) portfolio["accounts"][acc_name] = account_value portfolio["total_value"] += account_value - + return portfolio - + def get_portfolio_distribution(self, account_name: Optional[str] = None) -> Dict[str, Any]: """ Get portfolio distribution by tokens with percentages. @@ -872,8 +872,8 @@ def get_account_distribution(self) -> Dict[str, Any]: return self.portfolio_analytics_service.get_account_distribution(self.accounts_state) async def place_trade(self, account_name: str, connector_name: str, trading_pair: str, - trade_type: TradeType, amount: Decimal, order_type: OrderType = OrderType.LIMIT, - price: Optional[Decimal] = None, position_action: PositionAction = PositionAction.OPEN) -> str: + trade_type: TradeType, amount: Decimal, order_type: OrderType = OrderType.LIMIT, + price: Optional[Decimal] = None, position_action: PositionAction = PositionAction.OPEN) -> str: """ Place a trade using the specified account and connector. @@ -912,40 +912,37 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair # Check if trading rules are loaded if not connector.trading_rules: raise HTTPException( - status_code=503, + status_code=503, detail=f"Trading rules not yet loaded for {connector_name}. Please try again in a moment." ) - + # Validate trading pair and get trading rule if trading_pair not in connector.trading_rules: available_pairs = list(connector.trading_rules.keys())[:10] # Show first 10 more_text = f" (and {len(connector.trading_rules) - 10} more)" if len(connector.trading_rules) > 10 else "" raise HTTPException( - status_code=400, + status_code=400, detail=f"Trading pair '{trading_pair}' not supported on {connector_name}. " f"Available pairs: {available_pairs}{more_text}" ) - + trading_rule = connector.trading_rules[trading_pair] - + # Validate order type is supported if order_type not in connector.supported_order_types(): supported_types = [ot.name for ot in connector.supported_order_types()] - raise HTTPException( - status_code=400, - detail=f"Order type '{order_type.name}' not supported. Supported types: {supported_types}") - + raise HTTPException(status_code=400, detail=f"Order type '{order_type.name}' not supported. Supported types: {supported_types}") + # Quantize amount according to trading rules quantized_amount = connector.quantize_order_amount(trading_pair, amount) - + # Validate minimum order size if quantized_amount < trading_rule.min_order_size: raise HTTPException( - status_code=400, - detail=f"Order amount {quantized_amount} is below minimum order size " - f"{trading_rule.min_order_size} for {trading_pair}" + status_code=400, + detail=f"Order amount {quantized_amount} is below minimum order size {trading_rule.min_order_size} for {trading_pair}" ) - + # Calculate and validate notional size if order_type in [OrderType.LIMIT, OrderType.LIMIT_MAKER]: quantized_price = connector.quantize_order_price(trading_pair, price) @@ -959,14 +956,15 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair except Exception as e: logger.error(f"Error getting market price for {trading_pair}: {e}") notional_size = price * quantized_amount if price else Decimal("0") - + if notional_size < trading_rule.min_notional_size: raise HTTPException( status_code=400, - detail=f"Order notional value {notional_size} is below minimum notional size " - f"{trading_rule.min_notional_size} for {trading_pair}. " + detail=f"Order notional value {notional_size} is below minimum notional size {trading_rule.min_notional_size} for {trading_pair}. " f"Increase the amount or price to meet the minimum requirement." ) + + try: # Place the order using the connector with quantized values @@ -988,17 +986,16 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair position_action=position_action ) - logger.info(f"Placed {trade_type} order for {amount} {trading_pair} on {connector_name} " - f"(Account: {account_name}). Order ID: {order_id}") + logger.info(f"Placed {trade_type} order for {amount} {trading_pair} on {connector_name} (Account: {account_name}). Order ID: {order_id}") return order_id - + except HTTPException: # Re-raise HTTP exceptions as-is raise except Exception as e: logger.error(f"Failed to place {trade_type} order: {e}") raise HTTPException(status_code=500, detail=f"Failed to place trade: {str(e)}") - + async def get_connector_instance(self, account_name: str, connector_name: str): """ Get a connector instance for direct access. @@ -1021,38 +1018,38 @@ async def get_connector_instance(self, account_name: str, connector_name: str): async def get_active_orders(self, account_name: str, connector_name: str) -> Dict[str, Any]: """ Get active orders for a specific connector. - + Args: account_name: Name of the account connector_name: Name of the connector - + Returns: Dictionary of active orders """ connector = await self.get_connector_instance(account_name, connector_name) return {order_id: order.to_json() for order_id, order in connector.in_flight_orders.items()} - + async def cancel_order(self, account_name: str, connector_name: str, client_order_id: str) -> str: """ Cancel an active order. - + Args: account_name: Name of the account connector_name: Name of the connector client_order_id: Client order ID to cancel - + Returns: Client order ID that was cancelled - + Raises: HTTPException: 404 if order not found, 500 if cancellation fails """ connector = await self.get_connector_instance(account_name, connector_name) - + # Check if order exists in in-flight orders if client_order_id not in connector.in_flight_orders: raise HTTPException(status_code=404, detail=f"Order '{client_order_id}' not found in active orders") - + try: result = connector.cancel(trading_pair="NA", client_order_id=client_order_id) logger.info(f"Initiated cancellation for order {client_order_id} on {connector_name} (Account: {account_name})") @@ -1060,9 +1057,9 @@ async def cancel_order(self, account_name: str, connector_name: str, client_orde except Exception as e: logger.error(f"Failed to initiate cancellation for order {client_order_id}: {e}") raise HTTPException(status_code=500, detail=f"Failed to initiate order cancellation: {str(e)}") - + async def set_leverage(self, account_name: str, connector_name: str, - trading_pair: str, leverage: int) -> Dict[str, str]: + trading_pair: str, leverage: int) -> Dict[str, str]: """ Set leverage for a specific trading pair on a perpetual connector. Delegates to PerpetualTradingService. @@ -1070,7 +1067,7 @@ async def set_leverage(self, account_name: str, connector_name: str, return await self.perpetual_trading_service.set_leverage(account_name, connector_name, trading_pair, leverage) async def set_position_mode(self, account_name: str, connector_name: str, - position_mode: PositionMode) -> Dict[str, str]: + position_mode: PositionMode) -> Dict[str, str]: """ Set position mode for a perpetual connector. Delegates to PerpetualTradingService. diff --git a/services/trading_service.py b/services/trading_service.py index 007dcdb9..f788fc77 100644 --- a/services/trading_service.py +++ b/services/trading_service.py @@ -128,6 +128,17 @@ async def add_market( """ await self.ensure_connector(connector_name) + connector = self.connectors.get(connector_name) + if not connector: + raise ValueError(f"Connector {connector_name} not available. Check credentials.") + + # Sync pair-derived state (throttler limits, per-pair trading rules) BEFORE + # the early return below: a no-op when already synced, and it retries a + # previously failed sync even when the market is already tracked with a + # healthy order book — otherwise a transient failure (e.g. node outage + # during the rules fetch) would stick until restart (#207/#208). + await self._register_trading_pair_with_connector(connector, trading_pair) + if connector_name not in self._markets: self._markets[connector_name] = set() @@ -151,11 +162,6 @@ async def add_market( self._markets[connector_name].add(trading_pair) - # Get connector from our account's connectors - connector = self.connectors.get(connector_name) - if not connector: - raise ValueError(f"Connector {connector_name} not available. Check credentials.") - # Initialize order book via MarketDataService (uses best available connector) logger.info(f"Initializing order book for {connector_name}/{trading_pair}") success = await self._market_data_service.initialize_order_book( @@ -170,9 +176,6 @@ async def add_market( logger.info(f"Order book initialized successfully for {connector_name}/{trading_pair}") - # Register trading pair with connector - await self._register_trading_pair_with_connector(connector, trading_pair) - # Update balances to include tokens from new trading pair if hasattr(connector, '_update_balances'): try: From 13517982d2c95173c8d88d9cd3d0216c4ea85dd7 Mon Sep 17 00:00:00 2001 From: Michael Feng Date: Thu, 6 Aug 2026 23:13:51 -0700 Subject: [PATCH 4/4] (fix) set_position_mode: register pair and fail loudly instead of silent no-op Position-mode implementations apply the switch through the connector's trading pairs (the py-base default uses trading_pairs[0]; bybit loops over all of them) and both log a warning and return when the list is empty. With API connectors created as trading_pairs=[], the endpoint reported success while the exchange was never called. - PositionModeRequest gains optional trading_pair (backwards compatible) - The pair is registered via sync_pair_derived_state before switching (also picks up pair-templated throttler limits, e.g. bybit switch-mode) - With no pair provided and none registered, the service now returns 400 with instructions instead of a false success flake8 skipped: remaining violations in routers/trading.py are pre-existing on main. Co-Authored-By: Claude Fable 5 --- models/accounts.py | 10 ++++-- routers/trading.py | 3 +- services/accounts_service.py | 6 ++-- services/perpetual_trading_service.py | 24 ++++++++++++-- test/test_sync_pair_derived_state.py | 47 +++++++++++++++++++++++++++ 5 files changed, 83 insertions(+), 7 deletions(-) diff --git a/models/accounts.py b/models/accounts.py index 4f7b8c2a..33eeb20e 100644 --- a/models/accounts.py +++ b/models/accounts.py @@ -1,5 +1,6 @@ +from typing import Any, Dict, Optional + from pydantic import BaseModel, Field -from typing import Dict, Any class LeverageRequest(BaseModel): @@ -11,8 +12,13 @@ class LeverageRequest(BaseModel): class PositionModeRequest(BaseModel): """Request model for setting position mode on perpetual connectors""" position_mode: str = Field(description="Position mode (HEDGE or ONEWAY)") + trading_pair: Optional[str] = Field( + default=None, + description="Pair to register on the connector before switching. Position-mode " + "implementations apply the switch through the connector's trading " + "pairs, so at least one registered pair is required.") class CredentialRequest(BaseModel): """Request model for adding connector credentials""" - credentials: Dict[str, Any] = Field(description="Connector credentials dictionary") \ No newline at end of file + credentials: Dict[str, Any] = Field(description="Connector credentials dictionary") diff --git a/routers/trading.py b/routers/trading.py index 4b6df289..2d882da4 100644 --- a/routers/trading.py +++ b/routers/trading.py @@ -424,7 +424,8 @@ async def set_position_mode( try: # Convert string to PositionMode enum mode = PositionMode[request.position_mode.upper()] - result = await accounts_service.set_position_mode(account_name, connector_name, mode) + result = await accounts_service.set_position_mode( + account_name, connector_name, mode, trading_pair=request.trading_pair) return result except KeyError: raise HTTPException( diff --git a/services/accounts_service.py b/services/accounts_service.py index 884591a6..59b12a55 100644 --- a/services/accounts_service.py +++ b/services/accounts_service.py @@ -1067,12 +1067,14 @@ async def set_leverage(self, account_name: str, connector_name: str, return await self.perpetual_trading_service.set_leverage(account_name, connector_name, trading_pair, leverage) async def set_position_mode(self, account_name: str, connector_name: str, - position_mode: PositionMode) -> Dict[str, str]: + position_mode: PositionMode, + trading_pair: Optional[str] = None) -> Dict[str, str]: """ Set position mode for a perpetual connector. Delegates to PerpetualTradingService. """ - return await self.perpetual_trading_service.set_position_mode(account_name, connector_name, position_mode) + return await self.perpetual_trading_service.set_position_mode( + account_name, connector_name, position_mode, trading_pair=trading_pair) async def get_position_mode(self, account_name: str, connector_name: str) -> Dict[str, str]: """ diff --git a/services/perpetual_trading_service.py b/services/perpetual_trading_service.py index da757f8e..a903e9bd 100644 --- a/services/perpetual_trading_service.py +++ b/services/perpetual_trading_service.py @@ -1,6 +1,6 @@ import asyncio import logging -from typing import Any, Awaitable, Callable, Dict, List +from typing import Any, Awaitable, Callable, Dict, List, Optional from fastapi import HTTPException from hummingbot.core.data_type.common import PositionMode @@ -83,7 +83,8 @@ async def set_leverage(self, account_name: str, connector_name: str, raise HTTPException(status_code=500, detail=f"Failed to set leverage: {str(e)}") async def set_position_mode(self, account_name: str, connector_name: str, - position_mode: PositionMode) -> Dict[str, str]: + position_mode: PositionMode, + trading_pair: Optional[str] = None) -> Dict[str, str]: """ Set position mode for a perpetual connector. @@ -91,6 +92,9 @@ async def set_position_mode(self, account_name: str, connector_name: str, account_name: Name of the account connector_name: Name of the connector (must be perpetual) position_mode: PositionMode.HEDGE or PositionMode.ONEWAY + trading_pair: Pair to register before switching. Position-mode + implementations apply the switch through the connector's trading + pairs, so at least one registered pair is required. Returns: Dictionary with success status and message @@ -109,6 +113,22 @@ async def set_position_mode(self, account_name: str, connector_name: str, detail=f"Position mode '{position_mode.value}' not supported. Supported modes: {supported_values}" ) + # Position-mode implementations apply the switch through the connector's + # trading pairs (the py-base default and e.g. bybit both log a warning and + # return when the list is empty), and API connectors are created with + # trading_pairs=[] — so without a registered pair the exchange is never + # called while this endpoint would report success. Register the provided + # pair, then refuse to proceed with an empty list rather than lie. + if trading_pair: + from services.unified_connector_service import UnifiedConnectorService + await UnifiedConnectorService.sync_pair_derived_state(connector, trading_pair) + if not getattr(connector, "trading_pairs", None): + raise HTTPException( + status_code=400, + detail=f"No trading pairs registered on {connector_name}; pass trading_pair " + f"so the position mode switch can be applied on the exchange" + ) + try: # Try to call the method - it might be sync or async result = connector.set_position_mode(position_mode) diff --git a/test/test_sync_pair_derived_state.py b/test/test_sync_pair_derived_state.py index 5a2e9663..326b1c27 100644 --- a/test/test_sync_pair_derived_state.py +++ b/test/test_sync_pair_derived_state.py @@ -114,6 +114,53 @@ class Minimal: assert minimal._trading_pairs == ["BTC-USDT"] +def test_set_position_mode_refuses_empty_pairs_and_registers_provided_pair(): + """Position-mode implementations iterate connector.trading_pairs and silently + no-op when the list is empty — the service must refuse loudly instead of + reporting success, and must register a provided pair before switching.""" + from fastapi import HTTPException + from hummingbot.core.data_type.common import PositionMode + + from services.perpetual_trading_service import PerpetualTradingService + + class FakePerpConnector(FakePerPairRulesConnector): + def __init__(self): + super().__init__(trading_pairs=[]) + self.mode_set = None + + @property + def trading_pairs(self): + return self._trading_pairs or [] + + def supported_position_modes(self): + return [PositionMode.HEDGE, PositionMode.ONEWAY] + + def set_position_mode(self, mode): + self.mode_set = mode + + connector = FakePerpConnector() + + async def provider(account_name, connector_name): + return connector + + service = PerpetualTradingService(provider) + + # No pair provided and none registered -> loud 400, exchange never touched + try: + _run(service.set_position_mode("master", "bybit_perpetual", PositionMode.HEDGE)) + raise AssertionError("expected HTTPException for empty trading_pairs") + except HTTPException as e: + assert e.status_code == 400 + assert connector.mode_set is None + + # Pair provided -> registered (with rules/throttler synced), then switched + result = _run(service.set_position_mode( + "master", "bybit_perpetual", PositionMode.HEDGE, trading_pair="BTC-USDT")) + assert connector._trading_pairs == ["BTC-USDT"] + assert connector.mode_set == PositionMode.HEDGE + assert result["status"] == "success" + + def test_rules_fetch_failure_is_swallowed(): connector = FakePerPairRulesConnector(trading_pairs=[])