diff --git a/main.py b/main.py index a7d93ab2..af674e16 100644 --- a/main.py +++ b/main.py @@ -83,10 +83,12 @@ def patched_save_to_yml(yml_path, cm): from utils.security import BackendAPISecurity # noqa: E402 from utils.validation_errors import validation_exception_handler # noqa: E402 -# Set up logging configuration +# Set up logging configuration. force: an imported dependency (pyinjective) has already +# configured the root logger, which would otherwise leave it at WARNING logging.basicConfig( level=logging.INFO, - format='%(asctime)s - %(name)s - %(levelname)s - %(message)s' + format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', + force=True ) # Enable info logging for MQTT manager diff --git a/models/accounts.py b/models/accounts.py index 4f7b8c2a..edf1c318 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="Pair to register on the connector before switching. Connectors apply the " + "switch through their registered pairs, so one is required if none is registered yet.") 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..32314a87 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: Pair to register first, required if the connector has none registered 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 no registered + 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..b1fd488b 100644 --- a/services/accounts_service.py +++ b/services/accounts_service.py @@ -7,6 +7,7 @@ from fastapi import HTTPException from hummingbot.client.config.config_crypt import ETHKeyFileSecretManger +from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase from hummingbot.core.data_type.common import OrderType, PositionAction, PositionMode, TradeType from config import settings @@ -14,7 +15,7 @@ from services.gateway_client import GatewayClient from services.gateway_transaction_poller import GatewayTransactionPoller from services.gateway_wallet_service import GatewayWalletService, balance_entry -from services.perpetual_trading_service import PerpetualTradingService +from services.perpetual_trading_service import PerpetualTradingService, register_trading_pair from services.portfolio_analytics_service import PortfolioAnalyticsService from utils.file_system import fs_util from utils.gateway_certs import build_client_ssl_context @@ -921,7 +922,12 @@ async def place_trade(self, account_name: str, connector_name: str, trading_pair ) trading_rule = connector.trading_rules[trading_pair] - + + # A perpetual order goes out under the connector's position mode, which some + # connectors can only read from the account once a pair is registered (bitget). + if isinstance(connector, PerpetualDerivativePyBase): + await register_trading_pair(connector, 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 +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 b0b24485..5e0f8634 100644 --- a/services/perpetual_trading_service.py +++ b/services/perpetual_trading_service.py @@ -1,14 +1,35 @@ -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.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__) +async def register_trading_pair(connector: ConnectorBase, trading_pair: str, + first_pair: Optional[bool] = None) -> None: + """Register a pair on a connector, reading the account's position mode on a perpetual's first pair. + + Some connectors can only read the account's position mode once a pair is registered + (bitget queries it per pair), so the read at connector init found nothing and the local + mode sits at the ONEWAY default. Read it again before any order is placed under it. + + A caller that has already set up the pair's order book passes first_pair as it was before: + the order book tracker shares the connector's pair list, so the pair is already in it. + """ + if first_pair is None: + first_pair = not connector._trading_pairs + if trading_pair not in connector._trading_pairs: + connector._trading_pairs.append(trading_pair) + if (first_pair and isinstance(connector, PerpetualDerivativePyBase) + and 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 +98,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,16 +107,39 @@ 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 on the connector first. Connectors apply the + switch through their registered pairs, so at least one is required. Returns: Dictionary with success status and message Raises: - HTTPException: If account/connector not found, not perpetual, or operation fails + HTTPException: 400 if no pair is registered, the pair is unknown or the mode is + unsupported; 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 + # Register the pair before validating: bybit's supported_position_modes() depends on + # the registered pairs and returns both modes when there are none. + 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}" + ) + await register_trading_pair(connector, trading_pair) + + # With no registered pair the connector cannot apply the switch: the base + # implementation logs a warning and returns, bybit/bitget flip only the local mode. + if not connector.trading_pairs: + 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" + ) + supported_modes = connector.supported_position_modes() if position_mode not in supported_modes: supported_values = [mode.value for mode in supported_modes] @@ -104,20 +149,26 @@ async def set_position_mode(self, account_name: str, connector_name: str, ) 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} - + # 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" + ) + + 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]: """ Get current position mode for a perpetual connector. diff --git a/services/trading_service.py b/services/trading_service.py index 11041d61..9527d4bb 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 register_trading_pair + if TYPE_CHECKING: from services.market_data_service import MarketDataService from services.unified_connector_service import UnifiedConnectorService @@ -156,6 +158,10 @@ async def add_market( if not connector: raise ValueError(f"Connector {connector_name} not available. Check credentials.") + # The order book tracker shares the connector's pair list, so note whether this is the + # connector's first pair before the order book registers it + first_pair = not connector._trading_pairs + # 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,8 +176,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 register_trading_pair(connector, trading_pair, first_pair=first_pair) # Update balances to include tokens from new trading pair if hasattr(connector, '_update_balances'): @@ -217,22 +222,6 @@ async def remove_market( logger.info(f"Removed market {connector_name}/{trading_pair}") - def _register_trading_pair_with_connector( - self, - connector: ConnectorBase, - trading_pair: str - ): - """ - Register a trading pair with the connector's internal structures. - - 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) - logger.debug(f"Registered {trading_pair} with connector {type(connector).__name__}") - # ======================================== # ScriptStrategyBase-compatible methods # These are called by executors via self._strategy.method() diff --git a/services/unified_connector_service.py b/services/unified_connector_service.py index 403e7d48..5283ee11 100644 --- a/services/unified_connector_service.py +++ b/services/unified_connector_service.py @@ -24,7 +24,7 @@ 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 @@ -684,8 +684,12 @@ 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. + if len(connector.supported_position_modes()) > 1: + await connector._initialize_position_mode() 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..820807fb --- /dev/null +++ b/test/test_position_mode_sync.py @@ -0,0 +1,158 @@ +"""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. + +These drive the real bitget connector with only its 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.bitget_perpetual import bitget_perpetual_constants as CONSTANTS +from hummingbot.connector.derivative.bitget_perpetual.bitget_perpetual_derivative import BitgetPerpetualDerivative +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 +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.orders = [] + + async def get(self, path_url, params=None, is_auth_required=False, **kwargs): + assert path_url == CONSTANTS.ACCOUNT_INFO_ENDPOINT + 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 _bitget(account: FakeBitgetAccount) -> BitgetPerpetualDerivative: + 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"})) + connector._api_get = account.get + connector._api_post = account.post + return connector + + +async def test_direct_order_on_a_fresh_connector_goes_out_in_the_accounts_hedge_mode(): + account = FakeBitgetAccount("hedge_mode") + connector = _bitget(account) + connector._trading_rules["BTC-USDT"] = TradingRule( + "BTC-USDT", min_order_size=Decimal("0.001"), min_price_increment=Decimal("0.1"), + min_base_amount_increment=Decimal("0.001"), min_notional_size=Decimal("5")) + 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" + + +def _service(connector) -> PerpetualTradingService: + async def provider(account_name, connector_name): + return connector + return PerpetualTradingService(provider) + + +async def test_first_pair_adopts_the_bitget_accounts_hedge_mode(): + account = FakeBitgetAccount("hedge_mode") + connector = _bitget(account) + + 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 account.switch_requests == [] + + +async def test_switch_without_a_registered_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) + + assert e.value.status_code == 400 + assert connector.position_mode == PositionMode.ONEWAY + assert account.switch_requests == [] + + +async def test_switch_with_an_unlisted_pair_is_refused_without_registering_it(): + connector = _bitget(FakeBitgetAccount("one_way_mode")) + + 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 connector.trading_pairs == [] + + +async def test_switch_reaches_the_exchange_through_the_given_pair(): + account = FakeBitgetAccount("one_way_mode") + connector = _bitget(account) + + result = await _service(connector).set_position_mode( + "master", "bitget_perpetual", PositionMode.HEDGE, trading_pair="BTC-USDT") + + assert result["status"] == "success" + assert account.pos_mode == "hedge_mode" + assert connector.position_mode == PositionMode.HEDGE + + +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, trading_pair="BTC-USDT") + + assert e.value.status_code == 502 + assert account.switch_requests == ["hedge_mode"] + assert connector.position_mode == PositionMode.ONEWAY