diff --git a/models/accounts.py b/models/accounts.py index 4f7b8c2a..a5ed7f57 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,12 @@ 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="Optional. Only for exchanges whose position mode is not account-wide: selects " + "the product type on bitget (USDT by default) or the symbol on bybit.") 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 f622a4b9..f32e0ae7 100644 --- a/routers/trading.py +++ b/routers/trading.py @@ -424,17 +424,20 @@ async def set_position_mode( account_name: Name of the account connector_name: Name of the perpetual connector position_mode: Position mode to set (HEDGE or ONEWAY) + trading_pair: Optional scope where the mode is not account-wide (bitget product type, bybit symbol) Returns: Success message with status Raises: - HTTPException: 400 if not a perpetual connector or invalid position mode + HTTPException: 400 if not a perpetual connector, invalid position mode or unknown + trading pair; 502 if the exchange did not accept the switch """ 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 e1ce570f..0e38216f 100644 --- a/services/accounts_service.py +++ b/services/accounts_service.py @@ -921,7 +921,7 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair ) 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()] @@ -1061,12 +1061,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 b0b24485..7f18549f 100644 --- a/services/perpetual_trading_service.py +++ b/services/perpetual_trading_service.py @@ -1,14 +1,69 @@ -import asyncio import logging -from typing import Any, Awaitable, Callable, Dict, List +from contextlib import asynccontextmanager +from typing import Any, Awaitable, Callable, Dict, List, Optional from fastapi import HTTPException +from hummingbot.connector.connector_base import ConnectorBase +from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase from hummingbot.core.data_type.common import PositionMode # Create module-specific logger logger = logging.getLogger(__name__) +def _reads_position_mode(connector: PerpetualDerivativePyBase) -> bool: + """Whether the connector can ask the exchange for the account's position mode.""" + return (type(connector)._fetch_account_position_mode + is not PerpetualDerivativePyBase._fetch_account_position_mode) + + +def _switches_pair_by_pair(connector: PerpetualDerivativePyBase) -> bool: + """Whether the mode lives on each symbol rather than on the account (bybit). + + Such a connector loops over its registered pairs to switch and cannot report a mode, + so there is no account-level default the API could address on its own. + """ + return (type(connector)._execute_set_position_mode + is not PerpetualDerivativePyBase._execute_set_position_mode + and not _reads_position_mode(connector)) + + +@asynccontextmanager +async def _exchange_context(connector: PerpetualDerivativePyBase, trading_pair: Optional[str] = None): + """Lend the connector a listed pair for the duration of an account-level call. + + hummingbot addresses the account through the connector's registered pairs: the base + switch returns early with none, and bitget derives its product type from the first one. + The API creates connectors with no pairs, so it supplies one here and takes it back, + rather than asking the caller for it. The pair list is shared with the order book + tracker, so the pair must not stay registered without an order book. + + With no trading_pair given, a USDT pair is borrowed only if none is registered. + """ + pairs = connector._trading_pairs + pair = trading_pair + if pair is None and not pairs: + rules = connector.trading_rules + pair = next((p for p in rules if p.endswith("-USDT")), next(iter(rules), None)) + borrowed = pair is not None and pair not in pairs + if borrowed: + pairs.insert(0, pair) + try: + yield + finally: + if borrowed and pair in pairs: + pairs.remove(pair) + + +async def sync_position_mode(connector: ConnectorBase) -> None: + """Adopt the exchange's position mode as the connector's local mode, if it can be read.""" + if not isinstance(connector, PerpetualDerivativePyBase) or not _reads_position_mode(connector): + return + async with _exchange_context(connector): + if len(connector.supported_position_modes()) > 1: + await connector._initialize_position_mode() + + class PerpetualTradingService: """ Perpetual-specific trading operations: leverage, position mode and position queries. @@ -77,7 +132,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. @@ -85,38 +141,65 @@ 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: Optional scope for exchanges whose mode is not account-wide: the + product type on bitget (USDT by default), the symbol on bybit. Never registered. Returns: Dictionary with success status and message Raises: - HTTPException: If account/connector not found, not perpetual, or operation fails + HTTPException: 400 if the pair is unknown, the mode is unsupported or the exchange + switches per symbol and none is given; 502 if the exchange did not accept the switch """ connector = await self._get_perpetual_connector(account_name, connector_name) - # Check if the requested position mode is supported - supported_modes = connector.supported_position_modes() - if position_mode not in supported_modes: - supported_values = [mode.value for mode in supported_modes] + if trading_pair and trading_pair not in connector.trading_pairs: + try: + await connector.exchange_symbol_associated_to_pair(trading_pair=trading_pair) + except KeyError: + raise HTTPException( + status_code=400, + detail=f"Trading pair '{trading_pair}' is not listed on {connector_name}" + ) + + if not trading_pair and not connector.trading_pairs and _switches_pair_by_pair(connector): raise HTTPException( status_code=400, - detail=f"Position mode '{position_mode.value}' not supported. Supported modes: {supported_values}" + detail=f"{connector_name} sets the position mode per symbol; pass trading_pair " + f"to say which one to switch" ) - try: - # Try to call the method - it might be sync or async - result = connector.set_position_mode(position_mode) - # If it's a coroutine, await it - if asyncio.iscoroutine(result): - await result - - message = f"Position mode set to {position_mode.value} on {connector_name}" - logger.info(f"Set position mode to {position_mode.value} on {connector_name} (Account: {account_name})") - return {"status": "success", "message": message} + async with _exchange_context(connector, trading_pair): + # Validated with the pair in place: bybit's supported_position_modes() depends on + # the pairs and returns both modes when there are none. + supported_modes = connector.supported_position_modes() + if position_mode not in supported_modes: + supported_values = [mode.value for mode in supported_modes] + raise HTTPException( + status_code=400, + detail=f"Position mode '{position_mode.value}' not supported. Supported modes: {supported_values}" + ) + + try: + # connector.set_position_mode() runs this in the background and cannot report a + # rejection (e.g. Binance -4068 with open positions). Awaiting it directly does, + # since the local mode only changes once the exchange confirms. + await connector._execute_set_position_mode(position_mode) + except Exception as e: + logger.error(f"Failed to set position mode to {position_mode.value}: {e}") + raise HTTPException(status_code=500, detail=f"Failed to set position mode: {str(e)}") + + if connector.position_mode != position_mode: + raise HTTPException( + status_code=502, + detail=f"{connector_name} did not accept position mode {position_mode.value} " + f"(still {connector.position_mode.value}); check for open positions or orders " + f"and the connector logs" + ) - except Exception as e: - logger.error(f"Failed to set position mode to {position_mode.value}: {e}") - raise HTTPException(status_code=500, detail=f"Failed to set position mode: {str(e)}") + message = f"Position mode set to {position_mode.value} on {connector_name}" + logger.info(f"Set position mode to {position_mode.value} on {connector_name} (Account: {account_name})") + return {"status": "success", "message": message} async def get_position_mode(self, account_name: str, connector_name: str) -> Dict[str, str]: """ @@ -138,6 +221,8 @@ async def get_position_mode(self, account_name: str, connector_name: str) -> Dic raise HTTPException(status_code=400, detail=f"Connector '{connector_name}' does not support position mode") try: + # Report the exchange's mode, not a cached one: it may have been changed elsewhere + await sync_position_mode(connector) current_mode = connector.position_mode return { "position_mode": current_mode.value if current_mode else "UNKNOWN", diff --git a/services/trading_service.py b/services/trading_service.py index 11041d61..4620157f 100644 --- a/services/trading_service.py +++ b/services/trading_service.py @@ -12,6 +12,8 @@ from hummingbot.connector.connector_base import ConnectorBase from hummingbot.core.data_type.common import OrderType, PositionAction +from services.perpetual_trading_service import sync_position_mode + if TYPE_CHECKING: from services.market_data_service import MarketDataService from services.unified_connector_service import UnifiedConnectorService @@ -173,6 +175,9 @@ async def add_market( # Register trading pair with connector self._register_trading_pair_with_connector(connector, trading_pair) + # Cheap re-read: picks up a mode changed on the exchange, or a read that failed at init + await sync_position_mode(connector) + # Update balances to include tokens from new trading pair if hasattr(connector, '_update_balances'): try: diff --git a/services/unified_connector_service.py b/services/unified_connector_service.py index 403e7d48..02ab4753 100644 --- a/services/unified_connector_service.py +++ b/services/unified_connector_service.py @@ -24,10 +24,11 @@ from hummingbot.connector.exchange_py_base import ExchangePyBase from hummingbot.connector.gateway.gateway import Gateway from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase -from hummingbot.core.data_type.common import OrderType, PositionAction, PositionMode, TradeType +from hummingbot.core.data_type.common import OrderType, PositionAction, TradeType from hummingbot.core.data_type.in_flight_order import InFlightOrder, OrderState from hummingbot.core.utils.async_utils import safe_ensure_future +from services.perpetual_trading_service import sync_position_mode from utils.file_system import fs_util from utils.hummingbot_api_config_adapter import HummingbotAPIConfigAdapter from utils.security import BackendAPISecurity @@ -684,8 +685,11 @@ async def _create_and_initialize_trading_connector( # Perpetual-specific setup if self._is_perpetual_connector(connector): - if PositionMode.HEDGE in connector.supported_position_modes(): - connector.set_position_mode(PositionMode.HEDGE) + # Adopt the exchange's current mode rather than forcing one: with no pairs + # registered yet a switch cannot reach the exchange, and bybit/bitget would + # only flip the local mode to HEDGE. start_network() does the same, but the API + # runs the polling tasks itself and never calls it for CEX connectors. + await sync_position_mode(connector) await connector._update_positions() # Load existing orders from database diff --git a/test/test_position_mode_sync.py b/test/test_position_mode_sync.py new file mode 100644 index 00000000..4bb28eac --- /dev/null +++ b/test/test_position_mode_sync.py @@ -0,0 +1,265 @@ +"""The API's position mode is the exchange's, and a switch it reports is one the exchange made. + +The API creates perpetual connectors with no trading pairs. With none registered, a +position-mode switch cannot reach the exchange: bitget's and bybit's implementations loop +over the empty pair list and flip only the local mode, and bitget cannot even read the +account's mode until it has a pair to query with. So the API used to report HEDGE on +one-way bitget accounts, the switch endpoint reported success for switches that never +happened, and orders went out with the wrong position side. #210. + +hummingbot reaches the account through the connector's registered pairs, so the API lends +the connector a listed pair for the call and takes it back; the caller is not asked for one. + +These drive the real connectors with only their HTTP layer replaced by a fake account. +""" +import asyncio +from decimal import Decimal +from unittest.mock import AsyncMock, MagicMock + +import pytest +from bidict import bidict +from fastapi import HTTPException +from hummingbot.connector.derivative.binance_perpetual.binance_perpetual_derivative import BinancePerpetualDerivative +from hummingbot.connector.derivative.bitget_perpetual import bitget_perpetual_constants as CONSTANTS +from hummingbot.connector.derivative.bitget_perpetual.bitget_perpetual_derivative import BitgetPerpetualDerivative +from hummingbot.connector.derivative.bybit_perpetual.bybit_perpetual_derivative import BybitPerpetualDerivative +from hummingbot.connector.trading_rule import TradingRule +from hummingbot.core.data_type.common import OrderType, PositionAction, PositionMode, TradeType + +from services.accounts_service import AccountsService +from services.perpetual_trading_service import PerpetualTradingService, sync_position_mode +from services.trading_service import AccountTradingInterface + + +class FakeBitgetAccount: + def __init__(self, pos_mode: str, accept_switch: bool = True): + self.pos_mode = pos_mode + self.accept_switch = accept_switch + self.switch_requests = [] + self.reads = [] + self.orders = [] + + async def get(self, path_url, params=None, is_auth_required=False, **kwargs): + assert path_url == CONSTANTS.ACCOUNT_INFO_ENDPOINT + self.reads.append(params["productType"]) + return {"code": CONSTANTS.RET_CODE_OK, "data": {"posMode": self.pos_mode}} + + async def post(self, path_url, data=None, is_auth_required=False, **kwargs): + if path_url == CONSTANTS.PLACE_ORDER_ENDPOINT: + self.orders.append(data) + return {"code": CONSTANTS.RET_CODE_OK, "data": {"orderId": "1"}} + assert path_url == CONSTANTS.SET_POSITION_MODE_ENDPOINT + self.switch_requests.append(data["posMode"]) + if not self.accept_switch: + return {"code": "40920", "msg": "position exists"} + self.pos_mode = data["posMode"] + return {"code": CONSTANTS.RET_CODE_OK, "data": {}} + + +def _rule(trading_pair: str) -> TradingRule: + return TradingRule( + trading_pair, min_order_size=Decimal("0.001"), min_price_increment=Decimal("0.1"), + min_base_amount_increment=Decimal("0.001"), min_notional_size=Decimal("5")) + + +def _bitget(account: FakeBitgetAccount) -> BitgetPerpetualDerivative: + """A connector as the API creates it: symbols and rules loaded, no pair registered.""" + connector = BitgetPerpetualDerivative( + bitget_perpetual_api_key="k", + bitget_perpetual_secret_key="s", + bitget_perpetual_passphrase="p", + trading_pairs=[], + ) + connector._set_trading_pair_symbol_map(bidict({"BTCUSDT": "BTC-USDT", "BTCPERP": "BTC-USDC"})) + connector._trading_rules.update({"BTC-USDC": _rule("BTC-USDC"), "BTC-USDT": _rule("BTC-USDT")}) + connector._api_get = account.get + connector._api_post = account.post + return connector + + +def _service(connector) -> PerpetualTradingService: + async def provider(account_name, connector_name): + return connector + return PerpetualTradingService(provider) + + +def _no_pair_left_behind(connector) -> bool: + # The order book tracker subscribes to this same list + return connector.trading_pairs == [] and connector.order_book_tracker._trading_pairs == [] + + +async def test_hedge_account_is_adopted_with_no_pair_registered(): + account = FakeBitgetAccount("hedge_mode") + connector = _bitget(account) + + await sync_position_mode(connector) + + assert connector.position_mode == PositionMode.HEDGE + assert account.reads == ["USDT-FUTURES"] + assert account.switch_requests == [] + assert _no_pair_left_behind(connector) + + +async def test_direct_order_on_a_fresh_connector_goes_out_in_the_accounts_hedge_mode(): + account = FakeBitgetAccount("hedge_mode") + connector = _bitget(account) + await sync_position_mode(connector) # as connector creation does + service = AccountsService.__new__(AccountsService) + service.list_accounts = lambda: ["master"] + service._connector_service = MagicMock() + service._connector_service.get_trading_connector = AsyncMock(return_value=connector) + + await service.place_trade("master", "bitget_perpetual", "BTC-USDT", TradeType.BUY, Decimal("0.01"), + OrderType.LIMIT, Decimal("60000"), PositionAction.OPEN) + for _ in range(100): + if account.orders: + break + await asyncio.sleep(0.01) + + assert account.orders[0]["tradeSide"] == "open" + assert _no_pair_left_behind(connector) + + +async def test_adding_a_market_reads_the_mode_again_after_a_read_that_failed(): + account = FakeBitgetAccount("hedge_mode") + connector = _bitget(account) + connector._api_get = AsyncMock(side_effect=IOError("timeout")) + await sync_position_mode(connector) + assert connector.position_mode == PositionMode.ONEWAY + connector._api_get = account.get + + async def initialize_order_book(connector_name, trading_pair, **kwargs): + # The tracker's pair list is the connector's own, so the order book registers the pair + connector.order_book_tracker._trading_pairs.append(trading_pair) + return True + market_data = MagicMock() + market_data.initialize_order_book = initialize_order_book + connector_service = MagicMock() + connector_service.get_trading_connector = AsyncMock(return_value=connector) + connector_service.get_account_connectors.return_value = {"bitget_perpetual": connector} + interface = AccountTradingInterface(connector_service, market_data, "master") + + await interface.add_market("bitget_perpetual", "BTC-USDT") + + assert connector.position_mode == PositionMode.HEDGE + assert connector.trading_pairs == ["BTC-USDT"] + + +async def test_get_reports_the_exchanges_mode_not_a_cached_one(): + account = FakeBitgetAccount("one_way_mode") + connector = _bitget(account) + await sync_position_mode(connector) + account.pos_mode = "hedge_mode" # switched in the exchange UI + + result = await _service(connector).get_position_mode("master", "bitget_perpetual") + + assert result["position_mode"] == "HEDGE" + assert _no_pair_left_behind(connector) + + +async def test_switch_reaches_the_exchange_with_no_pair_registered_or_given(): + account = FakeBitgetAccount("one_way_mode") + connector = _bitget(account) + + result = await _service(connector).set_position_mode("master", "bitget_perpetual", PositionMode.HEDGE) + + assert result["status"] == "success" + assert account.pos_mode == "hedge_mode" + assert connector.position_mode == PositionMode.HEDGE + assert _no_pair_left_behind(connector) + + +async def test_a_given_pair_selects_the_product_type_and_is_not_registered(): + account = FakeBitgetAccount("one_way_mode") + connector = _bitget(account) + + await _service(connector).set_position_mode( + "master", "bitget_perpetual", PositionMode.HEDGE, trading_pair="BTC-USDC") + + assert account.reads == ["USDC-FUTURES"] + assert account.switch_requests == ["hedge_mode"] + assert _no_pair_left_behind(connector) + + +async def test_switch_with_an_unlisted_pair_is_refused(): + account = FakeBitgetAccount("one_way_mode") + connector = _bitget(account) + + with pytest.raises(HTTPException) as e: + await _service(connector).set_position_mode( + "master", "bitget_perpetual", PositionMode.HEDGE, trading_pair="NOPE-USDT") + + assert e.value.status_code == 400 + assert account.switch_requests == [] + assert _no_pair_left_behind(connector) + + +async def test_switch_the_exchange_rejects_is_reported_as_rejected(): + account = FakeBitgetAccount("one_way_mode", accept_switch=False) + connector = _bitget(account) + + with pytest.raises(HTTPException) as e: + await _service(connector).set_position_mode("master", "bitget_perpetual", PositionMode.HEDGE) + + assert e.value.status_code == 502 + assert account.switch_requests == ["hedge_mode"] + assert connector.position_mode == PositionMode.ONEWAY + assert _no_pair_left_behind(connector) + + +async def test_account_wide_exchange_switches_with_no_pair(): + connector = BinancePerpetualDerivative( + binance_perpetual_api_key="k", binance_perpetual_api_secret="s", trading_pairs=[]) + connector._trading_rules["BTC-USDT"] = _rule("BTC-USDT") + exchange = {"dualSidePosition": False} + + async def get(path_url, **kwargs): + return dict(exchange) + + async def post(path_url, data=None, **kwargs): + exchange.update(data) + return {"msg": "success", "code": 200} + connector._api_get, connector._api_post = get, post + + await _service(connector).set_position_mode("master", "binance_perpetual", PositionMode.HEDGE) + + assert exchange["dualSidePosition"] is True + assert connector.position_mode == PositionMode.HEDGE + assert _no_pair_left_behind(connector) + + +def _bybit(switched: list) -> BybitPerpetualDerivative: + connector = BybitPerpetualDerivative( + bybit_perpetual_api_key="k", bybit_perpetual_secret_key="s", trading_pairs=[]) + connector._set_trading_pair_symbol_map(bidict({"BTCUSDT": "BTC-USDT"})) + connector._trading_rules["BTC-USDT"] = _rule("BTC-USDT") + + async def post(path_url, data=None, **kwargs): + switched.append(data["symbol"]) + return {"retCode": 0, "retMsg": "OK"} + connector._api_post = post + return connector + + +async def test_per_symbol_exchange_is_not_switched_through_a_symbol_nobody_named(): + switched = [] + connector = _bybit(switched) + + with pytest.raises(HTTPException) as e: + await _service(connector).set_position_mode("master", "bybit_perpetual", PositionMode.HEDGE) + + assert e.value.status_code == 400 + assert switched == [] + assert connector.position_mode == PositionMode.ONEWAY + + +async def test_per_symbol_exchange_switches_the_symbol_it_is_given(): + switched = [] + connector = _bybit(switched) + + await _service(connector).set_position_mode( + "master", "bybit_perpetual", PositionMode.HEDGE, trading_pair="BTC-USDT") + + assert switched == ["BTCUSDT"] + assert connector.position_mode == PositionMode.HEDGE + assert _no_pair_left_behind(connector)