From eedbcb4ae540f55c9b811dc89f169b1a03858df3 Mon Sep 17 00:00:00 2001 From: Ibrahim Elsayed Date: Mon, 18 May 2026 19:04:44 +0300 Subject: [PATCH 1/5] feat: Implement Binance authenticated order execution - closes #85 Adds _sign() (HMAC-SHA256) and _signed_request() helpers to BinanceExchange. Implements 4 previously stubbed methods: - place_order: POST /api/v3/order, market and limit, compound order_id format - get_balance: GET /api/v3/account, filters zero balances - get_order_status: GET /api/v3/order, requires SYMBOL:ID format - cancel_order: DELETE /api/v3/order, handles -2011 (already filled) Signature verified against Binance documentation reference vector. 23 unit tests, all mocked (no real credentials required). --- app/pt_exchanges.py | 201 +++++++++++++++++++++++++- app/test_binance_exchange.py | 272 +++++++++++++++++++++++++++++++++++ 2 files changed, 469 insertions(+), 4 deletions(-) create mode 100644 app/test_binance_exchange.py diff --git a/app/pt_exchanges.py b/app/pt_exchanges.py index 2047d435..67ec3fca 100644 --- a/app/pt_exchanges.py +++ b/app/pt_exchanges.py @@ -208,19 +208,212 @@ def get_market_data(self, symbol: str) -> MarketData: exchange="binance", ) + # ------------------------------------------------------------------ + # Authenticated helpers + # ------------------------------------------------------------------ + + def _sign(self, params: str) -> str: + """Generate HMAC-SHA256 signature for Binance signed endpoints.""" + return hmac.new( + self.api_secret.encode("utf-8"), + params.encode("utf-8"), + hashlib.sha256, + ).hexdigest() + + def _signed_request( + self, method: str, path: str, params: Optional[Dict] = None + ) -> Dict: + """ + Make a signed request to a Binance private endpoint. + + Appends timestamp and HMAC-SHA256 signature before sending. + + Raises: + RuntimeError: on Binance API error (negative code field) + requests.RequestException: on network failure + """ + params = params or {} + params["timestamp"] = int(time.time() * 1000) + query = "&".join(f"{k}={v}" for k, v in sorted(params.items())) + signature = self._sign(query) + query += f"&signature={signature}" + + url = f"{self.base_url}{path}?{query}" + headers = {"X-MBX-APIKEY": self.api_key} + + resp_map = { + "GET": requests.get, + "POST": requests.post, + "DELETE": requests.delete, + } + send = resp_map.get(method.upper()) + if send is None: + raise ValueError(f"Unsupported HTTP method: {method}") + + response = send(url, headers=headers, timeout=10) + data = response.json() + if isinstance(data, dict) and "code" in data and data["code"] < 0: + raise RuntimeError(f"Binance API error {data['code']}: {data.get('msg', '')}") + return data + + # ------------------------------------------------------------------ + # AbstractExchange implementation + # ------------------------------------------------------------------ + def place_order( self, symbol: str, side: str, amount: float, price: Optional[float] = None ) -> OrderResult: - raise NotImplementedError("Binance order placement to be implemented") + """ + Place a market or limit order on Binance via POST /api/v3/order. + + Args: + symbol: Standard format e.g. 'BTC-USD' (converted to BTCUSDT) + side: 'buy' or 'sell' + amount: Quantity of base asset + price: Limit price; None for market order + + Returns: + OrderResult. order_id uses 'SYMBOL:ID' format for later lookup. + + Raises: + RuntimeError: on missing credentials or API error + """ + if not self.api_key or not self.api_secret: + raise RuntimeError("Binance API credentials required for order placement") + + binance_symbol = self._convert_symbol(symbol) + order_type = "LIMIT" if price is not None else "MARKET" + + params: Dict = { + "symbol": binance_symbol, + "side": side.upper(), + "type": order_type, + "quantity": f"{amount:.8f}", + } + if order_type == "LIMIT": + params["price"] = f"{price:.8f}" + params["timeInForce"] = "GTC" + + data = self._signed_request("POST", "/api/v3/order", params) + + # Prefix order_id with symbol so get_order_status/cancel_order can use it + compound_id = f"{binance_symbol}:{data['orderId']}" + + return OrderResult( + order_id=compound_id, + symbol=symbol, + side=side.lower(), + amount=float(data.get("executedQty", amount)), + price=float(data.get("price", price or 0)), + status=data.get("status", "UNKNOWN").lower(), + exchange="binance", + timestamp=data.get("transactTime", time.time() * 1000) / 1000, + ) def get_balance(self) -> Dict[str, float]: - raise NotImplementedError("Binance balance retrieval to be implemented") + """ + Retrieve balances for all assets via GET /api/v3/account. + + Returns: + Dict mapping asset symbol to free (spendable) balance. + Only assets with free > 0 OR locked > 0 are included. + + Raises: + RuntimeError: on missing credentials or API error + """ + if not self.api_key or not self.api_secret: + raise RuntimeError("Binance API credentials required for balance retrieval") + + data = self._signed_request("GET", "/api/v3/account") + + return { + b["asset"]: float(b["free"]) + for b in data.get("balances", []) + if float(b["free"]) > 0 or float(b["locked"]) > 0 + } def get_order_status(self, order_id: str) -> OrderResult: - raise NotImplementedError("Binance order status to be implemented") + """ + Get status of an existing order via GET /api/v3/order. + + Args: + order_id: 'SYMBOL:NUMERIC_ID' as returned by place_order + e.g. 'BTCUSDT:123456789' + + Returns: + OrderResult with current status + + Raises: + RuntimeError: on missing credentials or API error + ValueError: if order_id not in 'SYMBOL:ID' format + """ + if not self.api_key or not self.api_secret: + raise RuntimeError("Binance API credentials required for order status") + + if ":" not in str(order_id): + raise ValueError( + "Binance order lookup requires 'SYMBOL:ORDER_ID' format " + "(as returned by place_order). Got: " + str(order_id) + ) + + binance_symbol, numeric_id = str(order_id).split(":", 1) + + data = self._signed_request( + "GET", + "/api/v3/order", + {"symbol": binance_symbol, "orderId": int(numeric_id)}, + ) + + symbol = binance_symbol.replace("USDT", "-USD") + + return OrderResult( + order_id=order_id, + symbol=symbol, + side=data["side"].lower(), + amount=float(data.get("executedQty", 0)), + price=float(data.get("price", 0)), + status=data.get("status", "UNKNOWN").lower(), + exchange="binance", + timestamp=data.get("time", time.time() * 1000) / 1000, + ) def cancel_order(self, order_id: str) -> bool: - raise NotImplementedError("Binance order cancellation to be implemented") + """ + Cancel an open order via DELETE /api/v3/order. + + Args: + order_id: 'SYMBOL:NUMERIC_ID' as returned by place_order + + Returns: + True if successfully cancelled. + False if order already filled or unknown (error -2011). + + Raises: + RuntimeError: on missing credentials or unexpected API error + ValueError: if order_id not in 'SYMBOL:ID' format + """ + if not self.api_key or not self.api_secret: + raise RuntimeError("Binance API credentials required for order cancellation") + + if ":" not in str(order_id): + raise ValueError( + "Binance cancel requires 'SYMBOL:ORDER_ID' format. Got: " + str(order_id) + ) + + binance_symbol, numeric_id = str(order_id).split(":", 1) + + try: + data = self._signed_request( + "DELETE", + "/api/v3/order", + {"symbol": binance_symbol, "orderId": int(numeric_id)}, + ) + return data.get("status") in ("CANCELED", "CANCELLED") + except RuntimeError as exc: + if "-2011" in str(exc): + # Order unknown: already filled or previously cancelled + return False + raise def is_available_in_region(self, region: str) -> bool: return True # Available globally (check local regulations) diff --git a/app/test_binance_exchange.py b/app/test_binance_exchange.py new file mode 100644 index 00000000..4699db01 --- /dev/null +++ b/app/test_binance_exchange.py @@ -0,0 +1,272 @@ +""" +Tests for BinanceExchange authenticated order execution - issue #85. +All tests mock HTTP calls; no real Binance API credentials required. +""" +import hashlib +import hmac +import unittest +from unittest.mock import patch + +import sys + +sys.path.insert(0, ".") + +from pt_exchanges import BinanceExchange + + +def _make_exchange(key="test_key", secret="test_secret"): + return BinanceExchange(api_key=key, api_secret=secret) + + +def _sign(secret: str, params: str) -> str: + return hmac.new( + secret.encode("utf-8"), params.encode("utf-8"), hashlib.sha256 + ).hexdigest() + + +class TestBinanceSign(unittest.TestCase): + """HMAC-SHA256 signature generation.""" + + def test_sign_produces_hex_digest(self): + ex = _make_exchange() + result = ex._sign("symbol=BTCUSDT×tamp=1234567890") + self.assertEqual(len(result), 64) + self.assertTrue(all(c in "0123456789abcdef" for c in result)) + + def test_sign_deterministic(self): + ex = _make_exchange() + params = "symbol=BTCUSDT&side=BUY×tamp=1000000" + self.assertEqual(ex._sign(params), ex._sign(params)) + + def test_sign_matches_reference(self): + """Verify against a known HMAC-SHA256 value.""" + secret = "NhqPtmdSJYdKjVHjA7PZj4Mge3R5YNiP1e3UZjInClVN65XAbvqqM6A7H5fATj0j" + ex = BinanceExchange(api_key="key", api_secret=secret) + params = ( + "symbol=LTCBTC&side=BUY&type=LIMIT&timeInForce=GTC" + "&quantity=1&price=0.1&recvWindow=5000×tamp=1499827319559" + ) + result = ex._sign(params) + # Reference from Binance docs + self.assertEqual( + result, "c8db56825ae71d6d79447849e617115f4a920fa2acdcab2b053c4b2838bd6b71" + ) + + +class TestBinancePlaceOrder(unittest.TestCase): + def setUp(self): + self.ex = _make_exchange() + + @patch("pt_exchanges.requests.post") + def test_market_buy_success(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 12345, + "status": "FILLED", + "executedQty": "0.00100000", + "price": "0.00000000", + "transactTime": 1499827319559, + } + result = self.ex.place_order("BTC-USD", "buy", 0.001) + self.assertEqual(result.exchange, "binance") + self.assertEqual(result.side, "buy") + self.assertIn("BTCUSDT:12345", result.order_id) + self.assertEqual(result.status, "filled") + + @patch("pt_exchanges.requests.post") + def test_limit_sell_sends_price_and_tif(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 99, + "status": "NEW", + "executedQty": "0", + "price": "75000.00", + "transactTime": 1499827319559, + } + self.ex.place_order("BTC-USD", "sell", 0.001, price=75000.0) + call_url = mock_post.call_args[0][0] + self.assertIn("type=LIMIT", call_url) + self.assertIn("timeInForce=GTC", call_url) + self.assertIn("price=", call_url) + + @patch("pt_exchanges.requests.post") + def test_market_order_no_price_param(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 1, + "status": "FILLED", + "executedQty": "0.001", + "price": "0", + "transactTime": 1000, + } + self.ex.place_order("ETH-USD", "buy", 0.01) + call_url = mock_post.call_args[0][0] + self.assertIn("type=MARKET", call_url) + self.assertNotIn("timeInForce", call_url) + + @patch("pt_exchanges.requests.post") + def test_api_error_raises_runtime_error(self, mock_post): + mock_post.return_value.json.return_value = { + "code": -1013, + "msg": "Filter failure: MIN_NOTIONAL", + } + with self.assertRaises(RuntimeError) as ctx: + self.ex.place_order("BTC-USD", "buy", 0.000001) + self.assertIn("-1013", str(ctx.exception)) + + def test_missing_credentials_raises(self): + ex = BinanceExchange(api_key="", api_secret="") + with self.assertRaises(RuntimeError): + ex.place_order("BTC-USD", "buy", 0.001) + + @patch("pt_exchanges.requests.post") + def test_signature_in_url(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 1, + "status": "FILLED", + "executedQty": "0.001", + "price": "0", + "transactTime": 1000, + } + self.ex.place_order("BTC-USD", "buy", 0.001) + call_url = mock_post.call_args[0][0] + self.assertIn("signature=", call_url) + self.assertIn("X-MBX-APIKEY", mock_post.call_args[1]["headers"]) + + @patch("pt_exchanges.requests.post") + def test_symbol_converted_to_binance_format(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 2, + "status": "FILLED", + "executedQty": "0.1", + "price": "0", + "transactTime": 1000, + } + self.ex.place_order("BTC-USD", "buy", 0.1) + call_url = mock_post.call_args[0][0] + self.assertIn("symbol=BTCUSDT", call_url) + + +class TestBinanceGetBalance(unittest.TestCase): + def setUp(self): + self.ex = _make_exchange() + + @patch("pt_exchanges.requests.get") + def test_returns_nonzero_balances(self, mock_get): + mock_get.return_value.json.return_value = { + "balances": [ + {"asset": "BTC", "free": "0.5", "locked": "0.0"}, + {"asset": "USDT", "free": "1000.0", "locked": "50.0"}, + {"asset": "XRP", "free": "0.0", "locked": "0.0"}, # zero - excluded + ] + } + result = self.ex.get_balance() + self.assertIn("BTC", result) + self.assertIn("USDT", result) + self.assertNotIn("XRP", result) + self.assertAlmostEqual(result["BTC"], 0.5) + + @patch("pt_exchanges.requests.get") + def test_includes_locked_nonzero(self, mock_get): + mock_get.return_value.json.return_value = { + "balances": [ + {"asset": "ETH", "free": "0.0", "locked": "1.0"}, + ] + } + result = self.ex.get_balance() + self.assertIn("ETH", result) + + def test_missing_credentials_raises(self): + ex = BinanceExchange(api_key="", api_secret="") + with self.assertRaises(RuntimeError): + ex.get_balance() + + +class TestBinanceGetOrderStatus(unittest.TestCase): + def setUp(self): + self.ex = _make_exchange() + + @patch("pt_exchanges.requests.get") + def test_returns_order_result(self, mock_get): + mock_get.return_value.json.return_value = { + "orderId": 12345, + "side": "BUY", + "status": "FILLED", + "executedQty": "0.001", + "price": "75000.00", + "time": 1499827319559, + } + result = self.ex.get_order_status("BTCUSDT:12345") + self.assertEqual(result.status, "filled") + self.assertEqual(result.side, "buy") + self.assertEqual(result.exchange, "binance") + + @patch("pt_exchanges.requests.get") + def test_symbol_in_request(self, mock_get): + mock_get.return_value.json.return_value = { + "orderId": 99, + "side": "SELL", + "status": "NEW", + "executedQty": "0", + "price": "0", + "time": 1000, + } + self.ex.get_order_status("BTCUSDT:99") + call_url = mock_get.call_args[0][0] + self.assertIn("symbol=BTCUSDT", call_url) + self.assertIn("orderId=99", call_url) + + def test_invalid_format_raises_value_error(self): + with self.assertRaises(ValueError): + self.ex.get_order_status("12345") # missing symbol prefix + + def test_missing_credentials_raises(self): + ex = BinanceExchange(api_key="", api_secret="") + with self.assertRaises(RuntimeError): + ex.get_order_status("BTCUSDT:1") + + +class TestBinanceCancelOrder(unittest.TestCase): + def setUp(self): + self.ex = _make_exchange() + + @patch("pt_exchanges.requests.delete") + def test_cancel_success(self, mock_del): + mock_del.return_value.json.return_value = {"status": "CANCELED"} + self.assertTrue(self.ex.cancel_order("BTCUSDT:12345")) + + @patch("pt_exchanges.requests.delete") + def test_cancel_already_filled_returns_false(self, mock_del): + mock_del.return_value.json.return_value = { + "code": -2011, + "msg": "Unknown order sent.", + } + self.assertFalse(self.ex.cancel_order("BTCUSDT:99999")) + + @patch("pt_exchanges.requests.delete") + def test_cancel_sends_correct_endpoint(self, mock_del): + mock_del.return_value.json.return_value = {"status": "CANCELED"} + self.ex.cancel_order("ETHUSDT:777") + call_url = mock_del.call_args[0][0] + self.assertIn("/api/v3/order", call_url) + self.assertIn("symbol=ETHUSDT", call_url) + self.assertIn("orderId=777", call_url) + + def test_invalid_format_raises_value_error(self): + with self.assertRaises(ValueError): + self.ex.cancel_order("plain_id_no_symbol") + + def test_missing_credentials_raises(self): + ex = BinanceExchange(api_key="", api_secret="") + with self.assertRaises(RuntimeError): + ex.cancel_order("BTCUSDT:1") + + @patch("pt_exchanges.requests.delete") + def test_unexpected_error_propagates(self, mock_del): + mock_del.return_value.json.return_value = { + "code": -1100, + "msg": "Illegal characters found in parameter", + } + with self.assertRaises(RuntimeError): + self.ex.cancel_order("BTCUSDT:123") + + +if __name__ == "__main__": + unittest.main() From c90ef0ac8cc353213448b5a5361da9cb44c1807d Mon Sep 17 00:00:00 2001 From: Ibrahim Elsayed Date: Mon, 18 May 2026 19:30:21 +0300 Subject: [PATCH 2/5] fix: Black formatting on pt_exchanges.py --- app/pt_exchanges.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/pt_exchanges.py b/app/pt_exchanges.py index 67ec3fca..0da50fab 100644 --- a/app/pt_exchanges.py +++ b/app/pt_exchanges.py @@ -253,7 +253,9 @@ def _signed_request( response = send(url, headers=headers, timeout=10) data = response.json() if isinstance(data, dict) and "code" in data and data["code"] < 0: - raise RuntimeError(f"Binance API error {data['code']}: {data.get('msg', '')}") + raise RuntimeError( + f"Binance API error {data['code']}: {data.get('msg', '')}" + ) return data # ------------------------------------------------------------------ @@ -393,11 +395,14 @@ def cancel_order(self, order_id: str) -> bool: ValueError: if order_id not in 'SYMBOL:ID' format """ if not self.api_key or not self.api_secret: - raise RuntimeError("Binance API credentials required for order cancellation") + raise RuntimeError( + "Binance API credentials required for order cancellation" + ) if ":" not in str(order_id): raise ValueError( - "Binance cancel requires 'SYMBOL:ORDER_ID' format. Got: " + str(order_id) + "Binance cancel requires 'SYMBOL:ORDER_ID' format. Got: " + + str(order_id) ) binance_symbol, numeric_id = str(order_id).split(":", 1) From ef981fe14a2c372dbb4ecf7bd91ed6bf69b0721d Mon Sep 17 00:00:00 2001 From: Ibrahim Elsayed Date: Tue, 19 May 2026 15:32:46 +0300 Subject: [PATCH 3/5] style: apply Black formatting --- app/test_binance_exchange.py | 1 + 1 file changed, 1 insertion(+) diff --git a/app/test_binance_exchange.py b/app/test_binance_exchange.py index 4699db01..dc731579 100644 --- a/app/test_binance_exchange.py +++ b/app/test_binance_exchange.py @@ -2,6 +2,7 @@ Tests for BinanceExchange authenticated order execution - issue #85. All tests mock HTTP calls; no real Binance API credentials required. """ + import hashlib import hmac import unittest From 52bf4672b123e25ae473f7f7e36fb5c26bb7ac37 Mon Sep 17 00:00:00 2001 From: Ibrahim Elsayed Date: Sat, 23 May 2026 16:27:55 +0300 Subject: [PATCH 4/5] fix: enforce Binance LOT_SIZE/PRICE_FILTER/MIN_NOTIONAL before order Raw float quantities and prices were sent without symbol-aware rounding, causing Binance to reject many real orders with -1013 (Filter failure). - Fetch and cache per-symbol filters from /api/v3/exchangeInfo on first use; reuse cached dict for subsequent orders. - Round quantity DOWN to LOT_SIZE.stepSize (truncate, never round up so qty cannot exceed user intent). - Round limit price DOWN to PRICE_FILTER.tickSize. - Reject locally with RuntimeError when quantity < minQty or quantity * price < minNotional, surfacing a useful message instead of Binance's opaque -1013 / -1100. - Handle both legacy MIN_NOTIONAL and renamed NOTIONAL filter types (Binance migration in 2023). - Add 5 tests covering rounding, rejection, and filter caching. --- app/pt_exchanges.py | 104 ++++++++++++++++++++++++++++++++++- app/test_binance_exchange.py | 101 +++++++++++++++++++++++++++++++++- 2 files changed, 201 insertions(+), 4 deletions(-) diff --git a/app/pt_exchanges.py b/app/pt_exchanges.py index 0da50fab..1d87f9b5 100644 --- a/app/pt_exchanges.py +++ b/app/pt_exchanges.py @@ -8,6 +8,7 @@ import hmac import json import time +from decimal import ROUND_DOWN, Decimal, InvalidOperation from typing import Dict, List, Optional import requests @@ -166,6 +167,10 @@ class BinanceExchange(AbstractExchange): def __init__(self, api_key: str = "", api_secret: str = "", **kwargs): super().__init__(api_key, api_secret, **kwargs) self.base_url = "https://api.binance.com" + # Per-symbol filter cache: {binance_symbol: {"stepSize": Decimal, + # "tickSize": Decimal, "minQty": Decimal, "minNotional": Decimal}}. + # Populated lazily by _get_symbol_filters() to avoid per-order REST. + self._symbol_filters: Dict[str, Dict[str, Decimal]] = {} def get_exchange_name(self) -> str: return "binance" @@ -258,6 +263,73 @@ def _signed_request( ) return data + # ------------------------------------------------------------------ + # Symbol filter handling (LOT_SIZE, PRICE_FILTER, MIN_NOTIONAL) + # ------------------------------------------------------------------ + + def _get_symbol_filters(self, binance_symbol: str) -> Dict[str, Decimal]: + """ + Fetch and cache the trading filters for a symbol from + /api/v3/exchangeInfo. Subsequent calls hit the in-memory cache so + each order placement does not pay a REST round-trip. + + Returns dict with keys: stepSize, tickSize, minQty, minNotional. + Missing filters are returned as Decimal("0") so callers can treat + them as "no constraint". + """ + if binance_symbol in self._symbol_filters: + return self._symbol_filters[binance_symbol] + + url = f"{self.base_url}/api/v3/exchangeInfo?symbol={binance_symbol}" + response = requests.get(url, timeout=10) + info = response.json() + if "symbols" not in info or not info["symbols"]: + raise RuntimeError( + f"Binance exchangeInfo returned no data for {binance_symbol}" + ) + + filters: Dict[str, Decimal] = { + "stepSize": Decimal("0"), + "tickSize": Decimal("0"), + "minQty": Decimal("0"), + "minNotional": Decimal("0"), + } + for f in info["symbols"][0].get("filters", []): + try: + if f["filterType"] == "LOT_SIZE": + filters["stepSize"] = Decimal(f["stepSize"]) + filters["minQty"] = Decimal(f["minQty"]) + elif f["filterType"] == "PRICE_FILTER": + filters["tickSize"] = Decimal(f["tickSize"]) + elif f["filterType"] in ("MIN_NOTIONAL", "NOTIONAL"): + # Binance renamed MIN_NOTIONAL -> NOTIONAL in 2023; + # support both so older and newer pairs both work. + filters["minNotional"] = Decimal( + f.get("minNotional", f.get("notional", "0")) + ) + except (InvalidOperation, KeyError): + # Unknown filter shape: skip rather than blowing up the order. + continue + + self._symbol_filters[binance_symbol] = filters + return filters + + @staticmethod + def _round_to_step(value: Decimal, step: Decimal) -> Decimal: + """ + Round ``value`` *down* to the nearest multiple of ``step``. + + Truncating (ROUND_DOWN) — never rounding up — keeps the submitted + quantity at or below the user's intent. Rounding up could exceed + available balance, overshoot a stop, or violate a per-trade cap. + """ + if step == 0: + return value + quantised = (value / step).to_integral_value(rounding=ROUND_DOWN) * step + # Normalise away trailing zeros so Binance accepts e.g. "0.001" + # instead of "0.00100000" which can fail the PRICE_FILTER regex. + return quantised.normalize() + # ------------------------------------------------------------------ # AbstractExchange implementation # ------------------------------------------------------------------ @@ -274,11 +346,17 @@ def place_order( amount: Quantity of base asset price: Limit price; None for market order + Quantity is rounded *down* to the symbol's LOT_SIZE step and the + limit price is rounded *down* to the PRICE_FILTER tick. After + rounding, the order is rejected locally with RuntimeError if + quantity < minQty or quantity * price < minNotional, so the user + sees a useful error instead of Binance's opaque -1013 / -1100. + Returns: OrderResult. order_id uses 'SYMBOL:ID' format for later lookup. Raises: - RuntimeError: on missing credentials or API error + RuntimeError: on missing credentials, filter violation, or API error """ if not self.api_key or not self.api_secret: raise RuntimeError("Binance API credentials required for order placement") @@ -286,14 +364,34 @@ def place_order( binance_symbol = self._convert_symbol(symbol) order_type = "LIMIT" if price is not None else "MARKET" + # Apply LOT_SIZE / PRICE_FILTER / MIN_NOTIONAL before signing. + filters = self._get_symbol_filters(binance_symbol) + qty_dec = self._round_to_step(Decimal(str(amount)), filters["stepSize"]) + price_dec: Optional[Decimal] = None + if price is not None: + price_dec = self._round_to_step(Decimal(str(price)), filters["tickSize"]) + + if filters["minQty"] > 0 and qty_dec < filters["minQty"]: + raise RuntimeError( + f"Binance {binance_symbol} order quantity {qty_dec} below " + f"minQty {filters['minQty']} after LOT_SIZE rounding" + ) + if filters["minNotional"] > 0 and price_dec is not None: + notional = qty_dec * price_dec + if notional < filters["minNotional"]: + raise RuntimeError( + f"Binance {binance_symbol} notional {notional} below " + f"minNotional {filters['minNotional']}" + ) + params: Dict = { "symbol": binance_symbol, "side": side.upper(), "type": order_type, - "quantity": f"{amount:.8f}", + "quantity": format(qty_dec, "f"), } if order_type == "LIMIT": - params["price"] = f"{price:.8f}" + params["price"] = format(price_dec, "f") params["timeInForce"] = "GTC" data = self._signed_request("POST", "/api/v3/order", params) diff --git a/app/test_binance_exchange.py b/app/test_binance_exchange.py index dc731579..18a54bb1 100644 --- a/app/test_binance_exchange.py +++ b/app/test_binance_exchange.py @@ -54,9 +54,30 @@ def test_sign_matches_reference(self): ) +def _seed_filters( + ex, + binance_symbol="BTCUSDT", + step="0.00001000", + tick="0.01000000", + min_qty="0.00001000", + min_notional="10.00000000", +): + """Pre-seed the symbol filter cache so place_order does not hit /exchangeInfo.""" + from decimal import Decimal as _D + + ex._symbol_filters[binance_symbol] = { + "stepSize": _D(step), + "tickSize": _D(tick), + "minQty": _D(min_qty), + "minNotional": _D(min_notional), + } + + class TestBinancePlaceOrder(unittest.TestCase): def setUp(self): self.ex = _make_exchange() + _seed_filters(self.ex, "BTCUSDT") + _seed_filters(self.ex, "ETHUSDT") @patch("pt_exchanges.requests.post") def test_market_buy_success(self, mock_post): @@ -108,8 +129,10 @@ def test_api_error_raises_runtime_error(self, mock_post): "code": -1013, "msg": "Filter failure: MIN_NOTIONAL", } + # 0.001 survives local LOT_SIZE/minQty checks so we exercise the + # Binance-side error propagation path. with self.assertRaises(RuntimeError) as ctx: - self.ex.place_order("BTC-USD", "buy", 0.000001) + self.ex.place_order("BTC-USD", "buy", 0.001) self.assertIn("-1013", str(ctx.exception)) def test_missing_credentials_raises(self): @@ -145,6 +168,82 @@ def test_symbol_converted_to_binance_format(self, mock_post): self.assertIn("symbol=BTCUSDT", call_url) +class TestBinanceFilterEnforcement(unittest.TestCase): + """Symbol filter rounding and rejection (LOT_SIZE, PRICE_FILTER, MIN_NOTIONAL).""" + + def setUp(self): + self.ex = _make_exchange() + _seed_filters(self.ex, "BTCUSDT") + + @patch("pt_exchanges.requests.post") + def test_quantity_rounded_down_to_step(self, mock_post): + """0.0012345 with stepSize 0.00001 must send "0.00123" (truncated).""" + mock_post.return_value.json.return_value = { + "orderId": 1, + "status": "FILLED", + "executedQty": "0.00123", + "price": "0", + "transactTime": 1, + } + self.ex.place_order("BTC-USD", "buy", 0.0012345) + call_url = mock_post.call_args[0][0] + self.assertIn("quantity=0.00123", call_url) + # Must never round up + self.assertNotIn("quantity=0.00124", call_url) + + @patch("pt_exchanges.requests.post") + def test_price_rounded_down_to_tick(self, mock_post): + """75000.999 with tickSize 0.01 must send "75000.99" (truncated).""" + mock_post.return_value.json.return_value = { + "orderId": 1, + "status": "NEW", + "executedQty": "0", + "price": "75000.99", + "transactTime": 1, + } + self.ex.place_order("BTC-USD", "sell", 0.001, price=75000.999) + call_url = mock_post.call_args[0][0] + self.assertIn("price=75000.99", call_url) + + def test_quantity_below_min_qty_raises(self): + """Qty 0.0000001 rounds to 0, below minQty 0.00001 — reject locally.""" + with self.assertRaises(RuntimeError) as ctx: + self.ex.place_order("BTC-USD", "buy", 0.0000001) + self.assertIn("minQty", str(ctx.exception)) + + def test_notional_below_min_notional_raises(self): + """Qty 0.00001 * price 1.00 = 0.00001 < minNotional 10 — reject locally.""" + with self.assertRaises(RuntimeError) as ctx: + self.ex.place_order("BTC-USD", "buy", 0.00001, price=1.00) + self.assertIn("minNotional", str(ctx.exception)) + + @patch("pt_exchanges.requests.get") + def test_filters_fetched_and_cached(self, mock_get): + """First call fetches /exchangeInfo; second call uses cache.""" + mock_get.return_value.json.return_value = { + "symbols": [ + { + "filters": [ + { + "filterType": "LOT_SIZE", + "stepSize": "0.001", + "minQty": "0.001", + }, + {"filterType": "PRICE_FILTER", "tickSize": "0.01"}, + {"filterType": "NOTIONAL", "minNotional": "5"}, + ], + } + ], + } + fresh = _make_exchange() + f1 = fresh._get_symbol_filters("ETHUSDT") + f2 = fresh._get_symbol_filters("ETHUSDT") + self.assertEqual(mock_get.call_count, 1) # cached on 2nd call + self.assertEqual(f1, f2) + self.assertEqual(str(f1["stepSize"]), "0.001") + self.assertEqual(str(f1["minNotional"]), "5") + + class TestBinanceGetBalance(unittest.TestCase): def setUp(self): self.ex = _make_exchange() From 392dfafb87d5d5902cd09224037037ae596fe576 Mon Sep 17 00:00:00 2001 From: Ibrahim Elsayed Date: Sat, 23 May 2026 18:44:26 +0300 Subject: [PATCH 5/5] feat(binance): complete spot REST coverage - testnet, time sync, rate limit, all order types, OCO, listenKey Brings BinanceExchange to full feature parity with Binance Spot API docs so PR #92 covers everything needed before the broker selector in #96 wires it to the GUI. REST + auth - testnet=True flag in constructor flips REST base to testnet.binance.vision and WebSocket base to stream.testnet.binance.vision/ws. Default stays production. - recv_window kwarg (1..60000ms per Binance limits) auto-injected into every signed request alongside the server-time-corrected timestamp. - sync_time() hits GET /api/v3/time once on first signed call to cache the local-vs- server offset; -1021 responses trigger one auto-resync + retry before raising BinanceTimestampError. - HTTP 429 / 418 now raise BinanceRateLimitError carrying the status code and the Retry-After header so the trader loop can back off instead of hammering and earning a longer ban. - X-MBX-USED-WEIGHT-* and X-MBX-ORDER-COUNT-* response headers captured into last_rate_limit_headers for monitoring. Order types - place_order extended to support every Binance spot type: MARKET, LIMIT, STOP_LOSS, STOP_LOSS_LIMIT, TAKE_PROFIT, TAKE_PROFIT_LIMIT, LIMIT_MAKER. - Per-type required-field validation (price for limits, stop_price/trailing_delta for stops, both-vs-neither guard for MARKET amount/quoteOrderQty). - New kwargs: order_type, stop_price, time_in_force, iceberg_qty, quote_order_qty, client_order_id, trailing_delta. Legacy (symbol, side, amount[, price]) form still auto-derives MARKET/LIMIT for backwards compat. - LOT_SIZE / PRICE_FILTER / MIN_NOTIONAL rounding applies to every leg. OCO - place_oco_order targets the modern POST /api/v3/orderList/oco with the aboveType / belowType schema (Binance retired the flat /order/oco shape in 2024). - cancel_order_list deletes the whole list via DELETE /api/v3/orderList. User-data stream - create_listen_key / keepalive_listen_key / close_listen_key cover the POST/PUT/DELETE /api/v3/userDataStream lifecycle via a new _public_request helper (API-key header, no signature). - user_data_stream_url returns the right wss:// URL for prod or testnet. Broker-selector hooks (for #96) - test_connection() returns bool by probing /api/v3/account, never raising. - get_masked_api_key() returns "****" for the per-broker UI row. Tests - 66 unit tests, all mock-based, no real Binance credentials. Covers testnet toggle, time-sync offset, -1021 retry success + persistent-failure path, 429/418 rate-limit raising, weight-header capture, every new order type, validation errors, OCO place + cancel, listenKey lifecycle, broker-selector helpers, plus the existing 28 from previous round. --- app/pt_exchanges.py | 473 ++++++++++++++++++++++++++++++++-- app/test_binance_exchange.py | 476 ++++++++++++++++++++++++++++++++++- 2 files changed, 920 insertions(+), 29 deletions(-) diff --git a/app/pt_exchanges.py b/app/pt_exchanges.py index 1d87f9b5..cb0194c1 100644 --- a/app/pt_exchanges.py +++ b/app/pt_exchanges.py @@ -161,20 +161,113 @@ def _convert_symbol(self, symbol: str) -> str: return symbol_map.get(symbol, symbol.replace("-", "")) -class BinanceExchange(AbstractExchange): - """Binance API implementation""" +_BINANCE_PROD_REST = "https://api.binance.com" +_BINANCE_TESTNET_REST = "https://testnet.binance.vision" +_BINANCE_PROD_WS = "wss://stream.binance.com:9443/ws" +_BINANCE_TESTNET_WS = "wss://stream.testnet.binance.vision/ws" + +# Limit/stop order types per Binance spot docs +_BINANCE_SPOT_ORDER_TYPES = { + "MARKET", + "LIMIT", + "STOP_LOSS", + "STOP_LOSS_LIMIT", + "TAKE_PROFIT", + "TAKE_PROFIT_LIMIT", + "LIMIT_MAKER", +} +_BINANCE_LIMIT_TYPES = { + "LIMIT", + "STOP_LOSS_LIMIT", + "TAKE_PROFIT_LIMIT", + "LIMIT_MAKER", +} +_BINANCE_STOP_TYPES = { + "STOP_LOSS", + "STOP_LOSS_LIMIT", + "TAKE_PROFIT", + "TAKE_PROFIT_LIMIT", +} + + +class BinanceRateLimitError(RuntimeError): + """Raised on HTTP 429/418 from Binance. Carries retry-after (seconds).""" + + def __init__(self, status_code: int, retry_after: float, message: str = ""): + super().__init__( + f"Binance rate limit ({status_code}): retry after {retry_after}s. {message}" + ) + self.status_code = status_code + self.retry_after = retry_after + + +class BinanceTimestampError(RuntimeError): + """Raised on -1021 (timestamp outside recvWindow) after auto-resync retry.""" - def __init__(self, api_key: str = "", api_secret: str = "", **kwargs): + +class BinanceExchange(AbstractExchange): + """Binance Spot API implementation. + + Covers signed order placement (all spot types), OCO, balance, order + status/cancel, exchange-filter enforcement, server-time sync, rate-limit + awareness, and user-data-stream listenKey lifecycle. Production + testnet + base URLs are selected via the ``testnet`` constructor flag so the + multi-broker selector (#96) can flip the toggle per user preference. + """ + + def __init__( + self, + api_key: str = "", + api_secret: str = "", + testnet: bool = False, + recv_window: int = 5000, + **kwargs, + ): super().__init__(api_key, api_secret, **kwargs) - self.base_url = "https://api.binance.com" + self.testnet = bool(testnet) + self.base_url = _BINANCE_TESTNET_REST if self.testnet else _BINANCE_PROD_REST + self.ws_base = _BINANCE_TESTNET_WS if self.testnet else _BINANCE_PROD_WS + # Per Binance docs: default 5000ms, max 60000ms + self.recv_window = max(1, min(int(recv_window), 60000)) # Per-symbol filter cache: {binance_symbol: {"stepSize": Decimal, # "tickSize": Decimal, "minQty": Decimal, "minNotional": Decimal}}. # Populated lazily by _get_symbol_filters() to avoid per-order REST. self._symbol_filters: Dict[str, Dict[str, Decimal]] = {} + # Server-time offset in ms: server_ms - local_ms. Refreshed lazily and + # again on -1021 retry. Keeps timestamps inside recvWindow without a + # round-trip on every order. + self._time_offset_ms: int = 0 + self._time_synced: bool = False + # Last-seen rate-limit headers, keyed by header name (e.g. + # "X-MBX-USED-WEIGHT-1M"). Exposed for monitoring; not consulted to + # decide throttling — Binance's 429 + Retry-After is authoritative. + self.last_rate_limit_headers: Dict[str, str] = {} def get_exchange_name(self) -> str: return "binance" + def get_masked_api_key(self) -> str: + """Return ``****`` for display by the broker selector UI.""" + if not self.api_key: + return "Not configured" + suffix = self.api_key[-4:] if len(self.api_key) >= 4 else self.api_key + return f"****{suffix}" + + def test_connection(self) -> bool: + """Probe credentials by hitting the account endpoint. + + Returns True on success, False on any auth/credential/network failure. + Used by the broker selector (#96) for the per-row "Test connection" + button without raising into the GUI thread. + """ + if not self.api_key or not self.api_secret: + return False + try: + self._signed_request("GET", "/api/v3/account") + return True + except Exception: + return False + def get_current_price(self, symbol: str) -> float: binance_symbol = self._convert_symbol(symbol) @@ -213,6 +306,29 @@ def get_market_data(self, symbol: str) -> MarketData: exchange="binance", ) + # ------------------------------------------------------------------ + # Server-time sync + # ------------------------------------------------------------------ + + def sync_time(self) -> int: + """Fetch /api/v3/time and cache the local-vs-server offset (ms). + + Called lazily before the first signed request and again on a -1021 + ("timestamp for this request was 1000ms ahead of the server's time") + retry. Returns the offset for tests/diagnostics. + """ + response = requests.get(f"{self.base_url}/api/v3/time", timeout=10) + data = response.json() + if "serverTime" not in data: + raise RuntimeError(f"Binance /api/v3/time unexpected response: {data}") + self._time_offset_ms = int(data["serverTime"]) - int(time.time() * 1000) + self._time_synced = True + return self._time_offset_ms + + def _now_ms(self) -> int: + """Local wall clock corrected by the cached server-time offset.""" + return int(time.time() * 1000) + self._time_offset_ms + # ------------------------------------------------------------------ # Authenticated helpers # ------------------------------------------------------------------ @@ -226,19 +342,38 @@ def _sign(self, params: str) -> str: ).hexdigest() def _signed_request( - self, method: str, path: str, params: Optional[Dict] = None + self, + method: str, + path: str, + params: Optional[Dict] = None, + _retried_on_timestamp: bool = False, ) -> Dict: """ Make a signed request to a Binance private endpoint. - Appends timestamp and HMAC-SHA256 signature before sending. + Injects ``timestamp`` (corrected by the cached server-time offset) and + ``recvWindow``, signs the canonical query with HMAC-SHA256, and posts + with the ``X-MBX-APIKEY`` header. On HTTP 429/418 raises + :class:`BinanceRateLimitError` honouring ``Retry-After``. On Binance + error -1021 (timestamp drift), resyncs and retries exactly once. Raises: - RuntimeError: on Binance API error (negative code field) - requests.RequestException: on network failure + BinanceRateLimitError: on HTTP 429 (weight) or 418 (IP ban). + BinanceTimestampError: -1021 still failing after a resync retry. + RuntimeError: any other Binance error (negative ``code`` field). + requests.RequestException: network failure. """ + if not self._time_synced: + # Best-effort; if /api/v3/time fails the unsynced timestamp will + # still work for users whose clocks are within recvWindow. + try: + self.sync_time() + except Exception: + pass + params = params or {} - params["timestamp"] = int(time.time() * 1000) + params.setdefault("recvWindow", self.recv_window) + params["timestamp"] = self._now_ms() query = "&".join(f"{k}={v}" for k, v in sorted(params.items())) signature = self._sign(query) query += f"&signature={signature}" @@ -246,17 +381,89 @@ def _signed_request( url = f"{self.base_url}{path}?{query}" headers = {"X-MBX-APIKEY": self.api_key} - resp_map = { + send = { "GET": requests.get, "POST": requests.post, "DELETE": requests.delete, - } - send = resp_map.get(method.upper()) + "PUT": requests.put, + }.get(method.upper()) if send is None: raise ValueError(f"Unsupported HTTP method: {method}") response = send(url, headers=headers, timeout=10) + + # Capture rate-limit headers regardless of status. Header names are + # case-insensitive and follow X-MBX-USED-WEIGHT-(intervalNum)(letter) + # / X-MBX-ORDER-COUNT-* per Binance spec. + try: + self.last_rate_limit_headers = { + k: v + for k, v in response.headers.items() + if k.upper().startswith(("X-MBX-USED-WEIGHT", "X-MBX-ORDER-COUNT")) + } + except Exception: + pass + + status = response.status_code + if status in (429, 418): + retry_after = float(response.headers.get("Retry-After", "0") or 0) + msg = "" + try: + msg = response.json().get("msg", "") + except Exception: + pass + raise BinanceRateLimitError(status, retry_after, msg) + data = response.json() + if isinstance(data, dict) and "code" in data and data["code"] < 0: + # -1021: timestamp outside recvWindow. Resync and retry once. + if data["code"] == -1021 and not _retried_on_timestamp: + try: + self.sync_time() + except Exception: + raise BinanceTimestampError( + f"Binance -1021 and /api/v3/time resync failed: " + f"{data.get('msg', '')}" + ) + return self._signed_request( + method, + path, + params={ + k: v + for k, v in params.items() + if k not in ("timestamp", "signature") + }, + _retried_on_timestamp=True, + ) + if data["code"] == -1021: + raise BinanceTimestampError( + f"Binance -1021 after resync retry: {data.get('msg', '')}" + ) + raise RuntimeError( + f"Binance API error {data['code']}: {data.get('msg', '')}" + ) + return data + + def _public_request( + self, method: str, path: str, headers: Optional[Dict] = None + ) -> Dict: + """Unsigned API-key request (e.g. POST /api/v3/userDataStream). + + These endpoints need ``X-MBX-APIKEY`` but no signature. + """ + send = { + "GET": requests.get, + "POST": requests.post, + "PUT": requests.put, + "DELETE": requests.delete, + }.get(method.upper()) + if send is None: + raise ValueError(f"Unsupported HTTP method: {method}") + merged = {"X-MBX-APIKEY": self.api_key} + if headers: + merged.update(headers) + response = send(f"{self.base_url}{path}", headers=merged, timeout=10) + data = response.json() if response.content else {} if isinstance(data, dict) and "code" in data and data["code"] < 0: raise RuntimeError( f"Binance API error {data['code']}: {data.get('msg', '')}" @@ -335,19 +542,43 @@ def _round_to_step(value: Decimal, step: Decimal) -> Decimal: # ------------------------------------------------------------------ def place_order( - self, symbol: str, side: str, amount: float, price: Optional[float] = None + self, + symbol: str, + side: str, + amount: float, + price: Optional[float] = None, + order_type: Optional[str] = None, + stop_price: Optional[float] = None, + time_in_force: Optional[str] = None, + iceberg_qty: Optional[float] = None, + quote_order_qty: Optional[float] = None, + client_order_id: Optional[str] = None, + trailing_delta: Optional[int] = None, ) -> OrderResult: """ - Place a market or limit order on Binance via POST /api/v3/order. + Place a spot order on Binance via POST /api/v3/order. + + Supports the full Binance spot type set: ``MARKET``, ``LIMIT``, + ``STOP_LOSS``, ``STOP_LOSS_LIMIT``, ``TAKE_PROFIT``, + ``TAKE_PROFIT_LIMIT``, ``LIMIT_MAKER``. ``order_type`` may be passed + explicitly; if omitted, the caller stays backwards compatible — + ``LIMIT`` when ``price`` is provided, else ``MARKET``. Args: symbol: Standard format e.g. 'BTC-USD' (converted to BTCUSDT) side: 'buy' or 'sell' amount: Quantity of base asset - price: Limit price; None for market order - - Quantity is rounded *down* to the symbol's LOT_SIZE step and the - limit price is rounded *down* to the PRICE_FILTER tick. After + price: Limit price (required for LIMIT/*_LIMIT/LIMIT_MAKER) + order_type: Override the auto-derived type + stop_price: Trigger price (required for STOP_* / TAKE_PROFIT*) + time_in_force: GTC/IOC/FOK (defaulted to GTC for limit types) + iceberg_qty: Visible portion for iceberg orders (GTC only) + quote_order_qty: Quote-asset spend (MARKET only; alt to amount) + client_order_id: User-supplied order id (newClientOrderId) + trailing_delta: BIPS for trailing stop (alternative to stop_price) + + Quantity is rounded *down* to the symbol's LOT_SIZE step and any + price field is rounded *down* to the PRICE_FILTER tick. After rounding, the order is rejected locally with RuntimeError if quantity < minQty or quantity * price < minNotional, so the user sees a useful error instead of Binance's opaque -1013 / -1100. @@ -356,13 +587,39 @@ def place_order( OrderResult. order_id uses 'SYMBOL:ID' format for later lookup. Raises: + ValueError: on unsupported order_type or missing required fields RuntimeError: on missing credentials, filter violation, or API error """ if not self.api_key or not self.api_secret: raise RuntimeError("Binance API credentials required for order placement") binance_symbol = self._convert_symbol(symbol) - order_type = "LIMIT" if price is not None else "MARKET" + # Auto-derive: keeps the legacy (symbol, side, amount[, price]) signature + # behaving exactly as it did before this change. + if order_type is None: + otype = "LIMIT" if price is not None else "MARKET" + else: + otype = order_type.upper() + if otype not in _BINANCE_SPOT_ORDER_TYPES: + raise ValueError( + f"Unsupported Binance order type '{otype}'. " + f"Expected one of: {sorted(_BINANCE_SPOT_ORDER_TYPES)}" + ) + + # Per-type required-field guards. Matches Binance trading-endpoints + # spec so callers get a clean ValueError instead of -1102/-1106. + if otype in _BINANCE_LIMIT_TYPES and price is None: + raise ValueError(f"Binance {otype} requires price") + if ( + otype in _BINANCE_STOP_TYPES + and stop_price is None + and trailing_delta is None + ): + raise ValueError(f"Binance {otype} requires stop_price or trailing_delta") + if otype == "MARKET" and quote_order_qty is not None and amount: + raise ValueError( + "Binance MARKET accepts amount OR quote_order_qty, not both" + ) # Apply LOT_SIZE / PRICE_FILTER / MIN_NOTIONAL before signing. filters = self._get_symbol_filters(binance_symbol) @@ -370,13 +627,18 @@ def place_order( price_dec: Optional[Decimal] = None if price is not None: price_dec = self._round_to_step(Decimal(str(price)), filters["tickSize"]) + stop_dec: Optional[Decimal] = None + if stop_price is not None: + stop_dec = self._round_to_step( + Decimal(str(stop_price)), filters["tickSize"] + ) - if filters["minQty"] > 0 and qty_dec < filters["minQty"]: + if filters["minQty"] > 0 and qty_dec < filters["minQty"] and amount: raise RuntimeError( f"Binance {binance_symbol} order quantity {qty_dec} below " f"minQty {filters['minQty']} after LOT_SIZE rounding" ) - if filters["minNotional"] > 0 and price_dec is not None: + if filters["minNotional"] > 0 and price_dec is not None and amount: notional = qty_dec * price_dec if notional < filters["minNotional"]: raise RuntimeError( @@ -387,12 +649,24 @@ def place_order( params: Dict = { "symbol": binance_symbol, "side": side.upper(), - "type": order_type, - "quantity": format(qty_dec, "f"), + "type": otype, } - if order_type == "LIMIT": + if quote_order_qty is not None and otype == "MARKET": + params["quoteOrderQty"] = format(Decimal(str(quote_order_qty)), "f") + else: + params["quantity"] = format(qty_dec, "f") + + if otype in _BINANCE_LIMIT_TYPES: params["price"] = format(price_dec, "f") - params["timeInForce"] = "GTC" + params["timeInForce"] = (time_in_force or "GTC").upper() + if otype in _BINANCE_STOP_TYPES and stop_dec is not None: + params["stopPrice"] = format(stop_dec, "f") + if trailing_delta is not None: + params["trailingDelta"] = int(trailing_delta) + if iceberg_qty is not None: + params["icebergQty"] = format(Decimal(str(iceberg_qty)), "f") + if client_order_id: + params["newClientOrderId"] = client_order_id data = self._signed_request("POST", "/api/v3/order", params) @@ -410,6 +684,155 @@ def place_order( timestamp=data.get("transactTime", time.time() * 1000) / 1000, ) + # ------------------------------------------------------------------ + # OCO (One-Cancels-the-Other) - POST /api/v3/orderList/oco + # ------------------------------------------------------------------ + + def place_oco_order( + self, + symbol: str, + side: str, + quantity: float, + above_type: str, + below_type: str, + above_price: Optional[float] = None, + above_stop_price: Optional[float] = None, + above_time_in_force: Optional[str] = None, + below_price: Optional[float] = None, + below_stop_price: Optional[float] = None, + below_time_in_force: Optional[str] = None, + list_client_order_id: Optional[str] = None, + ) -> Dict: + """Place an OCO order list via the modern ``/api/v3/orderList/oco``. + + Each leg is described with ``aboveType`` / ``belowType`` + the leg's + price/stopPrice/timeInForce — this is the new schema Binance moved to + in 2024, replacing the legacy flat ``/api/v3/order/oco`` shape. + + Price restrictions are enforced server-side: + * SELL: LIMIT_MAKER price > last > STOP_LOSS_LIMIT stopPrice + * BUY: LIMIT_MAKER price < last < STOP_LOSS_LIMIT stopPrice + + Returns: + Raw Binance response dict (orderListId, contingencyType, orders, ...) + """ + if not self.api_key or not self.api_secret: + raise RuntimeError("Binance API credentials required for OCO placement") + + binance_symbol = self._convert_symbol(symbol) + filters = self._get_symbol_filters(binance_symbol) + qty_dec = self._round_to_step(Decimal(str(quantity)), filters["stepSize"]) + + if filters["minQty"] > 0 and qty_dec < filters["minQty"]: + raise RuntimeError( + f"Binance OCO {binance_symbol} quantity {qty_dec} below " + f"minQty {filters['minQty']}" + ) + + params: Dict = { + "symbol": binance_symbol, + "side": side.upper(), + "quantity": format(qty_dec, "f"), + "aboveType": above_type.upper(), + "belowType": below_type.upper(), + } + if above_price is not None: + params["abovePrice"] = format( + self._round_to_step(Decimal(str(above_price)), filters["tickSize"]), + "f", + ) + if above_stop_price is not None: + params["aboveStopPrice"] = format( + self._round_to_step( + Decimal(str(above_stop_price)), filters["tickSize"] + ), + "f", + ) + if above_time_in_force: + params["aboveTimeInForce"] = above_time_in_force.upper() + if below_price is not None: + params["belowPrice"] = format( + self._round_to_step(Decimal(str(below_price)), filters["tickSize"]), + "f", + ) + if below_stop_price is not None: + params["belowStopPrice"] = format( + self._round_to_step( + Decimal(str(below_stop_price)), filters["tickSize"] + ), + "f", + ) + if below_time_in_force: + params["belowTimeInForce"] = below_time_in_force.upper() + if list_client_order_id: + params["listClientOrderId"] = list_client_order_id + + return self._signed_request("POST", "/api/v3/orderList/oco", params) + + def cancel_order_list( + self, + symbol: str, + order_list_id: Optional[int] = None, + list_client_order_id: Optional[str] = None, + ) -> Dict: + """Cancel an entire OCO list via DELETE ``/api/v3/orderList``. + + Cancelling any single leg cancels the whole list per Binance spec, so + callers usually want this rather than two individual cancels. + """ + if not self.api_key or not self.api_secret: + raise RuntimeError("Binance API credentials required for OCO cancel") + if order_list_id is None and list_client_order_id is None: + raise ValueError( + "cancel_order_list requires order_list_id or list_client_order_id" + ) + binance_symbol = self._convert_symbol(symbol) + params: Dict = {"symbol": binance_symbol} + if order_list_id is not None: + params["orderListId"] = int(order_list_id) + if list_client_order_id is not None: + params["listClientOrderId"] = list_client_order_id + return self._signed_request("DELETE", "/api/v3/orderList", params) + + # ------------------------------------------------------------------ + # User-data stream (listenKey lifecycle) + # ------------------------------------------------------------------ + + def create_listen_key(self) -> str: + """POST /api/v3/userDataStream — returns a 60-min-valid listenKey. + + The caller is responsible for opening the WebSocket and calling + :meth:`keepalive_listen_key` every ~30 min. Closing via + :meth:`close_listen_key` is best practice but not required (Binance + expires the key automatically after 60 min of inactivity). + """ + if not self.api_key: + raise RuntimeError("Binance API key required to create a listenKey") + data = self._public_request("POST", "/api/v3/userDataStream") + if "listenKey" not in data: + raise RuntimeError(f"Binance create_listen_key bad response: {data}") + return data["listenKey"] + + def keepalive_listen_key(self, listen_key: str) -> None: + """PUT /api/v3/userDataStream?listenKey=X — extends the 60-min TTL. + + Call every ~30 min from the WS consumer thread to keep the stream + alive. Silently no-ops on success (Binance returns ``{}``). + """ + if not self.api_key: + raise RuntimeError("Binance API key required for keepalive") + self._public_request("PUT", f"/api/v3/userDataStream?listenKey={listen_key}") + + def close_listen_key(self, listen_key: str) -> None: + """DELETE /api/v3/userDataStream?listenKey=X — closes the stream.""" + if not self.api_key: + raise RuntimeError("Binance API key required to close listenKey") + self._public_request("DELETE", f"/api/v3/userDataStream?listenKey={listen_key}") + + def user_data_stream_url(self, listen_key: str) -> str: + """WebSocket URL for the given listenKey (prod or testnet).""" + return f"{self.ws_base}/{listen_key}" + def get_balance(self) -> Dict[str, float]: """ Retrieve balances for all assets via GET /api/v3/account. diff --git a/app/test_binance_exchange.py b/app/test_binance_exchange.py index 18a54bb1..e8e83978 100644 --- a/app/test_binance_exchange.py +++ b/app/test_binance_exchange.py @@ -6,17 +6,24 @@ import hashlib import hmac import unittest -from unittest.mock import patch +from unittest.mock import MagicMock, patch import sys sys.path.insert(0, ".") -from pt_exchanges import BinanceExchange +from pt_exchanges import ( + BinanceExchange, + BinanceRateLimitError, + BinanceTimestampError, +) -def _make_exchange(key="test_key", secret="test_secret"): - return BinanceExchange(api_key=key, api_secret=secret) +def _make_exchange(key="test_key", secret="test_secret", testnet=False): + ex = BinanceExchange(api_key=key, api_secret=secret, testnet=testnet) + # Skip /api/v3/time round-trip in tests; offset stays at 0 (good enough). + ex._time_synced = True + return ex def _sign(secret: str, params: str) -> str: @@ -368,5 +375,466 @@ def test_unexpected_error_propagates(self, mock_del): self.ex.cancel_order("BTCUSDT:123") +class TestBinanceTestnet(unittest.TestCase): + """Testnet flag flips REST + WebSocket base URLs.""" + + def test_default_is_production(self): + ex = BinanceExchange(api_key="k", api_secret="s") + self.assertEqual(ex.base_url, "https://api.binance.com") + self.assertEqual(ex.ws_base, "wss://stream.binance.com:9443/ws") + self.assertFalse(ex.testnet) + + def test_testnet_flag_switches_urls(self): + ex = BinanceExchange(api_key="k", api_secret="s", testnet=True) + self.assertEqual(ex.base_url, "https://testnet.binance.vision") + self.assertEqual(ex.ws_base, "wss://stream.testnet.binance.vision/ws") + self.assertTrue(ex.testnet) + + def test_recv_window_clamped_to_max(self): + ex = BinanceExchange(api_key="k", api_secret="s", recv_window=99999) + self.assertEqual(ex.recv_window, 60000) + + def test_recv_window_clamped_to_min(self): + ex = BinanceExchange(api_key="k", api_secret="s", recv_window=0) + self.assertEqual(ex.recv_window, 1) + + +class TestBinanceServerTimeSync(unittest.TestCase): + """sync_time + _now_ms offset behaviour.""" + + @patch("pt_exchanges.requests.get") + def test_sync_time_sets_positive_offset(self, mock_get): + mock_get.return_value.json.return_value = { + "serverTime": int(__import__("time").time() * 1000) + 5000 + } + ex = _make_exchange() + ex._time_synced = False # force a real sync + offset = ex.sync_time() + self.assertGreaterEqual(offset, 4000) # ~5000 ms + self.assertTrue(ex._time_synced) + + @patch("pt_exchanges.requests.get") + def test_sync_time_bad_response_raises(self, mock_get): + mock_get.return_value.json.return_value = {"unexpected": "shape"} + ex = _make_exchange() + ex._time_synced = False + with self.assertRaises(RuntimeError): + ex.sync_time() + + def test_now_ms_applies_offset(self): + ex = _make_exchange() + ex._time_offset_ms = 1234 + import time as _t + + actual = ex._now_ms() + expected = int(_t.time() * 1000) + 1234 + self.assertAlmostEqual(actual, expected, delta=200) + + @patch("pt_exchanges.requests.post") + @patch("pt_exchanges.requests.get") + def test_minus_1021_persists_raises_timestamp_error(self, mock_get, mock_post): + """Two consecutive -1021 responses (resync + retry both fail) → BinanceTimestampError.""" + mock_get.return_value.json.return_value = { + "serverTime": int(__import__("time").time() * 1000) + } + resp = MagicMock() + resp.status_code = 200 + resp.headers = {} + resp.json.return_value = {"code": -1021, "msg": "Timestamp drift"} + mock_post.return_value = resp + ex = _make_exchange() + _seed_filters(ex, "BTCUSDT") + with self.assertRaises(BinanceTimestampError): + ex.place_order("BTC-USD", "buy", 0.001) + + @patch("pt_exchanges.requests.post") + @patch("pt_exchanges.requests.get") + def test_minus_1021_triggers_resync_and_retry(self, mock_get, mock_post): + """First POST returns -1021. After /api/v3/time resync, retry succeeds.""" + # /api/v3/time response when resync triggers + mock_get.return_value.json.return_value = { + "serverTime": int(__import__("time").time() * 1000) + } + # POST fails once then succeeds + first = MagicMock() + first.status_code = 200 + first.headers = {} + first.json.return_value = {"code": -1021, "msg": "Timestamp drift"} + second = MagicMock() + second.status_code = 200 + second.headers = {} + second.json.return_value = { + "orderId": 1, + "status": "FILLED", + "executedQty": "0.001", + "price": "0", + "transactTime": 1000, + } + mock_post.side_effect = [first, second] + + ex = _make_exchange() + _seed_filters(ex, "BTCUSDT") + result = ex.place_order("BTC-USD", "buy", 0.001) + self.assertEqual(result.status, "filled") + # Retried exactly once + self.assertEqual(mock_post.call_count, 2) + + +class TestBinanceRateLimit(unittest.TestCase): + """HTTP 429/418 raise BinanceRateLimitError with Retry-After.""" + + @patch("pt_exchanges.requests.post") + def test_429_raises_rate_limit_error(self, mock_post): + resp = MagicMock() + resp.status_code = 429 + resp.headers = {"Retry-After": "12", "X-MBX-USED-WEIGHT-1M": "1200"} + resp.json.return_value = {"code": -1003, "msg": "Too many requests"} + mock_post.return_value = resp + + ex = _make_exchange() + _seed_filters(ex, "BTCUSDT") + with self.assertRaises(BinanceRateLimitError) as ctx: + ex.place_order("BTC-USD", "buy", 0.001) + self.assertEqual(ctx.exception.status_code, 429) + self.assertEqual(ctx.exception.retry_after, 12.0) + self.assertEqual(ex.last_rate_limit_headers.get("X-MBX-USED-WEIGHT-1M"), "1200") + + @patch("pt_exchanges.requests.post") + def test_418_raises_rate_limit_error(self, mock_post): + resp = MagicMock() + resp.status_code = 418 + resp.headers = {"Retry-After": "300"} + resp.json.return_value = {"code": -1003, "msg": "IP banned"} + mock_post.return_value = resp + ex = _make_exchange() + _seed_filters(ex, "BTCUSDT") + with self.assertRaises(BinanceRateLimitError) as ctx: + ex.place_order("BTC-USD", "buy", 0.001) + self.assertEqual(ctx.exception.status_code, 418) + + @patch("pt_exchanges.requests.post") + def test_used_weight_headers_captured(self, mock_post): + resp = MagicMock() + resp.status_code = 200 + resp.headers = { + "X-MBX-USED-WEIGHT-1M": "42", + "X-MBX-ORDER-COUNT-10S": "3", + "Content-Type": "application/json", + } + resp.json.return_value = { + "orderId": 1, + "status": "FILLED", + "executedQty": "0.001", + "price": "0", + "transactTime": 1, + } + mock_post.return_value = resp + ex = _make_exchange() + _seed_filters(ex, "BTCUSDT") + ex.place_order("BTC-USD", "buy", 0.001) + self.assertIn("X-MBX-USED-WEIGHT-1M", ex.last_rate_limit_headers) + self.assertIn("X-MBX-ORDER-COUNT-10S", ex.last_rate_limit_headers) + # Non-rate-limit headers excluded + self.assertNotIn("Content-Type", ex.last_rate_limit_headers) + + +class TestBinanceExtendedOrderTypes(unittest.TestCase): + """STOP_LOSS_LIMIT, TAKE_PROFIT_LIMIT, LIMIT_MAKER, quoteOrderQty, etc.""" + + def setUp(self): + self.ex = _make_exchange() + _seed_filters(self.ex, "BTCUSDT") + + @patch("pt_exchanges.requests.post") + def test_stop_loss_limit_sends_stop_price(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 1, + "status": "NEW", + "executedQty": "0", + "price": "70000", + "transactTime": 1, + } + self.ex.place_order( + "BTC-USD", + "sell", + 0.001, + order_type="STOP_LOSS_LIMIT", + price=70000.0, + stop_price=70500.0, + time_in_force="GTC", + ) + url = mock_post.call_args[0][0] + self.assertIn("type=STOP_LOSS_LIMIT", url) + self.assertIn("stopPrice=", url) + self.assertIn("timeInForce=GTC", url) + + @patch("pt_exchanges.requests.post") + def test_take_profit_limit_sends_stop_and_price(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 2, + "status": "NEW", + "executedQty": "0", + "price": "80000", + "transactTime": 1, + } + self.ex.place_order( + "BTC-USD", + "sell", + 0.001, + order_type="TAKE_PROFIT_LIMIT", + price=80000.0, + stop_price=79500.0, + ) + url = mock_post.call_args[0][0] + self.assertIn("type=TAKE_PROFIT_LIMIT", url) + self.assertIn("stopPrice=", url) + + @patch("pt_exchanges.requests.post") + def test_limit_maker_sends_price_no_stop(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 3, + "status": "NEW", + "executedQty": "0", + "price": "75000", + "transactTime": 1, + } + self.ex.place_order( + "BTC-USD", + "sell", + 0.001, + order_type="LIMIT_MAKER", + price=75000.0, + ) + url = mock_post.call_args[0][0] + self.assertIn("type=LIMIT_MAKER", url) + self.assertNotIn("stopPrice", url) + + @patch("pt_exchanges.requests.post") + def test_market_with_quote_order_qty(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 4, + "status": "FILLED", + "executedQty": "0", + "price": "0", + "transactTime": 1, + } + # quoteOrderQty path: amount=0 to bypass the both-given guard + self.ex.place_order( + "BTC-USD", + "buy", + 0, + order_type="MARKET", + quote_order_qty=100.0, + ) + url = mock_post.call_args[0][0] + self.assertIn("quoteOrderQty=", url) + self.assertNotIn("quantity=", url) + + @patch("pt_exchanges.requests.post") + def test_trailing_delta_sent_as_int(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 5, + "status": "NEW", + "executedQty": "0", + "price": "70000", + "transactTime": 1, + } + self.ex.place_order( + "BTC-USD", + "sell", + 0.001, + order_type="STOP_LOSS_LIMIT", + price=70000.0, + trailing_delta=500, + ) + url = mock_post.call_args[0][0] + self.assertIn("trailingDelta=500", url) + + @patch("pt_exchanges.requests.post") + def test_client_order_id_forwarded(self, mock_post): + mock_post.return_value.json.return_value = { + "orderId": 6, + "status": "FILLED", + "executedQty": "0.001", + "price": "0", + "transactTime": 1, + } + self.ex.place_order( + "BTC-USD", + "buy", + 0.001, + client_order_id="my-custom-id-123", + ) + url = mock_post.call_args[0][0] + self.assertIn("newClientOrderId=my-custom-id-123", url) + + def test_invalid_order_type_raises(self): + with self.assertRaises(ValueError): + self.ex.place_order("BTC-USD", "buy", 0.001, order_type="FUTURES_BRACKET") + + def test_limit_without_price_raises(self): + with self.assertRaises(ValueError): + self.ex.place_order("BTC-USD", "buy", 0.001, order_type="LIMIT") + + def test_stop_loss_limit_without_stop_raises(self): + with self.assertRaises(ValueError): + self.ex.place_order( + "BTC-USD", + "sell", + 0.001, + order_type="STOP_LOSS_LIMIT", + price=70000.0, + ) + + def test_market_with_both_qty_and_quote_raises(self): + with self.assertRaises(ValueError): + self.ex.place_order( + "BTC-USD", + "buy", + 0.001, + order_type="MARKET", + quote_order_qty=100.0, + ) + + +class TestBinanceOCO(unittest.TestCase): + def setUp(self): + self.ex = _make_exchange() + _seed_filters(self.ex, "BTCUSDT") + + @patch("pt_exchanges.requests.post") + def test_place_oco_uses_above_below_schema(self, mock_post): + mock_post.return_value.json.return_value = { + "orderListId": 42, + "contingencyType": "OCO", + "orders": [{"orderId": 1}, {"orderId": 2}], + } + result = self.ex.place_oco_order( + "BTC-USD", + "sell", + 0.001, + above_type="STOP_LOSS_LIMIT", + above_price=69000.0, + above_stop_price=68500.0, + above_time_in_force="GTC", + below_type="LIMIT_MAKER", + below_price=80000.0, + ) + url = mock_post.call_args[0][0] + self.assertIn("/api/v3/orderList/oco", url) + self.assertIn("aboveType=STOP_LOSS_LIMIT", url) + self.assertIn("belowType=LIMIT_MAKER", url) + self.assertIn("abovePrice=", url) + self.assertIn("aboveStopPrice=", url) + self.assertIn("belowPrice=", url) + self.assertEqual(result["orderListId"], 42) + + @patch("pt_exchanges.requests.delete") + def test_cancel_order_list_by_id(self, mock_del): + mock_del.return_value.json.return_value = {"orderListId": 42} + self.ex.cancel_order_list("BTC-USD", order_list_id=42) + url = mock_del.call_args[0][0] + self.assertIn("/api/v3/orderList", url) + self.assertIn("orderListId=42", url) + + def test_cancel_order_list_requires_some_id(self): + with self.assertRaises(ValueError): + self.ex.cancel_order_list("BTC-USD") + + def test_oco_missing_credentials_raises(self): + ex = BinanceExchange(api_key="", api_secret="") + with self.assertRaises(RuntimeError): + ex.place_oco_order( + "BTC-USD", + "sell", + 0.001, + above_type="STOP_LOSS_LIMIT", + below_type="LIMIT_MAKER", + ) + + +class TestBinanceListenKey(unittest.TestCase): + def setUp(self): + self.ex = _make_exchange() + + @patch("pt_exchanges.requests.post") + def test_create_returns_listen_key(self, mock_post): + mock_post.return_value.content = b'{"listenKey":"abc123"}' + mock_post.return_value.json.return_value = {"listenKey": "abc123"} + key = self.ex.create_listen_key() + self.assertEqual(key, "abc123") + url = mock_post.call_args[0][0] + self.assertIn("/api/v3/userDataStream", url) + # X-MBX-APIKEY required, no signature for listenKey endpoints + headers = mock_post.call_args[1]["headers"] + self.assertEqual(headers["X-MBX-APIKEY"], "test_key") + + @patch("pt_exchanges.requests.put") + def test_keepalive_puts_with_key_param(self, mock_put): + mock_put.return_value.content = b"{}" + mock_put.return_value.json.return_value = {} + self.ex.keepalive_listen_key("xyz789") + url = mock_put.call_args[0][0] + self.assertIn("listenKey=xyz789", url) + + @patch("pt_exchanges.requests.delete") + def test_close_deletes_with_key_param(self, mock_del): + mock_del.return_value.content = b"{}" + mock_del.return_value.json.return_value = {} + self.ex.close_listen_key("xyz789") + url = mock_del.call_args[0][0] + self.assertIn("listenKey=xyz789", url) + + def test_user_data_stream_url_prod(self): + url = self.ex.user_data_stream_url("k1") + self.assertEqual(url, "wss://stream.binance.com:9443/ws/k1") + + def test_user_data_stream_url_testnet(self): + ex = _make_exchange(testnet=True) + url = ex.user_data_stream_url("k2") + self.assertEqual(url, "wss://stream.testnet.binance.vision/ws/k2") + + def test_create_without_key_raises(self): + ex = BinanceExchange(api_key="", api_secret="") + with self.assertRaises(RuntimeError): + ex.create_listen_key() + + +class TestBinanceBrokerSelectorHelpers(unittest.TestCase): + """Hooks consumed by the broker-selector UI in #96.""" + + def test_masked_api_key_shows_last_four(self): + ex = BinanceExchange(api_key="abcdefghij1234", api_secret="x") + self.assertEqual(ex.get_masked_api_key(), "****1234") + + def test_masked_api_key_short_key(self): + ex = BinanceExchange(api_key="ab", api_secret="x") + self.assertEqual(ex.get_masked_api_key(), "****ab") + + def test_masked_api_key_empty(self): + ex = BinanceExchange(api_key="", api_secret="") + self.assertEqual(ex.get_masked_api_key(), "Not configured") + + def test_test_connection_no_creds_returns_false(self): + ex = BinanceExchange(api_key="", api_secret="") + self.assertFalse(ex.test_connection()) + + @patch("pt_exchanges.requests.get") + def test_test_connection_success(self, mock_get): + resp = MagicMock() + resp.status_code = 200 + resp.headers = {} + resp.json.return_value = {"balances": [], "canTrade": True} + mock_get.return_value = resp + ex = _make_exchange() + self.assertTrue(ex.test_connection()) + + @patch("pt_exchanges.requests.get") + def test_test_connection_swallows_errors(self, mock_get): + mock_get.side_effect = Exception("network down") + ex = _make_exchange() + self.assertFalse(ex.test_connection()) + + if __name__ == "__main__": unittest.main()