Feature/orderly#1
Conversation
WalkthroughAdds a new Orderly Network perpetual futures connector and supporting stack: constants, ed25519 auth, REST/WS public & private data sources, connector implementation, web helpers, config maps, mark-price support in framework, multiple Avellaneda strategy scripts, documentation, and small Cython build placeholders. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Strategy
participant Connector
participant Throttler
participant Auth
participant REST_API
Strategy->>Connector: _place_order(payload)
Connector->>Throttler: request_slot()
Throttler-->>Connector: slot_granted
Connector->>Auth: rest_authenticate(RESTRequest)
Auth-->>Connector: signed_request_headers
Connector->>REST_API: POST /v1/order (signed)
REST_API-->>Connector: response
Connector->>Connector: update_order_tracker(response)
Connector-->>Strategy: return order_id
sequenceDiagram
autonumber
participant Connector
participant WS
participant Auth
participant MsgHandler
participant InternalQueue
rect rgb(240,248,255)
Connector->>WS: connect(wss_url)
WS-->>Connector: connected
Connector->>Auth: get_ws_auth_payload()
Auth-->>Connector: auth_payload
Connector->>WS: send(auth_payload)
WS-->>Connector: auth_response
Connector->>WS: subscribe(channels...)
end
WS->>MsgHandler: incoming_message
MsgHandler->>InternalQueue: enqueue(parsed_message)
sequenceDiagram
autonumber
participant Connector
participant BatchAPI
participant OrderTracker
Connector->>BatchAPI: POST /v1/batch-order (orders[])
BatchAPI-->>Connector: batch_response
Connector->>OrderTracker: update_in_flight_orders(batch_response)
loop polling
Connector->>BatchAPI: GET /v1/orders
BatchAPI-->>Connector: statuses
Connector->>OrderTracker: reconcile(statuses)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Key areas for careful review:
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used🧬 Code graph analysis (1)scripts/pmm_kodiak.py (7)
🪛 Flake8 (7.3.0)scripts/pmm_kodiak.py[error] 139-139: unexpected indentation (comment) (E116) 🪛 Ruff (0.14.4)scripts/pmm_kodiak.py298-298: Do not catch blind exception: (BLE001) 🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
hummingbot/connector/derivative/orderly_perpetual/Progress.md(1 hunks)hummingbot/connector/derivative/orderly_perpetual/dummy.pxd(1 hunks)hummingbot/connector/derivative/orderly_perpetual/dummy.pyx(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_constants.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_user_stream_data_source.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_utils.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py(1 hunks)scripts/kodiak/pmm_avellaneda.py(1 hunks)scripts/kodiak/pmm_avellaneda_algo.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_utils.py (2)
hummingbot/client/config/config_data_types.py (1)
BaseConnectorConfigMap(73-88)hummingbot/core/data_type/trade_fee.py (1)
TradeFeeSchema(39-71)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_user_stream_data_source.py (6)
hummingbot/core/web_assistant/connections/data_types.py (2)
json(112-125)WSJSONRequest(139-145)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (2)
account_id(277-279)get_ws_auth_payload(281-320)hummingbot/core/data_type/user_stream_tracker_data_source.py (1)
UserStreamTrackerDataSource(11-126)hummingbot/core/web_assistant/web_assistants_factory.py (1)
get_ws_assistant(61-66)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (1)
wss_url(109-152)hummingbot/core/web_assistant/connections/ws_connection.py (1)
receive(62-71)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (7)
hummingbot/core/data_type/common.py (2)
TradeType(60-63)get(121-124)hummingbot/core/web_assistant/connections/data_types.py (6)
RESTMethod(14-25)WSJSONRequest(139-145)url(93-95)method(98-100)RESTRequest(29-37)json(112-125)hummingbot/core/web_assistant/web_assistants_factory.py (2)
WebAssistantsFactory(14-81)get_ws_assistant(61-66)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (5)
trading_pairs(166-168)domain(136-138)exchange_symbol_associated_to_pair(322-356)authenticator(101-128)_make_network_check_request(486-495)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (5)
public_rest_url(77-90)is_exchange_information_valid(225-263)format_trading_pair(266-293)private_rest_url(93-106)wss_url(109-152)hummingbot/core/web_assistant/rest_assistant.py (2)
execute_request(35-59)call(104-110)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
account_id(277-279)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (5)
hummingbot/core/api_throttler/async_throttler.py (1)
AsyncThrottler(54-81)hummingbot/core/web_assistant/connections/data_types.py (4)
RESTMethod(14-25)RESTRequest(29-37)headers(108-110)method(98-100)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (1)
domain(136-138)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
account_id(277-279)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
hummingbot/core/web_assistant/connections/data_types.py (6)
RESTMethod(14-25)RESTRequest(29-37)WSRequest(132-135)method(98-100)url(93-95)headers(108-110)
scripts/kodiak/pmm_avellaneda.py (7)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (4)
OrderType(8-15)PriceType(50-57)PositionAction(31-34)TradeType(60-63)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (7)
BuyOrderCompletedEvent(86-94)BuyOrderCreatedEvent(223-233)MarketOrderFailureEvent(77-82)OrderCancelledEvent(110-113)OrderFilledEvent(161-219)SellOrderCompletedEvent(98-106)SellOrderCreatedEvent(237-247)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/strategy_v2/models/executors.py (2)
TrackedOrder(21-124)price(50-54)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (5)
hummingbot/core/web_assistant/connections/data_types.py (5)
json(112-125)RESTMethod(14-25)RESTRequest(29-37)url(93-95)method(98-100)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
account_id(277-279)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (5)
build_api_factory(155-175)format_trading_pair(266-293)public_rest_url(77-90)is_exchange_information_valid(225-263)private_rest_url(93-106)hummingbot/core/web_assistant/rest_assistant.py (2)
execute_request(35-59)call(104-110)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_constants.py (1)
hummingbot/core/data_type/in_flight_order.py (1)
OrderState(25-36)
🪛 Flake8 (7.3.0)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_user_stream_data_source.py
[error] 253-253: f-string is missing placeholders
(F541)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py
[error] 34-34: 'hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS' imported but unused
(F401)
[error] 36-36: 'hummingbot.core.web_assistant.connections.data_types.RESTMethod' imported but unused
(F401)
scripts/kodiak/pmm_avellaneda.py
[error] 192-192: f-string is missing placeholders
(F541)
[error] 204-204: f-string is missing placeholders
(F541)
[error] 247-247: local variable 'connector' is assigned to but never used
(F841)
[error] 384-384: continuation line over-indented for visual indent
(E127)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.get_new_client_order_id' imported but unused
(F401)
🪛 LanguageTool
scripts/kodiak/pmm_avellaneda_algo.md
[grammar] ~1-~1: Use a hyphen to join words.
Context: Market Making Strategies for Crypto Perpetual F...
(QB_NEW_EN_HYPHEN)
[grammar] ~43-~43: Ensure spelling is correct
Context: ...ynamic sizing encourages mean reversion: pythondef calculate_order_sizes(base_size, invent...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
hummingbot/connector/derivative/orderly_perpetual/Progress.md
625-625: Bare URL used
(MD034, no-bare-urls)
626-626: Bare URL used
(MD034, no-bare-urls)
627-627: Bare URL used
(MD034, no-bare-urls)
628-628: Bare URL used
(MD034, no-bare-urls)
629-629: Bare URL used
(MD034, no-bare-urls)
630-630: Bare URL used
(MD034, no-bare-urls)
633-633: Bare URL used
(MD034, no-bare-urls)
634-634: Bare URL used
(MD034, no-bare-urls)
635-635: Bare URL used
(MD034, no-bare-urls)
735-735: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
737-737: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🪛 Ruff (0.14.3)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_user_stream_data_source.py
129-129: Consider moving this statement to an else block
(TRY300)
134-134: Use explicit conversion flag
Replace with conversion flag
(RUF010)
253-253: f-string without any placeholders
Remove extraneous f prefix
(F541)
282-282: Abstract raise to an inner function
(TRY301)
282-282: Avoid specifying long messages outside the exception class
(TRY003)
297-297: Abstract raise to an inner function
(TRY301)
297-297: Avoid specifying long messages outside the exception class
(TRY003)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py
53-53: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
93-93: Unused method argument: domain
(ARG002)
156-156: Avoid specifying long messages outside the exception class
(TRY003)
201-201: Do not catch blind exception: Exception
(BLE001)
211-211: Do not catch blind exception: Exception
(BLE001)
241-241: Avoid specifying long messages outside the exception class
(TRY003)
345-345: Do not catch blind exception: Exception
(BLE001)
407-407: Avoid specifying long messages outside the exception class
(TRY003)
470-470: Consider moving this statement to an else block
(TRY300)
475-475: Use explicit conversion flag
Replace with conversion flag
(RUF010)
542-542: Use explicit conversion flag
Replace with conversion flag
(RUF010)
643-643: Do not catch blind exception: Exception
(BLE001)
692-692: Do not catch blind exception: Exception
(BLE001)
765-765: Do not catch blind exception: Exception
(BLE001)
818-818: Consider moving this statement to an else block
(TRY300)
820-820: Do not catch blind exception: Exception
(BLE001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py
137-137: Avoid specifying long messages outside the exception class
(TRY003)
142-142: Avoid specifying long messages outside the exception class
(TRY003)
152-152: Avoid specifying long messages outside the exception class
(TRY003)
209-209: Unused function argument: throttler
(ARG001)
209-209: Unused function argument: domain
(ARG001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py
81-81: Abstract raise to an inner function
(TRY301)
81-81: Avoid specifying long messages outside the exception class
(TRY003)
88-88: Abstract raise to an inner function
(TRY301)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Do not catch blind exception: Exception
(BLE001)
93-93: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
93-93: Avoid specifying long messages outside the exception class
(TRY003)
scripts/kodiak/pmm_avellaneda.py
192-192: f-string without any placeholders
Remove extraneous f prefix
(F541)
204-204: f-string without any placeholders
Remove extraneous f prefix
(F541)
247-247: Local variable connector is assigned to but never used
Remove assignment to unused variable connector
(F841)
370-370: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
375-375: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
376-376: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
382-382: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
386-386: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
62-62: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
63-63: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
64-64: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
203-203: Unused method argument: trading_pair
(ARG002)
207-207: Unused method argument: trading_pair
(ARG002)
260-260: Do not catch blind exception: Exception
(BLE001)
304-304: Do not catch blind exception: Exception
(BLE001)
340-340: Abstract raise to an inner function
(TRY301)
340-340: Avoid specifying long messages outside the exception class
(TRY003)
347-347: Consider moving this statement to an else block
(TRY300)
406-406: Avoid specifying long messages outside the exception class
(TRY003)
439-439: Avoid specifying long messages outside the exception class
(TRY003)
477-477: Do not catch blind exception: Exception
(BLE001)
497-497: Unused method argument: request_exception
(ARG002)
527-527: Unused method argument: kwargs
(ARG002)
632-632: Unused method argument: kwargs
(ARG002)
685-685: Avoid specifying long messages outside the exception class
(TRY003)
718-718: Avoid specifying long messages outside the exception class
(TRY003)
747-747: Avoid specifying long messages outside the exception class
(TRY003)
828-828: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
828-828: Avoid specifying long messages outside the exception class
(TRY003)
829-829: Do not catch blind exception: Exception
(BLE001)
892-892: Do not catch blind exception: Exception
(BLE001)
931-931: Do not catch blind exception: Exception
(BLE001)
932-932: Use explicit conversion flag
Replace with conversion flag
(RUF010)
945-945: Unused method argument: trading_pair
(ARG002)
1041-1041: Consider moving this statement to an else block
(TRY300)
1043-1043: Do not catch blind exception: Exception
(BLE001)
1067-1067: Unused method argument: position_action
(ARG002)
1122-1122: Do not catch blind exception: Exception
(BLE001)
1149-1149: Unused method argument: event
(ARG002)
1154-1154: Unused method argument: event
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (2)
hummingbot/connector/derivative/orderly_perpetual/Progress.md (1)
1-736: Nice implementation timelineThe phased progress log is thorough and makes it easy to trace what shipped when. Thanks for documenting it so clearly.
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_constants.py (1)
1-335: Constant map looks solidThe endpoint paths, rate limits, and channel names line up with the latest Orderly docs—nice comprehensive coverage.
| ws_url = web_utils.wss_url("public", self._domain, self._connector.authenticator.account_id) # account_id is a mandatory parameter for public WebSocket URL | ||
| self.logger().info(f"[WEBSOCKET] Attempting to connect to public WebSocket: {ws_url}") | ||
|
|
||
| try: | ||
| ws: WSAssistant = await self._api_factory.get_ws_assistant() | ||
| self.logger().debug(f"[WEBSOCKET] WSAssistant created, connecting to: {ws_url}") | ||
|
|
||
| await ws.connect( | ||
| ws_url=ws_url, | ||
| ping_timeout=CONSTANTS.HEARTBEAT_TIME_INTERVAL | ||
| ) | ||
|
|
||
| self.logger().info(f"[WEBSOCKET] Successfully connected to public WebSocket: {ws_url}") | ||
| return ws | ||
|
|
||
| except Exception as e: | ||
| self.logger().error( | ||
| f"[WEBSOCKET] Failed to connect to public WebSocket: {ws_url}. " | ||
| f"Error type: {type(e).__name__}, Error: {str(e)}", |
There was a problem hiding this comment.
Guard when authenticator is absent
self._connector.authenticator returns None whenever the connector is instantiated without full API creds (e.g., trading_required=False for data-only usage). In that case this line raises AttributeError, and the public WebSocket never connects. Please fall back to the configured account_id (or fail fast with a clear error) instead of blindly dereferencing the authenticator.
- ws_url = web_utils.wss_url("public", self._domain, self._connector.authenticator.account_id) # account_id is a mandatory parameter for public WebSocket URL
+ account_id = getattr(self._connector._auth, "account_id", None) if getattr(self._connector, "_auth", None) else getattr(self._connector, "_orderly_perpetual_account_id", None)
+ if not account_id:
+ raise ValueError("Orderly public WebSocket requires account_id, but none is configured.")
+ ws_url = web_utils.wss_url("public", self._domain, account_id)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.3)
470-470: Consider moving this statement to an else block
(TRY300)
475-475: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
In
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py
around lines 457 to 475, guard access to self._connector.authenticator before
dereferencing account_id: first determine account_id by using
authenticator.account_id when authenticator is not None, otherwise fall back to
the connector's configured account_id (e.g., self._connector.account_id or the
appropriate stored config field); if neither is available, raise a clear error
(or log and fail fast) explaining that an account_id is required for the public
WebSocket URL. Update the ws_url construction and surrounding logs to use this
resolved account_id and ensure error handling messages reflect the
fallback/validation result.
| from hummingbot.connector.derivative.orderly_perpetual import orderly_perpetual_constants as CONSTANTS | ||
| from hummingbot.core.web_assistant.auth import AuthBase | ||
| from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest, WSRequest |
There was a problem hiding this comment.
Fix lint: drop unused imports
CONSTANTS and RESTMethod aren’t used anywhere in this module, so flake8 flags them with F401. Please remove the dead imports so the lint step passes.
-from hummingbot.connector.derivative.orderly_perpetual import orderly_perpetual_constants as CONSTANTS
-from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest, WSRequest
+from hummingbot.core.web_assistant.connections.data_types import RESTRequest, WSRequest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from hummingbot.connector.derivative.orderly_perpetual import orderly_perpetual_constants as CONSTANTS | |
| from hummingbot.core.web_assistant.auth import AuthBase | |
| from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest, WSRequest | |
| from hummingbot.core.web_assistant.auth import AuthBase | |
| from hummingbot.core.web_assistant.connections.data_types import RESTRequest, WSRequest |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 34-34: 'hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS' imported but unused
(F401)
[error] 36-36: 'hummingbot.core.web_assistant.connections.data_types.RESTMethod' imported but unused
(F401)
🤖 Prompt for AI Agents
In hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py
around lines 34 to 36, the imports of CONSTANTS and RESTMethod are unused and
cause flake8 F401; remove these dead imports (either delete CONSTANTS from the
orderly_perpetual_constants import and remove RESTMethod from the
connections.data_types import) and ensure no other code in the file references
them so lint will pass.
| from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple | ||
|
|
||
| from bidict import bidict | ||
|
|
||
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS | ||
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_web_utils as web_utils | ||
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_api_order_book_data_source import ( | ||
| OrderlyPerpetualAPIOrderBookDataSource, | ||
| ) | ||
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_auth import OrderlyPerpetualAuth | ||
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_user_stream_data_source import ( | ||
| OrderlyPerpetualUserStreamDataSource, | ||
| ) | ||
| from hummingbot.connector.derivative.position import Position | ||
| from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase | ||
| from hummingbot.connector.trading_rule import TradingRule | ||
| from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id | ||
| from hummingbot.core.api_throttler.data_types import RateLimit |
There was a problem hiding this comment.
Remove unused imports
TYPE_CHECKING, combine_to_hb_trading_pair, and get_new_client_order_id never get referenced, so flake8 complains (F401). Please drop the unused imports to keep lint green.
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Optional, Tuple
@@
-from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple | |
| from bidict import bidict | |
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS | |
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_web_utils as web_utils | |
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_api_order_book_data_source import ( | |
| OrderlyPerpetualAPIOrderBookDataSource, | |
| ) | |
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_auth import OrderlyPerpetualAuth | |
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_user_stream_data_source import ( | |
| OrderlyPerpetualUserStreamDataSource, | |
| ) | |
| from hummingbot.connector.derivative.position import Position | |
| from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase | |
| from hummingbot.connector.trading_rule import TradingRule | |
| from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id | |
| from hummingbot.core.api_throttler.data_types import RateLimit | |
| from typing import Any, Dict, List, Optional, Tuple | |
| from bidict import bidict | |
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS | |
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_web_utils as web_utils | |
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_api_order_book_data_source import ( | |
| OrderlyPerpetualAPIOrderBookDataSource, | |
| ) | |
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_auth import OrderlyPerpetualAuth | |
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_user_stream_data_source import ( | |
| OrderlyPerpetualUserStreamDataSource, | |
| ) | |
| from hummingbot.connector.derivative.position import Position | |
| from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase | |
| from hummingbot.connector.trading_rule import TradingRule | |
| from hummingbot.core.api_throttler.data_types import RateLimit |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.get_new_client_order_id' imported but unused
(F401)
🤖 Prompt for AI Agents
In
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
around lines 20 to 37, there are unused imports causing flake8 F401 errors;
remove TYPE_CHECKING from the typing import list and drop
combine_to_hb_trading_pair and get_new_client_order_id from the
hummingbot.connector.utils import so the file only imports names that are
actually referenced.
| pythondef market_making_cycle(): | ||
| # 1. Gather current state (WebSocket data, <50ms) | ||
| mid_price = get_current_mid_price() | ||
| inventory = get_current_position() | ||
| volatility = calculate_rolling_volatility(window=200) | ||
|
|
||
| # 2. Check if should quote (defensive filtering) | ||
| if not should_provide_liquidity(volatility, ROC_indicator, order_imbalance): | ||
| cancel_all_orders() # Takes ~500-1000ms | ||
| return | ||
|
|
||
| # 3. Calculate new quotes | ||
| reservation_price = calculate_reservation_price(mid_price, inventory, volatility) | ||
| spread = calculate_optimal_spread(volatility, inventory, latency=1.0) | ||
| bid_size, ask_size = calculate_order_sizes(BASE_SIZE, inventory) | ||
|
|
||
| # 4. Replace orders (single batched API call, ~500-1000ms) | ||
| new_bid = reservation_price - spread/2 | ||
| new_ask = reservation_price + spread/2 | ||
|
|
||
| replace_orders_atomic( | ||
| cancel_orders=[existing_bid_id, existing_ask_id], | ||
| new_orders=[ | ||
| {'side': 'buy', 'price': new_bid, 'size': bid_size}, | ||
| {'side': 'sell', 'price': new_ask, 'size': ask_size} | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Fix the broken code block markup.
The snippets are prefixed with pythondef, so they don’t render as runnable examples and the language hint is lost. Please wrap the blocks with fenced Markdown code (python … ), which also restores syntax highlighting.
Apply this diff to fix the code fence:
-pythondef market_making_cycle():
+```python
+def market_making_cycle():
@@
- )
+ )Remember to add the terminating triple backtick after the snippet as well.
<details>
<summary>🤖 Prompt for AI Agents</summary>
scripts/kodiak/pmm_avellaneda_algo.md lines 59-85: the fenced code block is
malformed (starts with "pythondef" and lacks proper triple-backtick
termination), so replace the opening "pythondef" with a proper fenced code block
marker "python" before the "def market_making_cycle()" line and add a closing "" after the final ")" to terminate the block; ensure indentation and content
remain unchanged so the snippet renders with Python syntax highlighting.
</details>
<!-- fingerprinting:phantom:medusa:sabertoothed -->
<!-- This is an auto-generated comment by CodeRabbit -->
| for idx, bid_spread in enumerate(self.config.bid_spread_levels): | ||
| ask_spread = self.config.ask_spread_levels[idx] | ||
| bid_price = reservation_price * (Decimal("1") - bid_spread) | ||
| ask_price = reservation_price * (Decimal("1") + ask_spread) | ||
|
|
||
| # Convert quote amount to base amount for both buy and sell orders | ||
| # For perpetual orders, amount must be in base currency (BTC), not quote (USDC) | ||
| bid_amount = Decimal(self.config.order_amount_quote) / bid_price | ||
| ask_amount = Decimal(self.config.order_amount_quote) / ask_price | ||
|
|
||
| bid_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT, | ||
| order_side=TradeType.BUY, | ||
| amount=bid_amount, | ||
| price=bid_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| ask_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT, | ||
| order_side=TradeType.SELL, | ||
| amount=ask_amount, | ||
| price=ask_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| if self._current_inventory >= self.config.max_inventory: | ||
| orders.extend([bid_order]) | ||
| elif self._current_inventory <= -self.config.max_inventory: | ||
| orders.extend([ask_order]) | ||
| else: | ||
| orders.extend([bid_order, ask_order]) | ||
| return orders |
There was a problem hiding this comment.
Guard against mismatched spread level lists.
Iterating with idx assumes ask_spread_levels is at least as long as bid_spread_levels; a shorter list will throw IndexError and abort the tick. Validate lengths (or iterate with zip) so misconfigured presets fail fast and safely.
Apply this diff to harden the logic:
- for idx, bid_spread in enumerate(self.config.bid_spread_levels):
- ask_spread = self.config.ask_spread_levels[idx]
+ if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels):
+ raise ValueError("bid_spread_levels and ask_spread_levels must have identical lengths.")
+
+ for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels):
raise ValueError("bid_spread_levels and ask_spread_levels must have identical lengths.")
for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):
bid_price = reservation_price * (Decimal("1") - bid_spread)
ask_price = reservation_price * (Decimal("1") + ask_spread)
# Convert quote amount to base amount for both buy and sell orders
# For perpetual orders, amount must be in base currency (BTC), not quote (USDC)
bid_amount = Decimal(self.config.order_amount_quote) / bid_price
ask_amount = Decimal(self.config.order_amount_quote) / ask_price
bid_order = PerpetualOrderCandidate(
trading_pair=self.config.trading_pair,
is_maker=True,
order_type=OrderType.LIMIT,
order_side=TradeType.BUY,
amount=bid_amount,
price=bid_price,
leverage=Decimal(self.config.leverage)
)
ask_order = PerpetualOrderCandidate(
trading_pair=self.config.trading_pair,
is_maker=True,
order_type=OrderType.LIMIT,
order_side=TradeType.SELL,
amount=ask_amount,
price=ask_price,
leverage=Decimal(self.config.leverage)
)
if self._current_inventory >= self.config.max_inventory:
orders.extend([bid_order])
elif self._current_inventory <= -self.config.max_inventory:
orders.extend([ask_order])
else:
orders.extend([bid_order, ask_order])
return orders
🤖 Prompt for AI Agents
In scripts/kodiak/pmm_avellaneda.py around lines 142-178, the loop indexes into
self.config.ask_spread_levels using enumerate over bid_spread_levels which can
raise IndexError if the two lists differ in length; add a guard to validate the
two spread-level lists are the same length at the start of the method (raise a
clear ValueError with context if not), or change the loop to iterate using
zip(self.config.bid_spread_levels, self.config.ask_spread_levels) so pairs are
processed safely; ensure any error message includes both list lengths and the
trading_pair so misconfigured presets fail fast and clearly.
| self.logger().info(f"After populate_collateral_entries:") | ||
| self.logger().info(f" order_collateral: {populated.order_collateral}") | ||
| self.logger().info(f" percent_fee_collateral: {populated.percent_fee_collateral}") | ||
| self.logger().info(f" amount: {populated.amount}") | ||
| self.logger().info(f" leverage: {populated.leverage}") | ||
| self.logger().info(f" position_close: {populated.position_close}") | ||
|
|
||
| proposal_adjusted = budget_checker.adjust_candidates(proposal, all_or_none=True) | ||
|
|
||
| # Debug: Check what happened after adjustment | ||
| if proposal_adjusted: | ||
| first_adjusted = proposal_adjusted[0] | ||
| self.logger().info(f"After adjust_candidates:") | ||
| self.logger().info(f" order_collateral: {first_adjusted.order_collateral}") | ||
| self.logger().info(f" amount: {first_adjusted.amount}") |
There was a problem hiding this comment.
Remove empty f-strings to satisfy lint.
f"After populate_collateral_entries:" and f"After adjust_candidates:" trigger Flake8 F541 because they have no placeholders, which will fail CI. Drop the f prefix (or add data) before merging.
Apply this diff to fix both log lines:
- self.logger().info(f"After populate_collateral_entries:")
+ self.logger().info("After populate_collateral_entries:")
@@
- self.logger().info(f"After adjust_candidates:")
+ self.logger().info("After adjust_candidates:")
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
self.logger().info("After populate_collateral_entries:")
self.logger().info(f" order_collateral: {populated.order_collateral}")
self.logger().info(f" percent_fee_collateral: {populated.percent_fee_collateral}")
self.logger().info(f" amount: {populated.amount}")
self.logger().info(f" leverage: {populated.leverage}")
self.logger().info(f" position_close: {populated.position_close}")
proposal_adjusted = budget_checker.adjust_candidates(proposal, all_or_none=True)
# Debug: Check what happened after adjustment
if proposal_adjusted:
first_adjusted = proposal_adjusted[0]
self.logger().info("After adjust_candidates:")
self.logger().info(f" order_collateral: {first_adjusted.order_collateral}")
self.logger().info(f" amount: {first_adjusted.amount}")
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 192-192: f-string is missing placeholders
(F541)
[error] 204-204: f-string is missing placeholders
(F541)
🪛 Ruff (0.14.3)
192-192: f-string without any placeholders
Remove extraneous f prefix
(F541)
204-204: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In scripts/kodiak/pmm_avellaneda.py around lines 192 to 206, the two logger.info
calls use f-strings without placeholders (e.g. f"After
populate_collateral_entries:" and f"After adjust_candidates:"), which triggers
Flake8 F541; remove the unnecessary f prefix (or add real placeholders) so those
calls use plain strings (e.g. "After populate_collateral_entries:" and "After
adjust_candidates:") to satisfy linting.
| connector = self.connectors[self.config.exchange] | ||
| active_orders = self.get_active_orders(connector_name=self.config.exchange) |
There was a problem hiding this comment.
Drop the unused connector assignment.
connector isn’t referenced later, so Flake8 flags F841 and the build breaks. Just remove the assignment.
Apply this diff to eliminate the unused variable:
- connector = self.connectors[self.config.exchange]
- active_orders = self.get_active_orders(connector_name=self.config.exchange)
+ active_orders = self.get_active_orders(connector_name=self.config.exchange)
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
active_orders = self.get_active_orders(connector_name=self.config.exchange)
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 247-247: local variable 'connector' is assigned to but never used
(F841)
🪛 Ruff (0.14.3)
247-247: Local variable connector is assigned to but never used
Remove assignment to unused variable connector
(F841)
🤖 Prompt for AI Agents
In scripts/kodiak/pmm_avellaneda.py around lines 247 to 248, remove the unused
assignment "connector = self.connectors[self.config.exchange]" because the
variable is never used (Flake8 F841). Simply delete that line and keep the call
that retrieves active orders: call
self.get_active_orders(connector_name=self.config.exchange) unchanged, ensuring
no other references to "connector" exist in nearby code.
| # Calculate inventory adjustment: q·γ·σ²·τ | ||
| inventory_adjustment = inventory * self.config.risk_aversion_gamma * \ | ||
| (self.config.volatility_sigma ** Decimal("2")) * tau_years | ||
|
|
||
| # Reservation price: r = s - q·γ·σ²·τ |
There was a problem hiding this comment.
Fix the over-indented continuation line.
The backslash continuation violates Flake8 E127, so linting fails. Wrap the expression in parentheses instead.
Apply this diff to comply:
- inventory_adjustment = inventory * self.config.risk_aversion_gamma * \
- (self.config.volatility_sigma ** Decimal("2")) * tau_years
+ inventory_adjustment = (
+ inventory
+ * self.config.risk_aversion_gamma
+ * (self.config.volatility_sigma ** Decimal("2"))
+ * tau_years
+ )
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
# Calculate inventory adjustment: q·γ·σ²·τ
inventory_adjustment = (
inventory
* self.config.risk_aversion_gamma
* (self.config.volatility_sigma ** Decimal("2"))
* tau_years
)
# Reservation price: r = s - q·γ·σ²·τ
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 384-384: continuation line over-indented for visual indent
(E127)
🪛 Ruff (0.14.3)
382-382: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
386-386: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
🤖 Prompt for AI Agents
In scripts/kodiak/pmm_avellaneda.py around lines 382 to 386, the
inventory_adjustment assignment currently uses a backslash line continuation
which triggers Flake8 E127; remove the trailing backslash and instead wrap the
whole right-hand expression in parentheses so the multiline continuation is
implicit, align the lines inside the parentheses with one level of indentation,
and keep the comment and surrounding logic unchanged.
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
scripts/pmm_avellaneda.py (4)
143-170: Guard against mismatched spread lists before iterating.Indexing
ask_spread_levels[idx]will raiseIndexErrorwhen the two spread lists get out of sync (misconfigured presets, user edits, etc.), killing the strategy mid-tick. Validate the lengths (or iterate withzip) so we fail fast with a clear message.- for idx, bid_spread in enumerate(self.config.bid_spread_levels): - ask_spread = self.config.ask_spread_levels[idx] + if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError( + f"bid_spread_levels ({len(self.config.bid_spread_levels)}) and " + f"ask_spread_levels ({len(self.config.ask_spread_levels)}) must match for {self.config.trading_pair}." + ) + + for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):
194-208: Remove empty f-strings to satisfy lint.Both log lines are plain strings; the
fprefix triggers Flake8 F541 and breaks linting. Drop thef.- self.logger().info(f"After populate_collateral_entries:") + self.logger().info("After populate_collateral_entries:") @@ - self.logger().info(f"After adjust_candidates:") + self.logger().info("After adjust_candidates:")
249-252: Remove the unusedconnectorassignment.
connectorisn’t referenced, so Flake8 flags F841. Delete the assignment to keep lint passing.- connector = self.connectors[self.config.exchange] active_orders = self.get_active_orders(connector_name=self.config.exchange)
384-389: Fix the over-indented continuation line.The backslash continuation violates Flake8 E127. Wrap the expression in parentheses to keep indentation legal and lint happy.
- inventory_adjustment = inventory * self.config.risk_aversion_gamma * \ - (self.config.volatility_sigma ** Decimal("2")) * tau_years + inventory_adjustment = ( + inventory + * self.config.risk_aversion_gamma + * (self.config.volatility_sigma ** Decimal("2")) + * tau_years + )hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
34-36: Fix lingering unused imports to satisfy lint
CONSTANTSandRESTMethodare still unused, so Flake8 raises F401 and the build stays blocked. Please drop them from the imports.-from hummingbot.connector.derivative.orderly_perpetual import orderly_perpetual_constants as CONSTANTS -from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest, WSRequest +from hummingbot.core.web_assistant.connections.data_types import RESTRequest, WSRequesthummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (1)
446-467: Guard against missing authenticator when building the WS URL
self._connector.authenticatorisNonewhenever the connector runs in data-only mode, so this dereference raisesAttributeErrorand the public stream never comes up. Resolve the account_id defensively (use the existing auth object when present, otherwise fall back to the configured account id, and fail fast with a clear message if neither is set) before callingweb_utils.wss_url.- ws_url = web_utils.wss_url("public", self._domain, self._connector.authenticator.account_id) # account_id is a mandatory parameter for public WebSocket URL + authenticator = getattr(self._connector, "_auth", None) + account_id = getattr(authenticator, "account_id", None) + if account_id is None: + account_id = getattr(self._connector, "_orderly_perpetual_account_id", None) + if account_id is None: + raise ValueError("Orderly public WebSocket requires an account_id; none was provided.") + ws_url = web_utils.wss_url("public", self._domain, account_id)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CONNECTOR_ARCHITECTURE_EXPLANATION.md(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_constants.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_utils.py(1 hunks)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py(1 hunks)scripts/pmm_avellaneda.py(1 hunks)scripts/pmm_avellaneda_batch.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_utils.py
🧰 Additional context used
🧬 Code graph analysis (7)
scripts/pmm_avellaneda.py (7)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (4)
OrderType(8-15)PriceType(50-57)PositionAction(31-34)TradeType(60-63)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (7)
BuyOrderCompletedEvent(86-94)BuyOrderCreatedEvent(223-233)MarketOrderFailureEvent(77-82)OrderCancelledEvent(110-113)OrderFilledEvent(161-219)SellOrderCompletedEvent(98-106)SellOrderCreatedEvent(237-247)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/strategy_v2/models/executors.py (2)
TrackedOrder(21-124)price(50-54)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (5)
hummingbot/core/api_throttler/async_throttler.py (1)
AsyncThrottler(54-81)hummingbot/core/web_assistant/connections/data_types.py (4)
RESTMethod(14-25)RESTRequest(29-37)headers(108-110)method(98-100)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (1)
domain(124-126)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
account_id(274-276)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (6)
hummingbot/core/data_type/common.py (2)
TradeType(60-63)get(121-124)hummingbot/core/web_assistant/connections/data_types.py (6)
RESTMethod(14-25)WSJSONRequest(139-145)url(93-95)method(98-100)RESTRequest(29-37)json(112-125)hummingbot/core/web_assistant/web_assistants_factory.py (2)
WebAssistantsFactory(14-81)get_ws_assistant(61-66)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (5)
OrderlyPerpetualDerivative(49-1864)domain(124-126)exchange_symbol_associated_to_pair(278-304)authenticator(101-116)_make_network_check_request(434-443)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (3)
public_rest_url(72-85)private_rest_url(88-101)wss_url(104-147)hummingbot/core/web_assistant/rest_assistant.py (2)
execute_request(35-59)call(104-110)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
hummingbot/core/web_assistant/connections/data_types.py (6)
RESTMethod(14-25)RESTRequest(29-37)WSRequest(132-135)method(98-100)url(93-95)headers(108-110)
scripts/pmm_avellaneda_batch.py (7)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (4)
OrderType(8-15)PriceType(50-57)PositionAction(31-34)TradeType(60-63)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (3)
batch_order_create(875-1152)name(96-98)batch_order_cancel(1154-1401)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (8)
hummingbot/core/web_assistant/connections/data_types.py (5)
json(112-125)RESTMethod(14-25)RESTRequest(29-37)url(93-95)method(98-100)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (2)
OrderlyPerpetualAuth(39-317)account_id(274-276)hummingbot/connector/utils.py (1)
combine_to_hb_trading_pair(34-36)hummingbot/core/data_type/in_flight_order.py (4)
InFlightOrder(91-395)OrderState(25-36)OrderUpdate(39-45)TradeUpdate(48-88)hummingbot/core/data_type/trade_fee.py (3)
TokenAmount(19-35)TradeFeeBase(75-245)new_perpetual_fee(115-130)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (2)
build_api_factory(150-170)public_rest_url(72-85)hummingbot/core/web_assistant/rest_assistant.py (1)
execute_request(35-59)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_constants.py (1)
hummingbot/core/data_type/in_flight_order.py (1)
OrderState(25-36)
🪛 Flake8 (7.3.0)
scripts/pmm_avellaneda.py
[error] 194-194: f-string is missing placeholders
(F541)
[error] 206-206: f-string is missing placeholders
(F541)
[error] 249-249: local variable 'connector' is assigned to but never used
(F841)
[error] 386-386: continuation line over-indented for visual indent
(E127)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py
[error] 34-34: 'hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS' imported but unused
(F401)
[error] 36-36: 'hummingbot.core.web_assistant.connections.data_types.RESTMethod' imported but unused
(F401)
scripts/pmm_avellaneda_batch.py
[error] 295-295: continuation line over-indented for visual indent
(E127)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
[error] 18-18: 'json' imported but unused
(F401)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 45-45: 'hummingbot.core.web_assistant.connections.data_types.RESTRequest' imported but unused
(F401)
🪛 LanguageTool
CONNECTOR_ARCHITECTURE_EXPLANATION.md
[uncategorized] ~535-~535: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...terators for real-time updates --- ## Rate Limiting Strategy The throttler uses a **slidin...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda.py
194-194: f-string without any placeholders
Remove extraneous f prefix
(F541)
206-206: f-string without any placeholders
Remove extraneous f prefix
(F541)
249-249: Local variable connector is assigned to but never used
Remove assignment to unused variable connector
(F841)
372-372: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
377-377: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
378-378: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
384-384: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
388-388: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py
132-132: Avoid specifying long messages outside the exception class
(TRY003)
137-137: Avoid specifying long messages outside the exception class
(TRY003)
147-147: Avoid specifying long messages outside the exception class
(TRY003)
204-204: Unused function argument: throttler
(ARG001)
204-204: Unused function argument: domain
(ARG001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py
53-53: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
93-93: Unused method argument: domain
(ARG002)
151-151: Avoid specifying long messages outside the exception class
(TRY003)
196-196: Do not catch blind exception: Exception
(BLE001)
206-206: Do not catch blind exception: Exception
(BLE001)
236-236: Avoid specifying long messages outside the exception class
(TRY003)
340-340: Do not catch blind exception: Exception
(BLE001)
396-396: Avoid specifying long messages outside the exception class
(TRY003)
459-459: Consider moving this statement to an else block
(TRY300)
464-464: Use explicit conversion flag
Replace with conversion flag
(RUF010)
531-531: Use explicit conversion flag
Replace with conversion flag
(RUF010)
632-632: Do not catch blind exception: Exception
(BLE001)
681-681: Do not catch blind exception: Exception
(BLE001)
754-754: Do not catch blind exception: Exception
(BLE001)
807-807: Consider moving this statement to an else block
(TRY300)
809-809: Do not catch blind exception: Exception
(BLE001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py
81-81: Abstract raise to an inner function
(TRY301)
81-81: Avoid specifying long messages outside the exception class
(TRY003)
88-88: Abstract raise to an inner function
(TRY301)
88-88: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Do not catch blind exception: Exception
(BLE001)
93-93: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
93-93: Avoid specifying long messages outside the exception class
(TRY003)
scripts/pmm_avellaneda_batch.py
281-281: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
286-286: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
287-287: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
293-293: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
297-297: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
62-62: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
63-63: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
64-64: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
191-191: Unused method argument: trading_pair
(ARG002)
195-195: Unused method argument: trading_pair
(ARG002)
239-239: Do not catch blind exception: Exception
(BLE001)
273-273: Do not catch blind exception: Exception
(BLE001)
292-292: Abstract raise to an inner function
(TRY301)
292-292: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Consider moving this statement to an else block
(TRY300)
354-354: Avoid specifying long messages outside the exception class
(TRY003)
387-387: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Do not catch blind exception: Exception
(BLE001)
445-445: Unused method argument: request_exception
(ARG002)
510-510: Unused method argument: kwargs
(ARG002)
623-623: Consider moving this statement to an else block
(TRY300)
625-625: Do not catch blind exception: Exception
(BLE001)
675-675: Unused method argument: amount
(ARG002)
676-676: Unused method argument: trade_type
(ARG002)
677-677: Unused method argument: order_type
(ARG002)
678-678: Unused method argument: price
(ARG002)
680-680: Unused method argument: position_action
(ARG002)
715-715: Unused method argument: amount
(ARG002)
716-716: Unused method argument: trade_type
(ARG002)
717-717: Unused method argument: order_type
(ARG002)
718-718: Unused method argument: price
(ARG002)
720-720: Unused method argument: position_action
(ARG002)
763-763: Unused method argument: kwargs
(ARG002)
818-818: Avoid specifying long messages outside the exception class
(TRY003)
873-873: Avoid specifying long messages outside the exception class
(TRY003)
909-912: Avoid specifying long messages outside the exception class
(TRY003)
962-962: Do not catch blind exception: Exception
(BLE001)
1003-1003: Do not catch blind exception: Exception
(BLE001)
1047-1047: Abstract raise to an inner function
(TRY301)
1047-1047: Avoid specifying long messages outside the exception class
(TRY003)
1133-1133: Consider moving this statement to an else block
(TRY300)
1138-1138: Do not catch blind exception: Exception
(BLE001)
1152-1152: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1152-1152: Avoid specifying long messages outside the exception class
(TRY003)
1181-1184: Avoid specifying long messages outside the exception class
(TRY003)
1220-1220: Do not catch blind exception: Exception
(BLE001)
1315-1315: Abstract raise to an inner function
(TRY301)
1315-1315: Avoid specifying long messages outside the exception class
(TRY003)
1394-1394: Consider moving this statement to an else block
(TRY300)
1399-1399: Do not catch blind exception: Exception
(BLE001)
1401-1401: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1401-1401: Avoid specifying long messages outside the exception class
(TRY003)
1436-1436: Avoid specifying long messages outside the exception class
(TRY003)
1521-1521: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1521-1521: Avoid specifying long messages outside the exception class
(TRY003)
1522-1522: Do not catch blind exception: Exception
(BLE001)
1591-1591: Do not catch blind exception: Exception
(BLE001)
1632-1632: Do not catch blind exception: Exception
(BLE001)
1633-1633: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1646-1646: Unused method argument: trading_pair
(ARG002)
1736-1736: Consider moving this statement to an else block
(TRY300)
1738-1738: Do not catch blind exception: Exception
(BLE001)
1762-1762: Unused method argument: position_action
(ARG002)
1817-1817: Do not catch blind exception: Exception
(BLE001)
1844-1844: Unused method argument: event
(ARG002)
1849-1849: Unused method argument: event
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
| import asyncio | ||
| import json | ||
| from decimal import Decimal | ||
| from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple | ||
|
|
||
| from bidict import bidict | ||
|
|
||
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_constants as CONSTANTS | ||
| import hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_web_utils as web_utils | ||
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_api_order_book_data_source import ( | ||
| OrderlyPerpetualAPIOrderBookDataSource, | ||
| ) | ||
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_auth import OrderlyPerpetualAuth | ||
| from hummingbot.connector.derivative.orderly_perpetual.orderly_perpetual_user_stream_data_source import ( | ||
| OrderlyPerpetualUserStreamDataSource, | ||
| ) | ||
| from hummingbot.connector.derivative.position import Position | ||
| from hummingbot.connector.perpetual_derivative_py_base import PerpetualDerivativePyBase | ||
| from hummingbot.connector.trading_rule import TradingRule | ||
| from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id | ||
| from hummingbot.core.api_throttler.data_types import RateLimit | ||
| from hummingbot.core.data_type.common import OrderType, PositionAction, PositionMode, PositionSide, TradeType | ||
| from hummingbot.core.data_type.in_flight_order import InFlightOrder, OrderState, OrderUpdate, TradeUpdate | ||
| from hummingbot.core.data_type.order_book_tracker_data_source import OrderBookTrackerDataSource | ||
| from hummingbot.core.data_type.trade_fee import TokenAmount, TradeFeeBase | ||
| from hummingbot.core.data_type.user_stream_tracker_data_source import UserStreamTrackerDataSource | ||
| from hummingbot.core.utils.async_utils import safe_gather | ||
| from hummingbot.core.utils.estimate_fee import build_trade_fee | ||
| from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest |
There was a problem hiding this comment.
Remove unused imports to unblock lint
The module still imports json, TYPE_CHECKING, combine_to_hb_trading_pair, and RESTRequest, but none are referenced. Flake8 flags them with F401, so please drop the unused names.
-import json
-from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
+from typing import Any, Dict, List, Optional, Tuple
@@
-from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id
+from hummingbot.connector.utils import get_new_client_order_id
@@
-from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest
+from hummingbot.core.web_assistant.connections.data_types import RESTMethod🧰 Tools
🪛 Flake8 (7.3.0)
[error] 18-18: 'json' imported but unused
(F401)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 45-45: 'hummingbot.core.web_assistant.connections.data_types.RESTRequest' imported but unused
(F401)
🤖 Prompt for AI Agents
In
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
around lines 17 to 45, the imports json, TYPE_CHECKING,
combine_to_hb_trading_pair, and RESTRequest are unused and trigger F401; remove
these names from the import list (delete the json and TYPE_CHECKING imports and
remove combine_to_hb_trading_pair and RESTRequest from their respective import
statements) and run the linter to confirm no other references remain.
| for idx, bid_spread in enumerate(self.config.bid_spread_levels): | ||
| ask_spread = self.config.ask_spread_levels[idx] | ||
| bid_price = reservation_price * (Decimal("1") - bid_spread) | ||
| ask_price = reservation_price * (Decimal("1") + ask_spread) | ||
|
|
||
| # Convert quote amount to base amount for both buy and sell orders | ||
| # For perpetual orders, amount must be in base currency (BTC), not quote (USDC) | ||
| bid_amount = Decimal(self.config.order_amount_quote) / bid_price | ||
| ask_amount = Decimal(self.config.order_amount_quote) / ask_price | ||
|
|
||
| bid_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT_MAKER, | ||
| order_side=TradeType.BUY, | ||
| amount=bid_amount, | ||
| price=bid_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| ask_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT_MAKER, | ||
| order_side=TradeType.SELL, | ||
| amount=ask_amount, | ||
| price=ask_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| if self._current_inventory >= self.config.max_inventory: | ||
| orders.extend([bid_order]) | ||
| elif self._current_inventory <= -self.config.max_inventory: | ||
| orders.extend([ask_order]) | ||
| else: | ||
| orders.extend([bid_order, ask_order]) | ||
| return orders |
There was a problem hiding this comment.
Fix spread list pairing before iterating.
Accessing ask_spread_levels[idx] will throw IndexError if the two lists ever differ (e.g., config typo), taking the whole tick loop down. Validate lengths (or iterate with zip) so the strategy fails fast with a clear error message instead of at runtime.
- for idx, bid_spread in enumerate(self.config.bid_spread_levels):
- ask_spread = self.config.ask_spread_levels[idx]
+ if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels):
+ raise ValueError(
+ f"bid_spread_levels ({len(self.config.bid_spread_levels)}) and "
+ f"ask_spread_levels ({len(self.config.ask_spread_levels)}) must match for {self.config.trading_pair}."
+ )
+
+ for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch.py around lines 126 to 162, the code indexes
ask_spread_levels by idx assuming both spread lists are equal length which can
raise IndexError at runtime; update the loop to either validate that
len(self.config.bid_spread_levels) == len(self.config.ask_spread_levels) at
start and raise a clear ValueError if not, or iterate using
zip(self.config.bid_spread_levels, self.config.ask_spread_levels) (or
zip_longest with an explicit error) so the pairing is safe and the code fails
fast with a descriptive error rather than crashing mid-loop.
| if self._current_inventory >= self.config.max_inventory: | ||
| orders.extend([bid_order]) | ||
| elif self._current_inventory <= -self.config.max_inventory: | ||
| orders.extend([ask_order]) | ||
| else: | ||
| orders.extend([bid_order, ask_order]) | ||
| return orders |
There was a problem hiding this comment.
Invert the inventory cap logic to avoid runaway exposure.
When you’re already long past the cap (_current_inventory >= max_inventory), the code still queues a BUY order, which increases exposure instead of reducing it. Likewise, when short past the cap, it still queues a SELL. Swap the sides so we only place the order that nudges inventory back toward neutral.
- if self._current_inventory >= self.config.max_inventory:
- orders.extend([bid_order])
- elif self._current_inventory <= -self.config.max_inventory:
- orders.extend([ask_order])
+ if self._current_inventory >= self.config.max_inventory:
+ orders.append(ask_order)
+ elif self._current_inventory <= -self.config.max_inventory:
+ orders.append(bid_order)🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch.py around lines 156 to 162, the inventory-cap
logic is inverted: when _current_inventory >= config.max_inventory the code
currently appends a bid (buy) which increases exposure, and when
_current_inventory <= -config.max_inventory it appends an ask (sell) which
increases short exposure; swap the sides so that when inventory >= max_inventory
you append ask_order (sell) to reduce exposure, when inventory <= -max_inventory
you append bid_order (buy) to cover/neutralize, and otherwise append both
bid_order and ask_order as before.
| if self._current_inventory >= self.config.max_inventory: | ||
| orders.extend([bid_order]) | ||
| elif self._current_inventory <= -self.config.max_inventory: | ||
| orders.extend([ask_order]) | ||
| else: | ||
| orders.extend([bid_order, ask_order]) |
There was a problem hiding this comment.
Fix inventory cap handling to reduce, not grow, exposure.
Once we’re long above the max cap we still enqueue a BUY (and vice versa when short), which pushes inventory further out of bounds. Swap the order sides so the strategy only places the hedge that moves inventory back toward neutral.
- if self._current_inventory >= self.config.max_inventory:
- orders.extend([bid_order])
- elif self._current_inventory <= -self.config.max_inventory:
- orders.extend([ask_order])
+ if self._current_inventory >= self.config.max_inventory:
+ orders.append(ask_order)
+ elif self._current_inventory <= -self.config.max_inventory:
+ orders.append(bid_order)🤖 Prompt for AI Agents
In scripts/pmm_avellaneda.py around lines 174 to 179, the inventory cap logic
currently enqueues orders that increase exposure when inventory is already
beyond limits; change it so when self._current_inventory >=
self.config.max_inventory you only enqueue the ask/sell order to reduce long
exposure, and when self._current_inventory <= -self.config.max_inventory you
only enqueue the bid/buy order to reduce short exposure; keep both bid and ask
in the else branch for neutral inventory.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
scripts/pmm_avellaneda_batch_multi.py (3)
66-88: Add error handling for malformed pair configuration strings.The
validate_pairsmethod parses user input but lacks error handling for malformed or non-numeric values. Ifparts[1]throughparts[6]contain non-numeric strings, theDecimal()orint()constructors will raise exceptions.Wrap the parsing in a try-except block:
def validate_pairs(cls, v): if isinstance(v, str): pairs = [] for pair_str in v.split(":"): - parts = pair_str.split(",") - if len(parts) >= 1: - trading_pair = parts[0].strip() - order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal("20") - risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal("6.0") - volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal("0.50") - risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal("2.0") - max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal("0.01") - leverage = int(parts[6].strip()) if len(parts) > 6 else 10 - pairs.append(PairConfig( - trading_pair=trading_pair, - order_amount_quote=order_amount_quote, - risk_aversion_gamma=risk_aversion_gamma, - volatility_sigma=volatility_sigma, - risk_horizon_tau_hours=risk_horizon_tau_hours, - max_inventory=max_inventory, - leverage=leverage - )) + try: + parts = pair_str.split(",") + if len(parts) >= 1: + trading_pair = parts[0].strip() + order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal("20") + risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal("6.0") + volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal("0.50") + risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal("2.0") + max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal("0.01") + leverage = int(parts[6].strip()) if len(parts) > 6 else 10 + pairs.append(PairConfig( + trading_pair=trading_pair, + order_amount_quote=order_amount_quote, + risk_aversion_gamma=risk_aversion_gamma, + volatility_sigma=volatility_sigma, + risk_horizon_tau_hours=risk_horizon_tau_hours, + max_inventory=max_inventory, + leverage=leverage + )) + except (ValueError, TypeError, IndexError) as e: + raise ValueError(f"Invalid pair configuration '{pair_str}': {e}") return pairs if pairs else [PairConfig()] return v
181-181: Consider renaming unused loop variable.The loop variable
pair_configis not used in the loop body. You can rename it to_to indicate it's intentionally unused.- for trading_pair, pair_config in self._pair_configs.items(): + for trading_pair, _ in self._pair_configs.items():
489-489: Consider renaming unused loop variable.Similar to Line 181, the
pair_configvariable is not used in the loop body.- for trading_pair, pair_config in self._pair_configs.items(): + for trading_pair in self._pair_configs.keys():
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/pmm_avellaneda_batch_multi.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/pmm_avellaneda_batch_multi.py (6)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (5)
OrderType(8-15)PriceType(50-57)PositionAction(31-34)TradeType(60-63)get(121-124)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda_batch_multi.py
74-74: Decimal() called with float literal argument
Replace with string literal
(RUF032)
75-75: Decimal() called with float literal argument
Replace with string literal
(RUF032)
76-76: Decimal() called with float literal argument
Replace with string literal
(RUF032)
77-77: Decimal() called with float literal argument
Replace with string literal
(RUF032)
115-115: Avoid specifying long messages outside the exception class
(TRY003)
131-131: Avoid specifying long messages outside the exception class
(TRY003)
181-181: Loop control variable pair_config not used within loop body
Rename unused pair_config to _pair_config
(B007)
209-209: Do not catch blind exception: Exception
(BLE001)
345-345: Do not catch blind exception: Exception
(BLE001)
419-419: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
424-424: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
425-425: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
437-437: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
440-440: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
489-489: Loop control variable pair_config not used within loop body
(B007)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (5)
scripts/pmm_avellaneda_batch_multi.py (5)
135-137: Good use of asyncio lock for concurrency safety.The combination of
asyncio.Lockand the_order_operation_in_progressflag correctly prevents concurrent order operations that could lead to race conditions.
240-246: Verify spread level pairing logic.The code iterates over
bid_spread_levelsusing indexidxand accessesask_spread_levels[idx]on Line 241. This assumes both lists have the same length, which is enforced only if the validation suggested in an earlier comment is added.After adding the spread level length validator to
PairConfig, this logic will be safe. Otherwise, verify that all configurations maintain equal-length spread level lists.
352-403: Fill handling and inventory tracking logic is correct.The
did_fill_ordermethod correctly:
- Updates per-pair inventory (BUY increases, SELL decreases)
- Maintains a rolling window of the last 6 fills per pair
- Logs fill events with inventory updates
202-212: Broad exception handling is acceptable here.Catching
Exceptionat Line 209 is appropriate in this context to ensure the_order_operation_in_progressflag is always reset in the finally block, preventing the strategy from getting stuck. Similar reasoning applies to Line 345.
416-443: Reservation price calculation is correctly implemented.The Avellaneda-Stoikov formula
r = s - q·γ·σ²·τis accurately implemented:
- Correctly converts tau from hours to years (Line 431)
- Properly calculates the inventory adjustment term
- Handles Decimal arithmetic throughout
| class PairConfig(BaseClientModel): | ||
| """Configuration for a single trading pair""" | ||
| trading_pair: str = Field("BTC-USDC") | ||
| order_amount_quote: Decimal = Field(20) | ||
| risk_aversion_gamma: Decimal = Field(6.0) | ||
| volatility_sigma: Decimal = Field(0.50) | ||
| risk_horizon_tau_hours: Decimal = Field(2.0) | ||
| bid_spread_levels: List[Decimal] = Field(default=[Decimal("0.001")]) # 10 bps | ||
| ask_spread_levels: List[Decimal] = Field(default=[Decimal("0.001")]) # 10 bps | ||
| max_inventory: Decimal = Field(0.01) # 1k usd | ||
| leverage: int = Field(10) | ||
|
|
||
| @field_validator("order_amount_quote", "risk_aversion_gamma", "volatility_sigma", | ||
| "risk_horizon_tau_hours", "max_inventory", mode="before") | ||
| @classmethod | ||
| def convert_to_decimal(cls, v): | ||
| """Convert float/int values to Decimal when loading from YAML""" | ||
| if isinstance(v, (int, float)): | ||
| return Decimal(str(v)) | ||
| return v | ||
|
|
||
| @field_validator("bid_spread_levels", "ask_spread_levels", mode="before") | ||
| @classmethod | ||
| def convert_list_to_decimal(cls, v): | ||
| """Convert list of float/int values to Decimal""" | ||
| if isinstance(v, list): | ||
| return [Decimal(str(x)) if isinstance(x, (int, float)) else x for x in v] | ||
| return v |
There was a problem hiding this comment.
Add validation to ensure spread level lists have equal length.
The bid_spread_levels and ask_spread_levels lists are independently configurable but must have matching lengths for the order creation logic at Line 241 to work correctly. Currently, if these lists have different lengths, an IndexError will be raised when iterating.
Add this validator to PairConfig:
+ @field_validator("ask_spread_levels")
+ @classmethod
+ def validate_spread_levels_length(cls, v, info):
+ """Ensure ask_spread_levels and bid_spread_levels have the same length"""
+ if info.data.get("bid_spread_levels") and len(v) != len(info.data["bid_spread_levels"]):
+ raise ValueError(
+ f"bid_spread_levels and ask_spread_levels must have the same length. "
+ f"Got {len(info.data['bid_spread_levels'])} bid levels and {len(v)} ask levels."
+ )
+ return v🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch_multi.py around lines 22 to 49, add a validator
to PairConfig that asserts bid_spread_levels and ask_spread_levels have equal
length; implement this as a root_validator (mode="after") that reads both
attributes after conversion, compares len(bid_spread_levels) and
len(ask_spread_levels), and raises a ValueError with a clear message including
the trading_pair and both lengths if they differ; ensure the validator handles
None or non-list values by treating them as empty lists so the check is robust.
| order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal(20) | ||
| risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal(6.0) | ||
| volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal(0.50) | ||
| risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal(2.0) | ||
| max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal(0.01) |
There was a problem hiding this comment.
Replace float literals with string literals in Decimal constructors.
Calling Decimal() with float literals can introduce precision issues. The static analysis tool correctly identifies this pattern.
Apply this diff:
- order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal(20)
- risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal(6.0)
- volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal(0.50)
- risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal(2.0)
- max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal(0.01)
+ order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal("20")
+ risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal("6.0")
+ volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal("0.50")
+ risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal("2.0")
+ max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal("0.01")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal(20) | |
| risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal(6.0) | |
| volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal(0.50) | |
| risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal(2.0) | |
| max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal(0.01) | |
| order_amount_quote = Decimal(parts[1].strip()) if len(parts) > 1 else Decimal("20") | |
| risk_aversion_gamma = Decimal(parts[2].strip()) if len(parts) > 2 else Decimal("6.0") | |
| volatility_sigma = Decimal(parts[3].strip()) if len(parts) > 3 else Decimal("0.50") | |
| risk_horizon_tau_hours = Decimal(parts[4].strip()) if len(parts) > 4 else Decimal("2.0") | |
| max_inventory = Decimal(parts[5].strip()) if len(parts) > 5 else Decimal("0.01") |
🧰 Tools
🪛 Ruff (0.14.4)
74-74: Decimal() called with float literal argument
Replace with string literal
(RUF032)
75-75: Decimal() called with float literal argument
Replace with string literal
(RUF032)
76-76: Decimal() called with float literal argument
Replace with string literal
(RUF032)
77-77: Decimal() called with float literal argument
Replace with string literal
(RUF032)
🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch_multi.py around lines 73 to 77, the Decimal
constructors are called with float literals which can introduce precision
issues; change each Decimal call that uses a float (e.g., Decimal(6.0),
Decimal(0.50), Decimal(2.0), Decimal(0.01)) to use string literals instead
(e.g., Decimal("6.0"), Decimal("0.50"), Decimal("2.0"), Decimal("0.01")) and
also replace the default Decimal(20) with Decimal("20") so all defaults use
string literals to preserve precision.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (1)
470-470: Guard authenticator access to prevent AttributeError.
self._connector.authenticatorcan beNonewhen the connector is instantiated without full API credentials (e.g.,trading_required=Falsefor data-only usage). Dereferencingaccount_iddirectly will raiseAttributeErrorand prevent the public WebSocket from connecting.This issue was previously flagged and remains unresolved.
Apply this fix to safely access
account_id:- ws_url = web_utils.wss_url("public", self._domain, self._connector.authenticator.account_id) # account_id is a mandatory parameter for public WebSocket URL + # Safely retrieve account_id with fallback + authenticator = getattr(self._connector, 'authenticator', None) + if authenticator: + account_id = authenticator.account_id + else: + # Fallback to connector's configured account_id if available + account_id = getattr(self._connector, '_orderly_perpetual_account_id', None) + + if not account_id: + raise ValueError( + "Orderly public WebSocket requires account_id, but authenticator is not configured. " + "Ensure API credentials are provided." + ) + + ws_url = web_utils.wss_url("public", self._domain, account_id) self.logger().info(f"[WEBSOCKET] Attempting to connect to public WebSocket: {ws_url}")
🧹 Nitpick comments (1)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (1)
157-163: Consider logging when mark/index prices are missing.The code silently falls back to
Decimal("0")forindex_priceandmark_pricewhen the market info request fails or data is missing. This could mask API issues or unexpected response formats. Consider logging a warning when prices default to zero.index_price = Decimal("0") mark_price = Decimal("0") if market_response.get("success", False): market_data = market_response.get("data", {}) index_price = Decimal(str(market_data.get("index_price", 0))) mark_price = Decimal(str(market_data.get("mark_price", 0))) + else: + self.logger().warning( + f"Failed to fetch mark/index prices for {trading_pair}, using zero: {market_response}" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py(1 hunks)hummingbot/connector/exchange_base.pyx(2 hunks)hummingbot/connector/perpetual_derivative_py_base.py(6 hunks)hummingbot/core/data_type/common.py(1 hunks)hummingbot/core/data_type/perpetual_api_order_book_data_source.py(1 hunks)scripts/pmm_avellaneda_batch_markprice.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
hummingbot/connector/perpetual_derivative_py_base.py (3)
hummingbot/core/data_type/common.py (1)
PriceType(50-58)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (2)
get_funding_info(108-173)listen_for_mark_price(213-234)hummingbot/core/data_type/perpetual_api_order_book_data_source.py (1)
get_funding_info(15-19)
scripts/pmm_avellaneda_batch_markprice.py (7)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (4)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)TradeType(61-64)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/perpetual_derivative_py_base.py (3)
set_leverage(119-120)get_price_by_type(304-314)budget_checker(63-65)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (8)
hummingbot/core/data_type/common.py (2)
TradeType(61-64)get(122-125)hummingbot/core/data_type/perpetual_api_order_book_data_source.py (4)
PerpetualAPIOrderBookDataSource(9-49)get_funding_info(15-19)listen_for_funding_info(21-33)_parse_funding_info_message(36-37)hummingbot/core/web_assistant/connections/data_types.py (6)
RESTMethod(14-25)WSJSONRequest(139-145)url(93-95)method(98-100)RESTRequest(29-37)json(112-125)hummingbot/core/web_assistant/web_assistants_factory.py (2)
WebAssistantsFactory(14-81)get_ws_assistant(61-66)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_user_stream_data_source.py (2)
logger(74-78)_connected_websocket_assistant(90-137)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (5)
trading_pairs(154-156)domain(124-126)exchange_symbol_associated_to_pair(278-304)authenticator(101-116)_make_network_check_request(434-443)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (3)
public_rest_url(72-85)private_rest_url(88-101)wss_url(104-147)hummingbot/core/web_assistant/rest_assistant.py (2)
execute_request(35-59)call(104-110)
🪛 Flake8 (7.3.0)
scripts/pmm_avellaneda_batch_markprice.py
[error] 302-302: continuation line over-indented for visual indent
(E127)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda_batch_markprice.py
227-227: Do not catch blind exception: Exception
(BLE001)
288-288: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
293-293: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
294-294: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
300-300: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
304-304: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py
53-53: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
94-94: Unused method argument: domain
(ARG002)
152-152: Avoid specifying long messages outside the exception class
(TRY003)
197-197: Do not catch blind exception: Exception
(BLE001)
207-207: Do not catch blind exception: Exception
(BLE001)
231-231: Do not catch blind exception: Exception
(BLE001)
260-260: Avoid specifying long messages outside the exception class
(TRY003)
364-364: Do not catch blind exception: Exception
(BLE001)
420-420: Avoid specifying long messages outside the exception class
(TRY003)
483-483: Consider moving this statement to an else block
(TRY300)
488-488: Use explicit conversion flag
Replace with conversion flag
(RUF010)
569-569: Use explicit conversion flag
Replace with conversion flag
(RUF010)
672-672: Do not catch blind exception: Exception
(BLE001)
721-721: Do not catch blind exception: Exception
(BLE001)
794-794: Do not catch blind exception: Exception
(BLE001)
881-881: Do not catch blind exception: Exception
(BLE001)
917-917: Consider moving this statement to an else block
(TRY300)
919-919: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (17)
hummingbot/connector/exchange_base.pyx (1)
338-356: LGTM! Clean extension for MarkPrice support.The addition of
PriceType.MarkPricehandling is well-implemented. The base class appropriately raisesNotImplementedErrorwith a clear message, and the docstring accurately reflects the new functionality. The perpetual derivative subclass will override this to provide actual mark price data.hummingbot/core/data_type/common.py (1)
58-58: LGTM! Proper enum extension.The addition of
MarkPrice = 8follows the established pattern and correctly extends thePriceTypeenum for perpetual derivative pricing scenarios.hummingbot/core/data_type/perpetual_api_order_book_data_source.py (1)
39-49: LGTM! Backward-compatible mark price queue support.The conditional addition of the mark price queue key is well-implemented. Using
hasattrensures backward compatibility with connectors that don't support mark price streams, while enabling those that do to opt-in seamlessly.hummingbot/connector/perpetual_derivative_py_base.py (5)
36-36: LGTM! Proper task initialization.The
_mark_price_listener_taskattribute is correctly initialized toNoneand will be managed in the start/stop network lifecycle.
99-101: LGTM! Conditional mark price listener startup.The conditional check using
hasattrensures the mark price listener only starts when the data source supports it, maintaining backward compatibility with connectors that don't provide mark price streams.
214-216: LGTM! Proper task cleanup.The mark price listener task is correctly canceled and reset to
Noneduring network shutdown, preventing resource leaks.
304-314: LGTM! Correct MarkPrice implementation for perpetual derivatives.The
get_price_by_typeoverride correctly:
- Returns
mark_pricefrom funding info whenPriceType.MarkPriceis requested- Delegates to the parent implementation for all other price types
This provides the specialized behavior needed for perpetual derivatives while maintaining compatibility with the base class interface.
387-394: LGTM! Clean mark price listener implementation.The
_listen_for_mark_pricemethod correctly delegates to the data source'slisten_for_mark_priceand feeds updates into the existingfunding_info_stream, maintaining consistency with the funding info flow.hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_api_order_book_data_source.py (9)
53-54: Verify class-level mutable attributes are intentional.Class-level mutable attributes (
_trading_pair_symbol_mapand_mapping_initialization_lock) will be shared across all instances of this class. If cross-instance sharing is not intended, move these to__init__. If sharing is intentional, annotate withtyping.ClassVarto make the intent explicit.from typing import ClassVar class OrderlyPerpetualAPIOrderBookDataSource(PerpetualAPIOrderBookDataSource): _logger: Optional[HummingbotLogger] = None _trading_pair_symbol_map: ClassVar[Dict[str, Dict[str, str]]] = {} _mapping_initialization_lock: ClassVar[asyncio.Lock] = asyncio.Lock()Or move to instance variables if sharing is not intended:
- _trading_pair_symbol_map: Dict[str, Dict[str, str]] = {} - _mapping_initialization_lock = asyncio.Lock() - def __init__( self, trading_pairs: List[str], connector: 'OrderlyPerpetualDerivative', api_factory: WebAssistantsFactory, domain: str = CONSTANTS.DOMAIN ): super().__init__(trading_pairs) self._connector = connector self._api_factory = api_factory self._domain = domain self._trading_pairs: List[str] = trading_pairs self._message_queue: Dict[str, asyncio.Queue] = defaultdict(asyncio.Queue) self._snapshot_messages_queue_key = "order_book_snapshot" self._mark_price_messages_queue_key = "mark_price" + self._trading_pair_symbol_map: Dict[str, Dict[str, str]] = {} + self._mapping_initialization_lock = asyncio.Lock()
175-211: LGTM! Robust funding info polling implementation.The polling-based approach is appropriate given Orderly's lack of WebSocket funding rate updates. The per-pair error handling ensures one symbol's failure doesn't block others, and the sleep interval is properly configurable.
372-422: LGTM! Correctly implements authenticated orderbook snapshot.The implementation properly recognizes that Orderly's orderbook snapshot endpoint requires authentication (unusual for most exchanges). The use of
private_rest_url,is_auth_required=True, andrest_assistant.call()ensures requests are properly signed.
424-461: LGTM! Clean orderbook snapshot transformation.The method correctly:
- Transforms REST API orderbook format (dict with "price" and "quantity" keys) to array format
- Handles timestamp conversion from milliseconds to seconds
- Creates proper
OrderBookMessagewith SNAPSHOT type
493-572: LGTM! Well-structured WebSocket subscription logic.The subscription implementation is solid:
- Correctly formats Orderly WebSocket subscription messages (
{symbol}@{channel})- Subscribes to orderbook, trades, and mark price channels
- Extensive debug logging aids troubleshooting
- Proper error handling with CancelledError propagation
574-615: LGTM! Correct message routing logic.The channel identification correctly:
- Filters out subscription confirmations and errors
- Parses Orderly's
{symbol}@{channel}topic format- Maps channels to appropriate message queues
617-676: LGTM! Correct orderbook diff parsing.The diff message parser properly:
- Extracts symbol from topic with fallback to data
- Converts timestamps from milliseconds to seconds
- Creates
OrderBookMessagewith DIFF type- Handles errors gracefully with logging
817-885: LGTM! Robust mark price message parser.The mark price parser correctly:
- Handles both dict and list data formats from Orderly
- Validates mark price is non-zero before processing
- Creates
FundingInfoUpdatewith only the updated mark price (and optionally index price)- Other funding info fields are preserved from existing state
This integrates well with the separate mark price WebSocket stream introduced in this PR.
887-921: LGTM! Proper network health check.The network check correctly:
- Uses the system info endpoint to verify connectivity
- Interprets status codes properly (0 = operational, 2 = maintenance)
- Returns
Falseon failures with appropriate logging
| for idx, bid_spread in enumerate(self.config.bid_spread_levels): | ||
| ask_spread = self.config.ask_spread_levels[idx] | ||
| bid_price = reservation_price * (Decimal("1") - bid_spread) | ||
| ask_price = reservation_price * (Decimal("1") + ask_spread) | ||
|
|
||
| # Convert quote amount to base amount for both buy and sell orders | ||
| # For perpetual orders, amount must be in base currency (BTC), not quote (USDC) | ||
| bid_amount = Decimal(self.config.order_amount_quote) / bid_price | ||
| ask_amount = Decimal(self.config.order_amount_quote) / ask_price | ||
|
|
||
| bid_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT_MAKER, | ||
| order_side=TradeType.BUY, | ||
| amount=bid_amount, | ||
| price=bid_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| ask_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT_MAKER, | ||
| order_side=TradeType.SELL, | ||
| amount=ask_amount, | ||
| price=ask_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| if self._current_inventory >= self.config.max_inventory: | ||
| orders.extend([bid_order]) | ||
| elif self._current_inventory <= -self.config.max_inventory: | ||
| orders.extend([ask_order]) | ||
| else: | ||
| orders.extend([bid_order, ask_order]) | ||
| return orders |
There was a problem hiding this comment.
Guard mismatched spread level lengths
This loop assumes bid_spread_levels and ask_spread_levels are the same length; if a user misconfigures them, we’ll crash with an IndexError. Add validation (or iterate with zip) so the strategy fails fast with a clear message instead of blowing up mid-run.
- for idx, bid_spread in enumerate(self.config.bid_spread_levels):
- ask_spread = self.config.ask_spread_levels[idx]
+ if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels):
+ raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.")
+
+ for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch_markprice.py around lines 127 to 163 the loop
indexes ask_spread_levels by position assuming both bid_spread_levels and
ask_spread_levels have the same length which can raise IndexError if
misconfigured; fix by validating the two lists' lengths before the loop and
raise a clear ValueError (or log an explicit error and stop) if they differ, or
change the loop to iterate using zip(self.config.bid_spread_levels,
self.config.ask_spread_levels) so only matching pairs are processed and
mismatched entries are ignored; include a clear message indicating which config
key is inconsistent so the strategy fails fast with actionable feedback.
| def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: | ||
| connector = self.connectors[self.config.exchange] | ||
| budget_checker = connector.budget_checker | ||
|
|
||
| proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) | ||
| return proposals_adjusted |
There was a problem hiding this comment.
Fix budget checker invocation
connector.budget_checker is a method that returns the checker instance (see hummingbot/connector/perpetual_derivative_py_base.py). Storing it without calling leaves you with a bound method, so budget_checker.adjust_candidates(...) will raise AttributeError at runtime. Call the method before using it.
- budget_checker = connector.budget_checker
-
- proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True)
+ budget_checker = connector.budget_checker()
+
+ proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: | |
| connector = self.connectors[self.config.exchange] | |
| budget_checker = connector.budget_checker | |
| proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) | |
| return proposals_adjusted | |
| def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: | |
| connector = self.connectors[self.config.exchange] | |
| budget_checker = connector.budget_checker() | |
| proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) | |
| return proposals_adjusted |
🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch_markprice.py around lines 165 to 170, the code
stores connector.budget_checker (a method) instead of calling it, so
budget_checker is a bound method and budget_checker.adjust_candidates(...) will
raise AttributeError; change the assignment to call the method (budget_checker =
connector.budget_checker()) to get the checker instance before calling
adjust_candidates, and then use that instance to invoke adjust_candidates(...).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
scripts/pmm_avellaneda_batch_markprice.py (2)
126-163: Guard mismatched spread level lengths.This loop assumes
bid_spread_levelsandask_spread_levelshave the same length; if they differ, line 127 will raiseIndexError.Apply this diff to validate lengths or use
zip:+ if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + - for idx, bid_spread in enumerate(self.config.bid_spread_levels): - ask_spread = self.config.ask_spread_levels[idx] + for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):
165-170: Fix budget checker invocation.
connector.budget_checkeris a method that returns the checker instance. Line 167 stores the bound method itself, so line 169 will raiseAttributeErrorwhen callingadjust_candidates.Apply this diff:
- budget_checker = connector.budget_checker + budget_checker = connector.budget_checker() proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/pmm_avellaneda_batch_markprice.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/pmm_avellaneda_batch_markprice.py (6)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (5)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/perpetual_derivative_py_base.py (3)
set_leverage(119-120)get_price_by_type(304-314)budget_checker(63-65)
🪛 Flake8 (7.3.0)
scripts/pmm_avellaneda_batch_markprice.py
[error] 310-310: continuation line over-indented for visual indent
(E127)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda_batch_markprice.py
227-227: Do not catch blind exception: Exception
(BLE001)
296-296: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
301-301: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
302-302: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
308-308: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
312-312: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
scripts/pmm_avellaneda_batch_markprice.py (1)
280-280: The review comment is incorrect. The pattern of accessing_perpetual_trading.get_position()is the established standard throughout the Hummingbot codebase, used consistently across all perpetual derivative connectors (Binance, Kucoin, Injective, Gate.io, Derive, Bitmart, Orderly, and dYdX perpetual implementations). This is not a brittle coupling but rather a fundamental architectural pattern for perpetual trading connectors. No alternative public API exists for this functionality.Likely an incorrect or invalid review comment.
| def apply_initial_setting(self): | ||
| if not self.account_config_set: | ||
| connector = self.connectors[self.config.exchange] | ||
| connector.set_leverage(self.config.trading_pair, self.config.leverage) | ||
| self.account_config_set = True |
There was a problem hiding this comment.
apply_initial_setting is never invoked.
This method sets leverage but is never called, so the configured leverage won't be applied to the trading pair. Add a call in __init__ (after super().__init__) or at the start of on_tick (before strategy logic).
Apply this diff to call it during initialization:
def __init__(self, connectors: Dict[str, ConnectorBase], config: PMMAvellanedaMultiConfig):
super().__init__(connectors)
self.config = config
+ self.apply_initial_setting()
self._last_update_timestamp: float = 0Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_batch_markprice.py around lines 107 to 111, the
apply_initial_setting method (which sets leverage) is never invoked so leverage
is never applied; call self.apply_initial_setting() in the class constructor
right after super().__init__ (ensuring connectors and self.config are
initialized at that point) so the leverage is set at startup; alternatively call
it at the start of on_tick before any strategy logic; ensure the call is guarded
so it runs only once (e.g., after super init or by checking account_config_set)
and handle/log any connector exceptions.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
scripts/pmm_avellaneda_batch_markprice.py (3)
125-163: Validate spread arrays before iterating.
Line 127 still assumes the ask list matches the bid list; a length mismatch triggers anIndexErrorand halts the strategy loop. Please add the length check (or iterate withzip) so we fail with a clear configuration error instead of crashing mid-run.- for idx, bid_spread in enumerate(self.config.bid_spread_levels): - ask_spread = self.config.ask_spread_levels[idx] + if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + + for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):
165-170: Callbudget_checker()before using it.
Line 167 stores the methodconnector.budget_checkerinstead of the checker instance, sobudget_checker.adjust_candidates(...)still raisesAttributeError. Please call the method first.- budget_checker = connector.budget_checker + budget_checker = connector.budget_checker()
68-112: Invoke leverage setup on initialization.
apply_initial_settingremains unused, so the configured leverage never reaches the exchange and we run at default leverage. Call it inside__init__to honor the configuration immediately.super().__init__(connectors) self.config = config self._last_update_timestamp: float = 0 self._cached_mid_price: Decimal = Decimal("0") self._cached_reservation_price: Decimal = Decimal("0") + self.apply_initial_setting()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/pmm_avellaneda_batch_markprice.py(1 hunks)scripts/pmm_avellaneda_kodiak.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/pmm_avellaneda_batch_markprice.py (6)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (5)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/perpetual_derivative_py_base.py (3)
set_leverage(119-120)get_price_by_type(304-314)budget_checker(63-65)
scripts/pmm_avellaneda_kodiak.py (6)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (5)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)
🪛 Flake8 (7.3.0)
scripts/pmm_avellaneda_batch_markprice.py
[error] 311-311: continuation line over-indented for visual indent
(E127)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda_batch_markprice.py
227-227: Do not catch blind exception: Exception
(BLE001)
296-296: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
301-301: Docstring contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF002)
302-302: Docstring contains ambiguous σ (GREEK SMALL LETTER SIGMA). Did you mean o (LATIN SMALL LETTER O)?
(RUF002)
309-309: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
313-313: Comment contains ambiguous γ (GREEK SMALL LETTER GAMMA). Did you mean y (LATIN SMALL LETTER Y)?
(RUF003)
scripts/pmm_avellaneda_kodiak.py
237-237: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
| def __init__(self, connectors: Dict[str, ConnectorBase], config: PMMAvellanedaMultiConfig): | ||
| super().__init__(connectors) | ||
| self.config = config | ||
| self._last_update_timestamp: float = 0 | ||
| self._cached_mark_price: Decimal = Decimal("0") | ||
| self._cached_reservation_price: Decimal = Decimal("0") | ||
|
|
||
| # DataFrame to store last 6 filled orders | ||
| self._filled_orders_df: pd.DataFrame = pd.DataFrame(columns=[ | ||
| "Timestamp", "Order ID", "Side", "Amount", "Price", "Inventory" | ||
| ]) | ||
|
|
||
| def _get_in_flight_order(self, order_id: str) -> Optional[InFlightOrder]: | ||
| """Get InFlightOrder from connector's order tracker""" | ||
| connector = self.connectors[self.config.exchange] | ||
| return connector._order_tracker.fetch_order(client_order_id=order_id) | ||
|
|
||
|
|
||
| # Built-in event handler methods (called automatically by ScriptStrategyBase) | ||
|
|
||
| def on_tick(self): | ||
| if self.create_timestamp <= self.current_timestamp: | ||
| proposals: List[PerpetualOrderCandidate] = self.create_proposal() | ||
| proposal_adjusted: List[PerpetualOrderCandidate] = self.adjust_proposal_to_budget(proposals) | ||
| # Execute cancel then place sequentially to avoid order accumulation | ||
| safe_ensure_future(self._cancel_and_place_orders(proposal_adjusted)) | ||
| self.create_timestamp = self.config.order_refresh_time + self.current_timestamp | ||
|
|
||
| async def _cancel_and_place_orders(self, proposal: List[PerpetualOrderCandidate]) -> None: | ||
| """ | ||
| Cancel all active orders and then place new orders sequentially. | ||
| This ensures old orders are cancelled before new ones are placed. | ||
| """ | ||
| # First, cancel all active orders and wait for completion | ||
| await self._async_cancel_all_orders() | ||
| # Then place new orders | ||
| await self._async_place_orders(proposal) | ||
|
|
||
|
|
||
| def apply_initial_setting(self): | ||
| if not self.account_config_set: | ||
| connector = self.connectors[self.config.exchange] | ||
| connector.set_leverage(self.config.trading_pair, self.config.leverage) | ||
| self.account_config_set = True | ||
|
|
||
| def create_proposal(self) -> List[PerpetualOrderCandidate]: | ||
| connector = self.connectors[self.config.exchange] |
There was a problem hiding this comment.
Invoke leverage setup on initialization.
apply_initial_setting never runs, so Line 109’s leverage configuration is skipped and the connector trades at its default leverage, contradicting the script config. Call it during __init__ so the requested leverage is applied exactly once when the strategy boots.
super().__init__(connectors)
self.config = config
self._last_update_timestamp: float = 0
self._cached_mark_price: Decimal = Decimal("0")
self._cached_reservation_price: Decimal = Decimal("0")
+ self.apply_initial_setting()🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_kodiak.py around lines 66 to 112, the
apply_initial_setting() method is never invoked so leverage is not configured;
call self.apply_initial_setting() at the end of the __init__ after initializing
self.config, flags, and any account_config_set attribute so the connector's
leverage is set exactly once at startup; ensure apply_initial_setting remains
idempotent (checks account_config_set) so repeated __init__ calls won't reapply
settings.
| for idx, bid_spread in enumerate(self.config.bid_spread_levels): | ||
| ask_spread = self.config.ask_spread_levels[idx] | ||
| bid_price = reservation_price * (Decimal("1") - bid_spread) | ||
| ask_price = reservation_price * (Decimal("1") + ask_spread) | ||
|
|
||
| # To make sure the limit maker orders are not immediately taken | ||
| if bid_price >= best_ask_price or ask_price <= best_bid_price: | ||
| bid_price = mid_price * (Decimal("1") - bid_spread) | ||
| ask_price = mid_price * (Decimal("1") + ask_spread) | ||
|
|
||
| # Convert quote amount to base amount for both buy and sell orders | ||
| # For perpetual orders, amount must be in base currency (BTC), not quote (USDC) | ||
| bid_amount = Decimal(self.config.order_amount_quote) / bid_price | ||
| ask_amount = Decimal(self.config.order_amount_quote) / ask_price | ||
|
|
||
| bid_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT_MAKER, | ||
| order_side=TradeType.BUY, | ||
| amount=bid_amount, | ||
| price=bid_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| ask_order = PerpetualOrderCandidate( | ||
| trading_pair=self.config.trading_pair, | ||
| is_maker=True, | ||
| order_type=OrderType.LIMIT_MAKER, | ||
| order_side=TradeType.SELL, | ||
| amount=ask_amount, | ||
| price=ask_price, | ||
| leverage=Decimal(self.config.leverage) | ||
| ) | ||
|
|
||
| # Inventory-based order placement logic: | ||
| if inventory >= self.config.max_inventory: | ||
| # At max long position, only place ask orders to reduce position | ||
| orders.extend([ask_order]) | ||
| elif inventory <= -self.config.max_inventory: | ||
| # At max short position, only place bid orders to reduce position | ||
| orders.extend([bid_order]) | ||
| else: | ||
| # Normal market making: place both sides | ||
| orders.extend([bid_order, ask_order]) | ||
| return orders |
There was a problem hiding this comment.
Validate spread arrays before iterating.
Line 129 assumes ask_spread_levels[idx] exists; if a user provides mismatched lengths, Line 130 will raise an IndexError and the strategy stops mid-run. Guard the configuration (or iterate with zip) so we fail fast with a clear error instead of crashing during order generation.
- for idx, bid_spread in enumerate(self.config.bid_spread_levels):
- ask_spread = self.config.ask_spread_levels[idx]
+ if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels):
+ raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.")
+
+ for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for idx, bid_spread in enumerate(self.config.bid_spread_levels): | |
| ask_spread = self.config.ask_spread_levels[idx] | |
| bid_price = reservation_price * (Decimal("1") - bid_spread) | |
| ask_price = reservation_price * (Decimal("1") + ask_spread) | |
| # To make sure the limit maker orders are not immediately taken | |
| if bid_price >= best_ask_price or ask_price <= best_bid_price: | |
| bid_price = mid_price * (Decimal("1") - bid_spread) | |
| ask_price = mid_price * (Decimal("1") + ask_spread) | |
| # Convert quote amount to base amount for both buy and sell orders | |
| # For perpetual orders, amount must be in base currency (BTC), not quote (USDC) | |
| bid_amount = Decimal(self.config.order_amount_quote) / bid_price | |
| ask_amount = Decimal(self.config.order_amount_quote) / ask_price | |
| bid_order = PerpetualOrderCandidate( | |
| trading_pair=self.config.trading_pair, | |
| is_maker=True, | |
| order_type=OrderType.LIMIT_MAKER, | |
| order_side=TradeType.BUY, | |
| amount=bid_amount, | |
| price=bid_price, | |
| leverage=Decimal(self.config.leverage) | |
| ) | |
| ask_order = PerpetualOrderCandidate( | |
| trading_pair=self.config.trading_pair, | |
| is_maker=True, | |
| order_type=OrderType.LIMIT_MAKER, | |
| order_side=TradeType.SELL, | |
| amount=ask_amount, | |
| price=ask_price, | |
| leverage=Decimal(self.config.leverage) | |
| ) | |
| # Inventory-based order placement logic: | |
| if inventory >= self.config.max_inventory: | |
| # At max long position, only place ask orders to reduce position | |
| orders.extend([ask_order]) | |
| elif inventory <= -self.config.max_inventory: | |
| # At max short position, only place bid orders to reduce position | |
| orders.extend([bid_order]) | |
| else: | |
| # Normal market making: place both sides | |
| orders.extend([bid_order, ask_order]) | |
| return orders | |
| if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): | |
| raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") | |
| for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels): | |
| bid_price = reservation_price * (Decimal("1") - bid_spread) | |
| ask_price = reservation_price * (Decimal("1") + ask_spread) | |
| # To make sure the limit maker orders are not immediately taken | |
| if bid_price >= best_ask_price or ask_price <= best_bid_price: | |
| bid_price = mid_price * (Decimal("1") - bid_spread) | |
| ask_price = mid_price * (Decimal("1") + ask_spread) | |
| # Convert quote amount to base amount for both buy and sell orders | |
| # For perpetual orders, amount must be in base currency (BTC), not quote (USDC) | |
| bid_amount = Decimal(self.config.order_amount_quote) / bid_price | |
| ask_amount = Decimal(self.config.order_amount_quote) / ask_price | |
| bid_order = PerpetualOrderCandidate( | |
| trading_pair=self.config.trading_pair, | |
| is_maker=True, | |
| order_type=OrderType.LIMIT_MAKER, | |
| order_side=TradeType.BUY, | |
| amount=bid_amount, | |
| price=bid_price, | |
| leverage=Decimal(self.config.leverage) | |
| ) | |
| ask_order = PerpetualOrderCandidate( | |
| trading_pair=self.config.trading_pair, | |
| is_maker=True, | |
| order_type=OrderType.LIMIT_MAKER, | |
| order_side=TradeType.SELL, | |
| amount=ask_amount, | |
| price=ask_price, | |
| leverage=Decimal(self.config.leverage) | |
| ) | |
| # Inventory-based order placement logic: | |
| if inventory >= self.config.max_inventory: | |
| # At max long position, only place ask orders to reduce position | |
| orders.extend([ask_order]) | |
| elif inventory <= -self.config.max_inventory: | |
| # At max short position, only place bid orders to reduce position | |
| orders.extend([bid_order]) | |
| else: | |
| # Normal market making: place both sides | |
| orders.extend([bid_order, ask_order]) | |
| return orders |
🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_kodiak.py around lines 128 to 173, the code indexes
ask_spread_levels by idx assuming both spread arrays are same length which can
raise an IndexError at runtime; fix by validating that
len(self.config.bid_spread_levels) == len(self.config.ask_spread_levels) before
the loop and raise a clear ValueError if they differ, or change the loop to
iterate with zip/enumerate over both lists (e.g., for idx, (bid_spread,
ask_spread) in enumerate(zip(...)):), ensuring you fail fast with a descriptive
error message if lengths mismatch.
| def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: | ||
| connector = self.connectors[self.config.exchange] | ||
| budget_checker = connector.budget_checker | ||
|
|
||
| proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) | ||
| return proposals_adjusted |
There was a problem hiding this comment.
Call budget_checker() before using it.
Line 179 stores the bound method connector.budget_checker instead of the checker instance, so budget_checker.adjust_candidates(...) raises AttributeError at runtime. Invoke the method to obtain the checker before calling adjust_candidates.
- budget_checker = connector.budget_checker
+ budget_checker = connector.budget_checker()🤖 Prompt for AI Agents
In scripts/pmm_avellaneda_kodiak.py around lines 175 to 180, the code assigns
the bound method connector.budget_checker to budget_checker instead of calling
it, so budget_checker.adjust_candidates(...) will raise AttributeError; fix by
invoking connector.budget_checker() to get the checker instance (e.g.,
budget_checker = connector.budget_checker()) and then call
budget_checker.adjust_candidates(proposals, all_or_none=True), ensuring any
required args for budget_checker() are provided and handling a possible None
return.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
scripts/pmm_avellaneda_kodiak.py (3)
66-77: Invoke leverage setup on initialization.
apply_initial_settingis never called, so Line 109's leverage configuration is skipped and the connector trades at its default leverage, contradicting the script config. Without this, the strategy will not respect the configured leverage of 100.Apply this diff to call the method during initialization:
super().__init__(connectors) self.config = config self._last_update_timestamp: float = 0 self._cached_mark_price: Decimal = Decimal("0") self._cached_reservation_price: Decimal = Decimal("0") # DataFrame to store last 6 filled orders self._filled_orders_df: pd.DataFrame = pd.DataFrame(columns=[ "Timestamp", "Order ID", "Side", "Amount", "Price", "Inventory" ]) + + self.apply_initial_setting()
128-173: Validate spread arrays before iterating.Line 129 assumes
ask_spread_levels[idx]exists; if a user provides mismatched lengths, Line 130 will raise anIndexErrorand the strategy stops mid-run. The strategy should fail fast with a clear error instead of crashing during order generation.Apply this diff to validate array lengths:
orders = [] + if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + for idx, bid_spread in enumerate(self.config.bid_spread_levels): ask_spread = self.config.ask_spread_levels[idx]
175-180: Callbudget_checker()before using it.Line 177 stores the bound method
connector.budget_checkerinstead of the checker instance, so Line 179'sbudget_checker.adjust_candidates(...)raisesAttributeErrorat runtime.Apply this diff to invoke the method:
def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: connector = self.connectors[self.config.exchange] - budget_checker = connector.budget_checker + budget_checker = connector.budget_checker() proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) return proposals_adjustedhummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (2)
17-46: Remove unused imports to unblock lint.The following imports are unused and trigger F401 errors:
json,TYPE_CHECKING,combine_to_hb_trading_pair, andRESTRequest.Apply this diff:
-import json from decimal import Decimal -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple from bidict import bidict @@ -from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id +from hummingbot.connector.utils import get_new_client_order_id @@ -from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest +from hummingbot.core.web_assistant.connections.data_types import RESTMethod
1190-1274: Validate that all orders belong to the same trading pair in batch cancellation.The batch cancellation collects orders from multiple trading pairs but submits them with
exchange_symbols[0]as thesymbolparameter (lines 1251, 1269). If the Orderly API requires thesymbolparameter to match all orders in the batch, orders for different symbols will fail.Apply this diff to add validation:
# Get exchange symbols for all trading pairs exchange_symbols = [] for trading_pair in symbols: try: symbol = await self.exchange_symbol_associated_to_pair(trading_pair) exchange_symbols.append(symbol) except Exception as e: self.logger().error( f"[BATCH CANCEL] Error getting exchange symbol for {trading_pair}: {e}" ) if not exchange_symbols: self.logger().error("[BATCH CANCEL] No valid symbols found") return [ { "client_order_id": order.client_order_id, "success": False, "error_message": "No valid symbols found" } for order in orders_to_cancel ] + + # Validate all orders are for the same symbol + unique_symbols = list(dict.fromkeys(exchange_symbols)) + if len(unique_symbols) > 1: + self.logger().error( + f"[BATCH CANCEL] All orders must belong to the same trading pair. " + f"Found {len(unique_symbols)} different symbols: {unique_symbols}" + ) + return [ + { + "client_order_id": order.client_order_id, + "success": False, + "error_message": "Batch cancellation requires all orders to be for the same symbol" + } + for order in orders_to_cancel + ] + symbol_param = unique_symbols[0] # Build API request rest_assistant = await self._web_assistants_factory.get_rest_assistant() if use_exchange_ids: @@ params = { "order_ids": order_ids_str, - "symbol": exchange_symbols[0] + "symbol": symbol_param } @@ params = { "client_order_ids": client_order_ids_str, - "symbol": exchange_symbols[0] + "symbol": symbol_param }
🧹 Nitpick comments (1)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (1)
58-89: Consider explicit Optional type annotations.The parameters
orderly_perpetual_api_key,orderly_perpetual_api_secret, andorderly_perpetual_account_iddefault toNonebut aren't explicitly typed asOptional[str]. PEP 484 recommends explicitOptionalor the union syntaxstr | None.Apply this diff:
def __init__( self, balance_asset_limit: Optional[Dict[str, Dict[str, Decimal]]] = None, rate_limits_share_pct: Decimal = Decimal("100"), - orderly_perpetual_api_key: str = None, - orderly_perpetual_api_secret: str = None, - orderly_perpetual_account_id: str = None, + orderly_perpetual_api_key: Optional[str] = None, + orderly_perpetual_api_secret: Optional[str] = None, + orderly_perpetual_account_id: Optional[str] = None, trading_pairs: Optional[List[str]] = None, trading_required: bool = True, domain: str = CONSTANTS.DOMAIN, ):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py(1 hunks)hummingbot/connector/perpetual_trading.py(1 hunks)scripts/pmm_avellaneda_kodiak.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/pmm_avellaneda_kodiak.py (8)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (5)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/perpetual_trading.py (3)
set_leverage(153-160)logger(34-37)get_position(115-122)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (3)
batch_order_create(875-1152)name(96-98)batch_order_cancel(1154-1383)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (9)
hummingbot/core/web_assistant/connections/data_types.py (5)
json(112-125)RESTMethod(14-25)RESTRequest(29-37)url(93-95)method(98-100)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (2)
OrderlyPerpetualAuth(39-317)account_id(274-276)hummingbot/connector/utils.py (1)
combine_to_hb_trading_pair(34-36)hummingbot/core/data_type/in_flight_order.py (4)
InFlightOrder(91-395)OrderState(25-36)OrderUpdate(39-45)TradeUpdate(48-88)hummingbot/core/data_type/trade_fee.py (3)
TokenAmount(19-35)TradeFeeBase(75-245)new_perpetual_fee(115-130)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (3)
build_api_factory(150-170)format_trading_pair(261-288)public_rest_url(72-85)hummingbot/connector/exchange/cube/cube_exchange.py (2)
_set_trading_pair_symbol_map(887-891)trading_pair_symbol_map(1002-1008)hummingbot/core/web_assistant/rest_assistant.py (1)
execute_request(35-59)
🪛 Flake8 (7.3.0)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
[error] 18-18: 'json' imported but unused
(F401)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 45-45: 'hummingbot.core.web_assistant.connections.data_types.RESTRequest' imported but unused
(F401)
[error] 1287-1287: local variable 'error_msg' is assigned to but never used
(F841)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda_kodiak.py
257-257: Do not catch blind exception: Exception
(BLE001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
62-62: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
63-63: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
64-64: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
191-191: Unused method argument: trading_pair
(ARG002)
195-195: Unused method argument: trading_pair
(ARG002)
239-239: Do not catch blind exception: Exception
(BLE001)
273-273: Do not catch blind exception: Exception
(BLE001)
292-292: Abstract raise to an inner function
(TRY301)
292-292: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Consider moving this statement to an else block
(TRY300)
354-354: Avoid specifying long messages outside the exception class
(TRY003)
387-387: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Do not catch blind exception: Exception
(BLE001)
445-445: Unused method argument: request_exception
(ARG002)
510-510: Unused method argument: kwargs
(ARG002)
623-623: Consider moving this statement to an else block
(TRY300)
625-625: Do not catch blind exception: Exception
(BLE001)
675-675: Unused method argument: amount
(ARG002)
676-676: Unused method argument: trade_type
(ARG002)
677-677: Unused method argument: order_type
(ARG002)
678-678: Unused method argument: price
(ARG002)
680-680: Unused method argument: position_action
(ARG002)
715-715: Unused method argument: amount
(ARG002)
716-716: Unused method argument: trade_type
(ARG002)
717-717: Unused method argument: order_type
(ARG002)
718-718: Unused method argument: price
(ARG002)
720-720: Unused method argument: position_action
(ARG002)
763-763: Unused method argument: kwargs
(ARG002)
818-818: Avoid specifying long messages outside the exception class
(TRY003)
873-873: Avoid specifying long messages outside the exception class
(TRY003)
909-912: Avoid specifying long messages outside the exception class
(TRY003)
962-962: Do not catch blind exception: Exception
(BLE001)
1003-1003: Do not catch blind exception: Exception
(BLE001)
1047-1047: Abstract raise to an inner function
(TRY301)
1047-1047: Avoid specifying long messages outside the exception class
(TRY003)
1133-1133: Consider moving this statement to an else block
(TRY300)
1138-1138: Do not catch blind exception: Exception
(BLE001)
1152-1152: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1152-1152: Avoid specifying long messages outside the exception class
(TRY003)
1181-1184: Avoid specifying long messages outside the exception class
(TRY003)
1220-1220: Do not catch blind exception: Exception
(BLE001)
1287-1287: Local variable error_msg is assigned to but never used
Remove assignment to unused variable error_msg
(F841)
1289-1289: Abstract raise to an inner function
(TRY301)
1289-1289: Avoid specifying long messages outside the exception class
(TRY003)
1376-1376: Consider moving this statement to an else block
(TRY300)
1381-1381: Do not catch blind exception: Exception
(BLE001)
1383-1383: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1383-1383: Avoid specifying long messages outside the exception class
(TRY003)
1418-1418: Avoid specifying long messages outside the exception class
(TRY003)
1503-1503: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1503-1503: Avoid specifying long messages outside the exception class
(TRY003)
1504-1504: Do not catch blind exception: Exception
(BLE001)
1573-1573: Do not catch blind exception: Exception
(BLE001)
1614-1614: Do not catch blind exception: Exception
(BLE001)
1615-1615: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1628-1628: Unused method argument: trading_pair
(ARG002)
1718-1718: Consider moving this statement to an else block
(TRY300)
1720-1720: Do not catch blind exception: Exception
(BLE001)
1744-1744: Unused method argument: position_action
(ARG002)
1799-1799: Do not catch blind exception: Exception
(BLE001)
1826-1826: Unused method argument: event
(ARG002)
1831-1831: Unused method argument: event
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
hummingbot/connector/perpetual_trading.py (1)
175-184: Fix the comment reference to the correct method name.The guard is correct and necessary to prevent KeyError during the initialization window. However, the comment at line 179 is inaccurate. The method
_init_funding_info()exists in the parent class (perpetual_derivative_py_base.py), not inperpetual_trading.py. The actual method that initializes funding info here isinitialize_funding_info()(line 67).The initialization flow confirms dropped updates are acceptable: the REST API fetch (via
_init_funding_info()in the parent) completes first, then the stream listener starts, so only updates arriving in that narrow window are dropped.Apply this diff:
- # It will be initialized by _init_funding_info() and subsequent updates will be processed + # It will be initialized by initialize_funding_info() and subsequent updates will be processedscripts/pmm_avellaneda_kodiak.py (5)
21-40: LGTM!The configuration model is well-structured with appropriate defaults and type hints. The commented-out fields provide helpful context for future enhancements.
86-102: LGTM!The tick orchestration and sequential cancel-then-place flow are correctly implemented, ensuring orders are properly refreshed without accumulation.
182-261: LGTM!The async order placement and cancellation methods are well-implemented with proper error handling and defensive checks. The broad exception catch at Line 257 is appropriate for robustness during cancellation, as it prevents strategy crashes while maintaining visibility through logging.
263-321: LGTM!The fill tracking and inventory calculation are correctly implemented. The defensive timestamp parsing and position-to-signed-inventory conversion properly handle edge cases.
323-406: LGTM!The reservation price calculation correctly implements the Avellaneda-Stoikov inventory-based adjustment with appropriate thresholds and sign handling. The status formatting provides comprehensive visibility into strategy state.
| order_params = { | ||
| "symbol": symbol, | ||
| "client_order_id": in_flight_order.client_order_id, | ||
| "side": "BUY" if in_flight_order.trade_type == TradeType.BUY else "SELL", | ||
| "order_type": orderly_order_type, | ||
| "order_quantity": float(in_flight_order.amount), | ||
| "reduce_only": in_flight_order.position == PositionAction.CLOSE, | ||
| } | ||
|
|
||
| # Add price for non-MARKET orders | ||
| if in_flight_order.order_type != OrderType.MARKET: | ||
| order_params["order_price"] = float(in_flight_order.price) | ||
|
|
||
| batch_orders.append(order_params) |
There was a problem hiding this comment.
Fix incorrect position_action reference.
Line 994 accesses in_flight_order.position to determine reduce_only, but InFlightOrder does not have a position attribute that stores PositionAction. The position action is not persisted in the InFlightOrder object after tracking starts.
You need to either:
- Store
position_actionwhen building the batch (extract fromorders_to_create), or - Track it separately in a map alongside
order_id_map
Apply this diff to track position_action:
# Step 1: Generate client_order_ids and track/validate orders
inflight_orders_to_create = []
order_id_map = {} # Map index to order_id for result matching
+ position_action_map = {} # Map client_order_id to position_action
for i, order_data in enumerate(orders_to_create):
try:
@@
if valid_order is not None:
inflight_orders_to_create.append(valid_order)
order_id_map[i] = order_id
+ position_action_map[order_id] = position_action
else:
@@
order_params = {
"symbol": symbol,
"client_order_id": in_flight_order.client_order_id,
"side": "BUY" if in_flight_order.trade_type == TradeType.BUY else "SELL",
"order_type": orderly_order_type,
"order_quantity": float(in_flight_order.amount),
- "reduce_only": in_flight_order.position == PositionAction.CLOSE,
+ "reduce_only": position_action_map.get(in_flight_order.client_order_id, PositionAction.NIL) == PositionAction.CLOSE,
}The same issue exists at lines 1045, 1086, 1101, and 1150 where in_flight_order.position is passed to _on_order_creation_failure - these should use the mapped position_action value instead.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
around lines 988-1001 (and also at lines 1045, 1086, 1101, 1150), the code
incorrectly reads in_flight_order.position to decide reduce_only and to pass
into _on_order_creation_failure even though InFlightOrder doesn't persist
PositionAction; extract and persist the position_action when building the batch
(e.g., pull PositionAction from orders_to_create entries while constructing
order_params) or maintain a separate map keyed by client_order_id/order_id
alongside order_id_map; then replace all uses of in_flight_order.position in
these locations with the mapped position_action value (use the map lookup when
handling creation success/failure and when setting reduce_only) so
position_action is correctly available throughout order creation flow.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (3)
17-46: Remove unused imports flagged in previous review.The imports
json,TYPE_CHECKING,combine_to_hb_trading_pair, andRESTRequestare still present despite being marked as addressed in a previous review. These are not referenced anywhere in the file.Apply this diff:
-import json from decimal import Decimal -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple @@ -from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id +from hummingbot.connector.utils import get_new_client_order_id @@ -from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest +from hummingbot.core.web_assistant.connections.data_types import RESTMethod
989-1002: Critical:in_flight_order.positionattribute doesn't exist.Line 995 accesses
in_flight_order.positionto setreduce_only, butInFlightOrderdoes not have apositionattribute. The same issue occurs at lines 1046, 1087, 1102, and 1151. This will causeAttributeErrorat runtime.The
position_actionextracted at line 943 must be persisted in a separate map. Apply this diff:# Step 1: Generate client_order_ids and track/validate orders inflight_orders_to_create = [] order_id_map = {} # Map index to order_id for result matching + position_action_map = {} # Map client_order_id to position_action for i, order_data in enumerate(orders_to_create): try: @@ if valid_order is not None: inflight_orders_to_create.append(valid_order) order_id_map[i] = order_id + position_action_map[order_id] = position_action else: @@ order_params = { "symbol": symbol, "client_order_id": in_flight_order.client_order_id, "side": "BUY" if in_flight_order.trade_type == TradeType.BUY else "SELL", "order_type": orderly_order_type, "order_quantity": float(in_flight_order.amount), - "reduce_only": in_flight_order.position == PositionAction.CLOSE, + "reduce_only": position_action_map.get(in_flight_order.client_order_id, PositionAction.NIL) == PositionAction.CLOSE, }Also update lines 1046, 1087, 1102, and 1151 to use
position_action_map.get(in_flight_order.client_order_id, PositionAction.NIL)instead ofin_flight_order.position.
1215-1253: Batch cancellation with mixed trading pairs will fail.Lines 1252 and 1270 always use
exchange_symbols[0]when submitting the batch request. Iforders_to_cancelincludes orders from multiple trading pairs, the API will reject orders that don't match the submitted symbol.Add validation to enforce single-pair batches:
# Get exchange symbols for all trading pairs exchange_symbols = [] for trading_pair in symbols: try: symbol = await self.exchange_symbol_associated_to_pair(trading_pair) exchange_symbols.append(symbol) except Exception as e: self.logger().error( f"[BATCH CANCEL] Error getting exchange symbol for {trading_pair}: {e}" ) + unique_symbols = list(dict.fromkeys(exchange_symbols)) + if len(unique_symbols) != 1: + raise ValueError( + "Batch order cancellation requires all orders to belong to the same trading pair." + ) + symbol_param = unique_symbols[0] + if not exchange_symbols: @@ params = { "order_ids": order_ids_str, - "symbol": exchange_symbols[0] + "symbol": symbol_param, }Apply the same fix at line 1270.
scripts/pmm_avellaneda_kodiak.py (3)
66-77: Leverage configuration is never applied.The
apply_initial_settingmethod (lines 115-119) sets the connector's leverage but is never called. Without this call, the strategy will trade at the default exchange leverage instead ofconfig.leverage.Call
apply_initial_setting()at the end of__init__:super().__init__(connectors) self.config = config self._last_update_timestamp: float = 0 self._cached_mark_price: Decimal = Decimal("0") self._cached_reservation_price: Decimal = Decimal("0") # DataFrame to store last 6 filled orders self._filled_orders_df: pd.DataFrame = pd.DataFrame(columns=[ "Timestamp", "Order ID", "Side", "Amount", "Price", "Inventory" ]) + + self.apply_initial_setting()
138-142: Validate spread array lengths to prevent IndexError.Line 139 indexes
ask_spread_levels[idx]assuming both spread arrays have the same length. If they don't, the code will crash withIndexErrorduring order generation.Add validation before the loop or use
zip():orders = [] + if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + for idx, bid_spread in enumerate(self.config.bid_spread_levels): ask_spread = self.config.ask_spread_levels[idx]Or use
zip()for safer iteration:orders = [] - for idx, bid_spread in enumerate(self.config.bid_spread_levels): - ask_spread = self.config.ask_spread_levels[idx] + for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels):
185-190: Callbudget_checker()method to get checker instance.Line 187 assigns the bound method
connector.budget_checkerinstead of calling it. Line 189 will raiseAttributeErrorbecause you cannot calladjust_candidates()on a method reference.def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: connector = self.connectors[self.config.exchange] - budget_checker = connector.budget_checker + budget_checker = connector.budget_checker() proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) return proposals_adjusted
🧹 Nitpick comments (1)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (1)
1286-1291: Remove unused variable assignment.Line 1288 assigns
error_msgbut never uses it. The variable can be safely removed.if not response.get("success", False): # Check if this is an "order not found/invalid" error - orders don't exist on exchange - error_msg = str(response) error_exception = IOError(f"Batch order cancellation failed: {response}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py(1 hunks)scripts/pmm_avellaneda_kodiak.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/pmm_avellaneda_kodiak.py (8)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (6)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)get(122-125)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/perpetual_trading.py (2)
set_leverage(153-160)get_position(115-122)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (2)
batch_order_create(876-1153)batch_order_cancel(1155-1402)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (5)
hummingbot/core/web_assistant/connections/data_types.py (5)
json(112-125)RESTMethod(14-25)RESTRequest(29-37)url(93-95)method(98-100)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (1)
account_id(274-276)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (1)
build_api_factory(150-170)hummingbot/core/web_assistant/rest_assistant.py (1)
execute_request(35-59)
🪛 Flake8 (7.3.0)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
[error] 18-18: 'json' imported but unused
(F401)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 45-45: 'hummingbot.core.web_assistant.connections.data_types.RESTRequest' imported but unused
(F401)
[error] 1288-1288: local variable 'error_msg' is assigned to but never used
(F841)
🪛 Ruff (0.14.4)
scripts/pmm_avellaneda_kodiak.py
267-267: Do not catch blind exception: Exception
(BLE001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
62-62: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
63-63: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
64-64: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
191-191: Unused method argument: trading_pair
(ARG002)
195-195: Unused method argument: trading_pair
(ARG002)
239-239: Do not catch blind exception: Exception
(BLE001)
273-273: Do not catch blind exception: Exception
(BLE001)
292-292: Abstract raise to an inner function
(TRY301)
292-292: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Consider moving this statement to an else block
(TRY300)
354-354: Avoid specifying long messages outside the exception class
(TRY003)
387-387: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Do not catch blind exception: Exception
(BLE001)
445-445: Unused method argument: request_exception
(ARG002)
511-511: Unused method argument: kwargs
(ARG002)
624-624: Consider moving this statement to an else block
(TRY300)
626-626: Do not catch blind exception: Exception
(BLE001)
676-676: Unused method argument: amount
(ARG002)
677-677: Unused method argument: trade_type
(ARG002)
678-678: Unused method argument: order_type
(ARG002)
679-679: Unused method argument: price
(ARG002)
681-681: Unused method argument: position_action
(ARG002)
716-716: Unused method argument: amount
(ARG002)
717-717: Unused method argument: trade_type
(ARG002)
718-718: Unused method argument: order_type
(ARG002)
719-719: Unused method argument: price
(ARG002)
721-721: Unused method argument: position_action
(ARG002)
764-764: Unused method argument: kwargs
(ARG002)
819-819: Avoid specifying long messages outside the exception class
(TRY003)
874-874: Avoid specifying long messages outside the exception class
(TRY003)
910-913: Avoid specifying long messages outside the exception class
(TRY003)
963-963: Do not catch blind exception: Exception
(BLE001)
1004-1004: Do not catch blind exception: Exception
(BLE001)
1048-1048: Abstract raise to an inner function
(TRY301)
1048-1048: Avoid specifying long messages outside the exception class
(TRY003)
1134-1134: Consider moving this statement to an else block
(TRY300)
1139-1139: Do not catch blind exception: Exception
(BLE001)
1153-1153: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1153-1153: Avoid specifying long messages outside the exception class
(TRY003)
1182-1185: Avoid specifying long messages outside the exception class
(TRY003)
1221-1221: Do not catch blind exception: Exception
(BLE001)
1288-1288: Local variable error_msg is assigned to but never used
Remove assignment to unused variable error_msg
(F841)
1316-1316: Abstract raise to an inner function
(TRY301)
1316-1316: Avoid specifying long messages outside the exception class
(TRY003)
1395-1395: Consider moving this statement to an else block
(TRY300)
1400-1400: Do not catch blind exception: Exception
(BLE001)
1402-1402: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1402-1402: Avoid specifying long messages outside the exception class
(TRY003)
1437-1437: Avoid specifying long messages outside the exception class
(TRY003)
1522-1522: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1522-1522: Avoid specifying long messages outside the exception class
(TRY003)
1523-1523: Do not catch blind exception: Exception
(BLE001)
1592-1592: Do not catch blind exception: Exception
(BLE001)
1633-1633: Do not catch blind exception: Exception
(BLE001)
1634-1634: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1647-1647: Unused method argument: trading_pair
(ARG002)
1737-1737: Consider moving this statement to an else block
(TRY300)
1739-1739: Do not catch blind exception: Exception
(BLE001)
1763-1763: Unused method argument: position_action
(ARG002)
1818-1818: Do not catch blind exception: Exception
(BLE001)
1845-1845: Unused method argument: event
(ARG002)
1850-1850: Unused method argument: event
(ARG002)
fix handling of potentially crossing orders
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
scripts/pmm_kodiak.py (3)
66-77: Invoke leverage setup on initialization.
apply_initial_settingis never called, so Line 118's leverage configuration is skipped and the connector trades at its default leverage, contradicting the script config. Call it during__init__so the requested leverage is applied exactly once when the strategy boots.Apply this diff:
def __init__(self, connectors: Dict[str, ConnectorBase], config: PMMAvellanedaMultiConfig): super().__init__(connectors) self.config = config self._last_update_timestamp: float = 0 self._cached_mark_price: Decimal = Decimal("0") self._cached_reservation_price: Decimal = Decimal("0") # DataFrame to store last 6 filled orders self._filled_orders_df: pd.DataFrame = pd.DataFrame(columns=[ "Timestamp", "Order ID", "Side", "Amount", "Price", "Inventory" ]) + self.apply_initial_setting()
153-201: Validate spread arrays before iterating.Line 154 assumes
ask_spread_levels[idx]exists; if a user provides mismatched lengths, this raisesIndexErrorand the strategy crashes mid-run. Guard the configuration (or iterate withzip) so we fail fast with a clear error instead of crashing during order generation.Apply this diff:
orders = [] - for idx, bid_spread in enumerate(self.config.bid_spread_levels): - ask_spread = self.config.ask_spread_levels[idx] + if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + + for bid_spread, ask_spread in zip(self.config.bid_spread_levels, self.config.ask_spread_levels): bid_price = reservation_price * (Decimal("1") - bid_spread) ask_price = reservation_price * (Decimal("1") + ask_spread)
203-208: Callbudget_checker()before using it.Line 205 stores the bound method
connector.budget_checkerinstead of the checker instance, sobudget_checker.adjust_candidates(...)raisesAttributeErrorat runtime. Invoke the method to obtain the checker before callingadjust_candidates.Apply this diff:
def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: connector = self.connectors[self.config.exchange] - budget_checker = connector.budget_checker + budget_checker = connector.budget_checker() proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) return proposals_adjustedhummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (2)
17-46: Remove unused imports to pass linting.The following imports are flagged by Flake8 (F401) and should be removed:
- Line 18:
json- Line 20:
TYPE_CHECKINGfrom typing- Line 36:
combine_to_hb_trading_pairfrom hummingbot.connector.utils- Line 45:
RESTRequestfrom connections.data_typesApply this diff:
-import json from decimal import Decimal -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple from bidict import bidict @@ -from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id +from hummingbot.connector.utils import get_new_client_order_id @@ -from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest +from hummingbot.core.web_assistant.connections.data_types import RESTMethod
1155-1402: Remove unused variable and add validation for same trading pair in batch cancellation.
- Line 1288: Remove unused variable
error_msg:if not response.get("success", False): # Check if this is an "order not found/invalid" error - orders don't exist on exchange - error_msg = str(response) error_exception = IOError(f"Batch order cancellation failed: {response}")
- Lines 1252, 1270: The code uses
exchange_symbols[0]as the symbol parameter, but Orderly's batch cancel API requires a single symbol. If orders span multiple trading pairs, the API will only apply the cancellation to orders matching the first symbol. Add validation before building the API request to ensure all orders belong to the same trading pair:# Get exchange symbols for all trading pairs exchange_symbols = [] for trading_pair in symbols: try: symbol = await self.exchange_symbol_associated_to_pair(trading_pair) exchange_symbols.append(symbol) except Exception as e: self.logger().error( f"[BATCH CANCEL] Error getting exchange symbol for {trading_pair}: {e}" ) + # Orderly batch cancel requires all orders to belong to the same symbol + if len(exchange_symbols) > 1: + raise ValueError( + f"Batch order cancellation requires all orders to belong to the same trading pair. " + f"Found {len(exchange_symbols)} different symbols: {exchange_symbols}" + ) + if not exchange_symbols: self.logger().error("[BATCH CANCEL] No valid symbols found")
🧹 Nitpick comments (2)
scripts/pmm_kodiak.py (1)
128-142: Remove unused variable.Line 140 calculates
mid_price_weightedbut never uses it, adding unnecessary code clutter.Apply this diff to remove the unused variable:
mid_price = (best_bid_price + best_ask_price) / 2 - mid_price_weighted = ((best_bid_price * best_ask_size) + (best_bid_size * best_ask_price)) / (best_bid_size + best_ask_size) - - # self.logger().info(f"Mid price: {mid_price}, Mid price weighted: {mid_price_weighted}, bid_size: {best_bid_size}, ask_size: {best_ask_size}, Difference: {mid_price_weighted - mid_price}") - inventory = self._get_current_inventory()hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (1)
58-89: Consider using modern type hint syntax.Lines 62-64 use implicit
Optionalwhich could be modernized to the PEP 484 preferredT | Nonesyntax for Python 3.10+.Apply this diff if targeting Python 3.10+:
def __init__( self, balance_asset_limit: Optional[Dict[str, Dict[str, Decimal]]] = None, rate_limits_share_pct: Decimal = Decimal("100"), - orderly_perpetual_api_key: str = None, - orderly_perpetual_api_secret: str = None, - orderly_perpetual_account_id: str = None, + orderly_perpetual_api_key: str | None = None, + orderly_perpetual_api_secret: str | None = None, + orderly_perpetual_account_id: str | None = None, trading_pairs: Optional[List[str]] = None, trading_required: bool = True, domain: str = CONSTANTS.DOMAIN, ):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py(1 hunks)scripts/pmm_kodiak.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/pmm_kodiak.py (6)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (6)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)get(122-125)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (7)
hummingbot/core/web_assistant/connections/data_types.py (5)
json(112-125)RESTMethod(14-25)RESTRequest(29-37)url(93-95)method(98-100)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (2)
OrderlyPerpetualAuth(39-317)account_id(274-276)hummingbot/core/data_type/in_flight_order.py (4)
InFlightOrder(91-395)OrderUpdate(39-45)TradeUpdate(48-88)quote_asset(169-170)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (4)
build_api_factory(150-170)format_trading_pair(261-288)public_rest_url(72-85)is_exchange_information_valid(220-258)hummingbot/connector/exchange/cube/cube_exchange.py (1)
trading_pair_symbol_map(1002-1008)hummingbot/core/web_assistant/rest_assistant.py (1)
execute_request(35-59)
🪛 Flake8 (7.3.0)
scripts/pmm_kodiak.py
[error] 128-128: unexpected indentation (comment)
(E116)
[error] 140-140: local variable 'mid_price_weighted' is assigned to but never used
(F841)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
[error] 18-18: 'json' imported but unused
(F401)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 45-45: 'hummingbot.core.web_assistant.connections.data_types.RESTRequest' imported but unused
(F401)
[error] 1288-1288: local variable 'error_msg' is assigned to but never used
(F841)
🪛 Ruff (0.14.4)
scripts/pmm_kodiak.py
140-140: Local variable mid_price_weighted is assigned to but never used
Remove assignment to unused variable mid_price_weighted
(F841)
285-285: Do not catch blind exception: Exception
(BLE001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
62-62: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
63-63: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
64-64: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
191-191: Unused method argument: trading_pair
(ARG002)
195-195: Unused method argument: trading_pair
(ARG002)
239-239: Do not catch blind exception: Exception
(BLE001)
273-273: Do not catch blind exception: Exception
(BLE001)
292-292: Abstract raise to an inner function
(TRY301)
292-292: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Consider moving this statement to an else block
(TRY300)
354-354: Avoid specifying long messages outside the exception class
(TRY003)
387-387: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Do not catch blind exception: Exception
(BLE001)
445-445: Unused method argument: request_exception
(ARG002)
511-511: Unused method argument: kwargs
(ARG002)
624-624: Consider moving this statement to an else block
(TRY300)
626-626: Do not catch blind exception: Exception
(BLE001)
676-676: Unused method argument: amount
(ARG002)
677-677: Unused method argument: trade_type
(ARG002)
678-678: Unused method argument: order_type
(ARG002)
679-679: Unused method argument: price
(ARG002)
681-681: Unused method argument: position_action
(ARG002)
716-716: Unused method argument: amount
(ARG002)
717-717: Unused method argument: trade_type
(ARG002)
718-718: Unused method argument: order_type
(ARG002)
719-719: Unused method argument: price
(ARG002)
721-721: Unused method argument: position_action
(ARG002)
764-764: Unused method argument: kwargs
(ARG002)
819-819: Avoid specifying long messages outside the exception class
(TRY003)
874-874: Avoid specifying long messages outside the exception class
(TRY003)
910-913: Avoid specifying long messages outside the exception class
(TRY003)
963-963: Do not catch blind exception: Exception
(BLE001)
1004-1004: Do not catch blind exception: Exception
(BLE001)
1048-1048: Abstract raise to an inner function
(TRY301)
1048-1048: Avoid specifying long messages outside the exception class
(TRY003)
1134-1134: Consider moving this statement to an else block
(TRY300)
1139-1139: Do not catch blind exception: Exception
(BLE001)
1153-1153: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1153-1153: Avoid specifying long messages outside the exception class
(TRY003)
1182-1185: Avoid specifying long messages outside the exception class
(TRY003)
1221-1221: Do not catch blind exception: Exception
(BLE001)
1288-1288: Local variable error_msg is assigned to but never used
Remove assignment to unused variable error_msg
(F841)
1316-1316: Abstract raise to an inner function
(TRY301)
1316-1316: Avoid specifying long messages outside the exception class
(TRY003)
1395-1395: Consider moving this statement to an else block
(TRY300)
1400-1400: Do not catch blind exception: Exception
(BLE001)
1402-1402: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1402-1402: Avoid specifying long messages outside the exception class
(TRY003)
1437-1437: Avoid specifying long messages outside the exception class
(TRY003)
1522-1522: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1522-1522: Avoid specifying long messages outside the exception class
(TRY003)
1523-1523: Do not catch blind exception: Exception
(BLE001)
1592-1592: Do not catch blind exception: Exception
(BLE001)
1633-1633: Do not catch blind exception: Exception
(BLE001)
1634-1634: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1647-1647: Unused method argument: trading_pair
(ARG002)
1737-1737: Consider moving this statement to an else block
(TRY300)
1739-1739: Do not catch blind exception: Exception
(BLE001)
1763-1763: Unused method argument: position_action
(ARG002)
1818-1818: Do not catch blind exception: Exception
(BLE001)
1845-1845: Unused method argument: event
(ARG002)
1850-1850: Unused method argument: event
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (17)
scripts/pmm_kodiak.py (8)
21-40: Configuration class looks well-structured.The PMMAvellanedaMultiConfig follows the BaseClientModel pattern correctly and includes appropriate fields for the Avellaneda-Stoikov strategy parameters.
78-82: LGTM!Clean helper method that correctly delegates to the connector's order tracker.
86-113: LGTM!The sequential cancel-then-place logic correctly prevents order accumulation, and the cancellation result verification ensures orders aren't placed if cancellations fail.
210-232: LGTM!The order placement logic correctly converts PerpetualOrderCandidate objects to the required dictionary format for batch creation.
233-290: LGTM!The batch cancellation logic correctly filters orders by trading pair and open state, with appropriate logging. The broad exception catch at Line 285 is acceptable here since cancellation failures are expected and properly handled with warning logs.
292-325: LGTM!The fill handler correctly updates tracking data with defensive timestamp parsing and maintains a rolling window of recent fills.
327-385: LGTM!The inventory tracking correctly retrieves signed position amounts from the connector, and the reservation price calculation properly implements the Avellaneda-Stoikov model with linear adjustment based on inventory thresholds.
389-436: LGTM!The status formatting provides comprehensive strategy state information using cached values efficiently, with clear presentation of active and filled orders.
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (9)
100-116: LGTM - Security issue resolved.The authenticator property correctly constructs and returns the auth object without logging sensitive credentials. The debug logging flagged in previous reviews has been properly removed.
310-428: LGTM - Trading rules implementation looks solid.The trading rules fetching and parsing logic correctly:
- Fetches from the Orderly API endpoint
- Validates response structure
- Handles parsing errors gracefully
- Maps Orderly fields to Hummingbot TradingRule objects
502-640: LGTM - Order validation logic is comprehensive.The
_start_tracking_and_validate_ordermethod properly:
- Calculates market order prices
- Quantizes price and amount
- Validates order type support
- Checks min order size
- Validates min notional size
- Handles failures through the order tracker
755-874: LGTM - Order placement and cancellation properly implemented.Both methods correctly:
- Map Hummingbot types to Orderly equivalents
- Use appropriate API endpoints
- Handle different order ID types (exchange vs client)
- Include required parameters (symbol, reduce_only, etc.)
1404-1526: LGTM - Order status tracking implemented correctly.Both methods properly:
- Handle missing exchange order IDs
- Fetch data from appropriate endpoints
- Parse responses into update objects
- Handle errors with appropriate exceptions
The exception re-raising patterns are acceptable for this context.
1532-1660: LGTM - Position management properly implemented.The position management logic correctly:
- Fetches and parses position data
- Handles zero positions (removal)
- Determines position side from quantity sign
- Updates position objects with PnL and entry price
- Enforces ONEWAY mode limitation per Orderly's design
1666-1741: LGTM - Balance and funding data handling is secure and correct.Both methods properly:
- Fetch data from appropriate endpoints
- Parse response structures
- Handle errors gracefully
- No longer log sensitive balance information (security issue from past reviews properly resolved)
1747-1865: LGTM - Fee handling and user stream processing properly implemented.The implementation correctly:
- Uses per-trade fee retrieval (appropriate for Orderly)
- Routes WebSocket events by topic
- Processes order updates through the order tracker
- Triggers position and balance refreshes on events
- Uses concurrent fetching in status polling loop
876-1153: Now let me search the actual codebase for the_start_tracking_and_validate_ordermethod to verify howposition_actionis passed toInFlightOrder:Based on my verification of the codebase, the review comment's concern appears to be incorrect. Here's what I found:
Analysis of InFlightOrder.position:
From the codebase examination,
InFlightOrder.positionis:
- Properly typed as
PositionAction(constructor parameter at line 106)- Assigned in the constructor (line 118:
self.position = position)- Correctly deserialized from JSON (line 242:
position=PositionAction(data["position"]))- Not modified after creation
Evidence against the review comment:
The code structure demonstrates thatInFlightOrderdoes persistPositionActioncorrectly in itspositionattribute. Usingin_flight_order.positiondirectly (as done at lines 995, 1046, 1087, 1102, 1151) should work as intended, assuming theposition_actionparameter is passed correctly to theInFlightOrderconstructor during creation.Remaining verification needed:
The only potential issue would be if_start_tracking_and_validate_orderfails to pass theposition_actionparameter when creating theInFlightOrder. Without direct access to that method's implementation, this cannot be definitively ruled out, but the current code structure strongly suggests this is functioning correctly.Recommendation:
The suggested fix (maintaining a separateposition_action_map) is unnecessary ifInFlightOrderis being instantiated correctly with theposition_actionparameter. The review comment should be reconsidered.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
conf/scripts/Kodiak_btc.yml (1)
1-9: See review comments fromconf/scripts/pmm_kodiak_example.yml.This file contains identical configuration to
conf/scripts/pmm_kodiak_example.yml. The issues flagged there regarding duplication, excessive leverage, and naming inconsistency apply here as well.
🧹 Nitpick comments (1)
conf/scripts/pmm_kodiak_example.yml (1)
1-9: Naming inconsistency between similar configuration files.This file uses snake_case (
pmm_kodiak_example.yml), whileconf/scripts/Kodiak_btc.ymluses camelCase. Standardize the naming convention across example configuration files for consistency and maintainability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
conf/scripts/Kodiak_btc.yml(1 hunks)conf/scripts/pmm_kodiak_example.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
| exchange: orderly_perpetual | ||
| trading_pair: BTC-USDC | ||
| order_amount_quote: 20 | ||
| bid_spread_levels: [0.0005] | ||
| ask_spread_levels: [0.0005] | ||
| order_refresh_time: 10 | ||
| max_inventory: 0.002 # 200 usd | ||
| max_price_adjustment: 0.001 # 10 bps | ||
| leverage: 100 |
There was a problem hiding this comment.
Duplicate configuration files with identical content.
This file is identical to conf/scripts/Kodiak_btc.yml. Having two separate files with the same configuration creates maintenance burden and potential for drift. Clarify the purpose of both files—if they serve different use cases, document those differences clearly; otherwise, consolidate into a single example file.
🤖 Prompt for AI Agents
In conf/scripts/pmm_kodiak_example.yml lines 1-9 the configuration is identical
to conf/scripts/Kodiak_btc.yml, causing duplicate files and maintenance risk;
either consolidate by keeping a single canonical example (e.g.,
conf/scripts/kodiak_example.yml) and remove the duplicate, or if both must
exist, add a header comment in each file explaining their distinct purpose and
update any documentation or references to point to the canonical file; ensure
any CI/tests or docs that reference the removed filename are updated to the
retained name (or replace the duplicate with a short README or symlink) so no
references break.
| order_refresh_time: 10 | ||
| max_inventory: 0.002 # 200 usd | ||
| max_price_adjustment: 0.001 # 10 bps | ||
| leverage: 100 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that 100x leverage is intentional and document associated risks.
The leverage value of 100 is extremely high and represents significant liquidation risk. Confirm this is:
- Intentional (not a typo or placeholder)
- Appropriate for the intended use case and acceptable risk tolerance
- Documented in strategy guidelines or risk management documentation
Consider adding a risk warning in accompanying documentation.
🏁 Script executed:
#!/bin/bash
# Examine the file and surrounding context
echo "=== File content: conf/scripts/pmm_kodiak_example.yml ==="
cat conf/scripts/pmm_kodiak_example.yml
echo -e "\n=== Search for other leverage values in the repository ==="
rg -i 'leverage' --type yaml -C 3
echo -e "\n=== Search for risk management or documentation files ==="
fd -e md -e txt -e rst | head -20 | xargs grep -l -i 'risk\|leverage' 2>/dev/null
echo -e "\n=== Check for other example or config files ==="
fd -e yml -e yaml | grep -E '(example|config|kodiak|pmm)' | head -20Length of output: 20635
🏁 Script executed:
#!/bin/bash
echo "=== File content: conf/scripts/pmm_kodiak_example.yml ==="
cat conf/scripts/pmm_kodiak_example.yml
echo -e "\n=== Search for leverage values across YAML files ==="
rg -i 'leverage' --type yaml -C 2
echo -e "\n=== Search for documentation mentioning risk or leverage ==="
fd -type f \( -name "*.md" -o -name "*.txt" -o -name "*.rst" \) | xargs grep -l -i 'leverage\|risk' 2>/dev/null || echo "No matches found"
echo -e "\n=== List YAML/YML files that might be config or example related ==="
fd -type f \( -name "*.yml" -o -name "*.yaml" \) | head -30Length of output: 16638
🏁 Script executed:
#!/bin/bash
# Search for leverage validation in the strategy and connector code
echo "=== Search for leverage validation or constraint checking ==="
rg -n 'leverage' --type py -C 3 | grep -iE '(valid|constraint|check|range|max|min|error)' | head -40
echo -e "\n=== Look for perpetual market making strategy files ==="
fd -type f -name "*perpetual*market*making*" --type f
echo -e "\n=== Search for orderly connector leverage handling ==="
fd -type f -path "*orderly*" -name "*.py" | head -20
echo -e "\n=== Check if leverage is validated before submission to exchange ==="
rg -n 'def.*leverage' --type py -A 5 | head -50Length of output: 10409
🏁 Script executed:
#!/bin/bash
# Find orderly perpetual connector files
echo "=== Orderly perpetual connector files ==="
find . -path "*orderly*perpetual*" -name "*.py" | grep -v __pycache__ | grep -v test
echo -e "\n=== Search for set_leverage in orderly connector ==="
find . -path "*orderly*perpetual*" -name "*.py" -exec grep -l "set_leverage" {} \;
echo -e "\n=== Look for leverage validation or constraint checking in orderly ==="
find . -path "*orderly*perpetual*" -name "*.py" -exec grep -n "leverage" {} + | grep -iE "(valid|range|check|between|1.*50|max)" | head -20Length of output: 1271
🏁 Script executed:
#!/bin/bash
# Extract the set_leverage method implementation
echo "=== set_leverage method in orderly_perpetual_derivative.py ==="
rg -A 30 'def set_leverage' hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
echo -e "\n=== Look for any validation or range checks ==="
rg -B 5 -A 10 'leverage' hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py | grep -iE '(valid|range|check|error|exception|1.*50|between)' -A 2 -B 2 | head -40Length of output: 806
🏁 Script executed:
#!/bin/bash
# Get the full _set_trading_pair_leverage method
echo "=== Full _set_trading_pair_leverage implementation ==="
rg -B 5 -A 50 'async def _set_trading_pair_leverage' hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
echo -e "\n=== Check for any leverage constraints in constants ==="
rg 'leverage' hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_constants.py | head -20Length of output: 2370
Reduce leverage from 100 to valid range (1-50) per Orderly Perpetual API constraints.
The value 100 violates the Orderly Perpetual API's documented maximum leverage of 50 ("Integer between 1 to 50" per orderly_swagger.yml). The connector's _set_trading_pair_leverage method performs no client-side validation and passes the value directly to the API, where it will be rejected at runtime. Reduce to a valid value (e.g., 10-20 for a conservative example, or 50 for maximum).
🤖 Prompt for AI Agents
In conf/scripts/pmm_kodiak_example.yml around line 9 the leverage is set to 100
which exceeds the Orderly Perpetual API allowed range (1–50); change the
leverage value to a valid integer within 1–50 (for example 10, 20, or 50) so the
connector won’t pass an invalid value to the API and cause runtime rejection.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (2)
17-46: Remove unused imports to unblock lint.The following imports are unused and causing F401 linter errors:
json(line 18)TYPE_CHECKING(line 20)combine_to_hb_trading_pair(line 36)RESTRequest(line 45)Note:
get_new_client_order_idon line 36 IS used (line 930), so keep that import.Apply this diff:
-import json from decimal import Decimal -from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Tuple from bidict import bidict @@ -from hummingbot.connector.utils import combine_to_hb_trading_pair, get_new_client_order_id +from hummingbot.connector.utils import get_new_client_order_id from hummingbot.core.api_throttler.data_types import RateLimit @@ -from hummingbot.core.web_assistant.connections.data_types import RESTMethod, RESTRequest +from hummingbot.core.web_assistant.connections.data_types import RESTMethod
1215-1275: Validate that all orders belong to the same symbol in batch cancellations.The code collects orders from multiple trading pairs but always uses
exchange_symbols[0]in the API request (lines 1252, 1270). If orders span different symbols (e.g., PERP_BTC_USDC and PERP_ETH_USDC), the Orderly API will reject orders that don't match the provided symbol parameter, causing silent failures.Apply this diff to validate that all orders share the same symbol:
# Get exchange symbols for all trading pairs exchange_symbols = [] for trading_pair in symbols: try: symbol = await self.exchange_symbol_associated_to_pair(trading_pair) exchange_symbols.append(symbol) except Exception as e: self.logger().error( f"[BATCH CANCEL] Error getting exchange symbol for {trading_pair}: {e}" ) + # Validate all orders belong to the same symbol + unique_symbols = list(dict.fromkeys(exchange_symbols)) + if len(unique_symbols) != 1: + raise ValueError( + f"Batch order cancellation requires all orders to belong to the same trading pair. " + f"Found {len(unique_symbols)} different symbols: {unique_symbols}" + ) + symbol_param = unique_symbols[0] + if not exchange_symbols: self.logger().error("[BATCH CANCEL] No valid symbols found") return [ @@ # Build comma-separated order_ids parameter order_ids_str = ",".join(exchange_order_ids) params = { "order_ids": order_ids_str, - "symbol": exchange_symbols[0] # Use first symbol (typically all orders are same symbol) + "symbol": symbol_param, } @@ # Build comma-separated client_order_ids parameter client_order_ids_str = ",".join(client_order_ids) params = { "client_order_ids": client_order_ids_str, - "symbol": exchange_symbols[0] # Use first symbol + "symbol": symbol_param, }scripts/pmm_kodiak.py (3)
67-80: Invoke leverage setup on initialization.
apply_initial_setting(lines 121-125) is never called, so the leverage configuration on line 124 is skipped. The connector will trade at its default leverage instead ofconfig.leverage. Call it at the end of__init__to apply the requested leverage exactly once when the strategy starts.Apply this diff:
# DataFrame to store last 6 filled orders self._filled_orders_df: pd.DataFrame = pd.DataFrame(columns=[ "Timestamp", "Order ID", "Side", "Amount", "Price", "Inventory" ]) + self.apply_initial_setting()
163-164: Validate spread arrays before iterating.Line 164 assumes
ask_spread_levels[idx]exists. If a user provides mismatched lengths, this will raiseIndexErrorand stop the strategy mid-run. Validate the configuration upfront so the strategy fails fast with a clear error instead of crashing during order generation.Apply this diff:
+ if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + orders = [] for idx, bid_spread in enumerate(self.config.bid_spread_levels): ask_spread = self.config.ask_spread_levels[idx]
211-216: Callbudget_checker()before using it.Line 213 stores the bound method
connector.budget_checkerinstead of the checker instance, so line 215'sbudget_checker.adjust_candidates(...)will raiseAttributeErrorat runtime. Invoke the method to obtain the checker before callingadjust_candidates.Apply this diff:
def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: connector = self.connectors[self.config.exchange] - budget_checker = connector.budget_checker + budget_checker = connector.budget_checker() proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) return proposals_adjusted
🧹 Nitpick comments (2)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (1)
1288-1289: Remove unused variableerror_msg.Line 1288 assigns
error_msgbut it's never used (line 1289 usesresponsedirectly).Apply this diff:
if not response.get("success", False): # Check if this is an "order not found/invalid" error - orders don't exist on exchange - error_msg = str(response) error_exception = IOError(f"Batch order cancellation failed: {response}")scripts/pmm_kodiak.py (1)
134-134: Fix comment indentation.The comment has unexpected indentation. While this doesn't affect functionality, it's flagged by the linter.
Apply this diff:
- # Get order book + # Get order book order_book = connector.get_order_book(self.config.trading_pair)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py(1 hunks)scripts/pmm_kodiak.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
scripts/pmm_kodiak.py (7)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (6)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)get(122-125)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (1)
fetch_order(172-184)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (3)
batch_order_create(876-1153)name(96-98)batch_order_cancel(1155-1402)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (6)
hummingbot/core/web_assistant/connections/data_types.py (5)
json(112-125)RESTMethod(14-25)RESTRequest(29-37)url(93-95)method(98-100)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_auth.py (2)
OrderlyPerpetualAuth(39-317)account_id(274-276)hummingbot/core/data_type/in_flight_order.py (1)
InFlightOrder(91-395)hummingbot/core/web_assistant/web_assistants_factory.py (1)
WebAssistantsFactory(14-81)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_web_utils.py (3)
build_api_factory(150-170)format_trading_pair(261-288)is_exchange_information_valid(220-258)hummingbot/core/web_assistant/rest_assistant.py (1)
execute_request(35-59)
🪛 Flake8 (7.3.0)
scripts/pmm_kodiak.py
[error] 134-134: unexpected indentation (comment)
(E116)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
[error] 18-18: 'json' imported but unused
(F401)
[error] 20-20: 'typing.TYPE_CHECKING' imported but unused
(F401)
[error] 36-36: 'hummingbot.connector.utils.combine_to_hb_trading_pair' imported but unused
(F401)
[error] 45-45: 'hummingbot.core.web_assistant.connections.data_types.RESTRequest' imported but unused
(F401)
[error] 1288-1288: local variable 'error_msg' is assigned to but never used
(F841)
🪛 Ruff (0.14.4)
scripts/pmm_kodiak.py
293-293: Do not catch blind exception: Exception
(BLE001)
hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py
62-62: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
63-63: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
64-64: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
191-191: Unused method argument: trading_pair
(ARG002)
195-195: Unused method argument: trading_pair
(ARG002)
239-239: Do not catch blind exception: Exception
(BLE001)
273-273: Do not catch blind exception: Exception
(BLE001)
292-292: Abstract raise to an inner function
(TRY301)
292-292: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Consider moving this statement to an else block
(TRY300)
354-354: Avoid specifying long messages outside the exception class
(TRY003)
387-387: Avoid specifying long messages outside the exception class
(TRY003)
425-425: Do not catch blind exception: Exception
(BLE001)
445-445: Unused method argument: request_exception
(ARG002)
511-511: Unused method argument: kwargs
(ARG002)
624-624: Consider moving this statement to an else block
(TRY300)
626-626: Do not catch blind exception: Exception
(BLE001)
676-676: Unused method argument: amount
(ARG002)
677-677: Unused method argument: trade_type
(ARG002)
678-678: Unused method argument: order_type
(ARG002)
679-679: Unused method argument: price
(ARG002)
681-681: Unused method argument: position_action
(ARG002)
716-716: Unused method argument: amount
(ARG002)
717-717: Unused method argument: trade_type
(ARG002)
718-718: Unused method argument: order_type
(ARG002)
719-719: Unused method argument: price
(ARG002)
721-721: Unused method argument: position_action
(ARG002)
764-764: Unused method argument: kwargs
(ARG002)
819-819: Avoid specifying long messages outside the exception class
(TRY003)
874-874: Avoid specifying long messages outside the exception class
(TRY003)
910-913: Avoid specifying long messages outside the exception class
(TRY003)
963-963: Do not catch blind exception: Exception
(BLE001)
1004-1004: Do not catch blind exception: Exception
(BLE001)
1048-1048: Abstract raise to an inner function
(TRY301)
1048-1048: Avoid specifying long messages outside the exception class
(TRY003)
1134-1134: Consider moving this statement to an else block
(TRY300)
1139-1139: Do not catch blind exception: Exception
(BLE001)
1153-1153: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1153-1153: Avoid specifying long messages outside the exception class
(TRY003)
1182-1185: Avoid specifying long messages outside the exception class
(TRY003)
1221-1221: Do not catch blind exception: Exception
(BLE001)
1288-1288: Local variable error_msg is assigned to but never used
Remove assignment to unused variable error_msg
(F841)
1316-1316: Abstract raise to an inner function
(TRY301)
1316-1316: Avoid specifying long messages outside the exception class
(TRY003)
1395-1395: Consider moving this statement to an else block
(TRY300)
1400-1400: Do not catch blind exception: Exception
(BLE001)
1402-1402: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1402-1402: Avoid specifying long messages outside the exception class
(TRY003)
1437-1437: Avoid specifying long messages outside the exception class
(TRY003)
1522-1522: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
1522-1522: Avoid specifying long messages outside the exception class
(TRY003)
1523-1523: Do not catch blind exception: Exception
(BLE001)
1591-1591: Do not catch blind exception: Exception
(BLE001)
1632-1632: Do not catch blind exception: Exception
(BLE001)
1633-1633: Use explicit conversion flag
Replace with conversion flag
(RUF010)
1646-1646: Unused method argument: trading_pair
(ARG002)
1736-1736: Consider moving this statement to an else block
(TRY300)
1738-1738: Do not catch blind exception: Exception
(BLE001)
1762-1762: Unused method argument: position_action
(ARG002)
1817-1817: Do not catch blind exception: Exception
(BLE001)
1844-1844: Unused method argument: event
(ARG002)
1849-1849: Unused method argument: event
(ARG002)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
scripts/pmm_kodiak.py (1)
364-397: LGTM: Reservation price logic is sound.The inventory-based reservation price adjustment correctly implements the Avellaneda-Stoikov model with a sensible threshold (25%) and linear scaling. For long positions, it lowers the price to encourage selling; for short positions, it raises the price to encourage buying, properly incentivizing mean reversion.
| self._cached_mark_price: Decimal = Decimal("0") | ||
| self._cached_reservation_price: Decimal = Decimal("0") |
There was a problem hiding this comment.
Initialize all cached values to prevent AttributeError.
Only _cached_mark_price and _cached_reservation_price are initialized, but format_status (lines 409-412) also accesses _cached_mid_price, _cached_reservation_price_mark, and _cached_reservation_price_mid. If format_status is called before create_proposal runs, an AttributeError will be raised.
Apply this diff to initialize all cached values:
self._last_update_timestamp: float = 0
self._cached_mark_price: Decimal = Decimal("0")
- self._cached_reservation_price: Decimal = Decimal("0")
+ self._cached_mid_price: Decimal = Decimal("0")
+ self._cached_reservation_price_mark: Decimal = Decimal("0")
+ self._cached_reservation_price_mid: Decimal = Decimal("0")
self._cooldown_until_timestamp: float = 0 # Timestamp until which we're in cooldown📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| self._cached_mark_price: Decimal = Decimal("0") | |
| self._cached_reservation_price: Decimal = Decimal("0") | |
| self._last_update_timestamp: float = 0 | |
| self._cached_mark_price: Decimal = Decimal("0") | |
| self._cached_mid_price: Decimal = Decimal("0") | |
| self._cached_reservation_price_mark: Decimal = Decimal("0") | |
| self._cached_reservation_price_mid: Decimal = Decimal("0") | |
| self._cooldown_until_timestamp: float = 0 # Timestamp until which we're in cooldown |
🤖 Prompt for AI Agents
In scripts/pmm_kodiak.py around lines 72 to 73, only _cached_mark_price and
_cached_reservation_price are initialized, but format_status later reads
_cached_mid_price, _cached_reservation_price_mark, and
_cached_reservation_price_mid which can cause AttributeError if called early;
add initializations for _cached_mid_price, _cached_reservation_price_mark, and
_cached_reservation_price_mid (set each to Decimal("0")) alongside the existing
cached fields so all accessed cache attributes exist before create_proposal
runs.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
scripts/pmm_kodiak.py (4)
78-79: Initialize all cached values to prevent AttributeError.Only
_cached_mark_priceand_cached_reservation_priceare initialized, butformat_status(lines 422-425) also accesses_cached_mid_price,_cached_reservation_price_mark, and_cached_reservation_price_mid. Ifformat_statusis called beforecreate_proposalruns, anAttributeErrorwill be raised.Apply this diff to initialize all cached values:
self._last_update_timestamp: float = 0 self._cached_mark_price: Decimal = Decimal("0") - self._cached_reservation_price: Decimal = Decimal("0") + self._cached_mid_price: Decimal = Decimal("0") + self._cached_reservation_price_mark: Decimal = Decimal("0") + self._cached_reservation_price_mid: Decimal = Decimal("0") self._cooldown_until_timestamp: float = 0 # Timestamp until which we're in cooldown
73-80: Invoke leverage setup on initialization.
apply_initial_setting(lines 127-132) is never invoked, so the leverage configuration at line 130 is skipped and the connector trades at its default leverage, contradicting the script config. Call it during__init__so the requested leverage is applied exactly once when the strategy boots.Apply this diff:
super().__init__(connectors) - # self.current_timestamp = None self.config = config self._last_update_timestamp: float = 0 self._cached_mark_price: Decimal = Decimal("0") self._cached_mid_price: Decimal = Decimal("0") self._cached_reservation_price_mark: Decimal = Decimal("0") self._cached_reservation_price_mid: Decimal = Decimal("0") self._cooldown_until_timestamp: float = 0 # Timestamp until which we're in cooldown # DataFrame to store last 6 filled orders self._filled_orders_df: pd.DataFrame = pd.DataFrame(columns=[ "Timestamp", "Order ID", "Side", "Amount", "Price", "Inventory" ]) + + self.apply_initial_setting()
169-215: Validate spread arrays before iterating.Line 170 assumes
ask_spread_levels[idx]exists; if a user provides mismatched lengths, anIndexErrorwill be raised and the strategy stops mid-run. Guard the configuration (or iterate withzip) so we fail fast with a clear error instead of crashing during order generation.Apply this diff:
orders = [] + if len(self.config.bid_spread_levels) != len(self.config.ask_spread_levels): + raise ValueError("bid_spread_levels and ask_spread_levels must have the same length.") + for idx, bid_spread in enumerate(self.config.bid_spread_levels): ask_spread = self.config.ask_spread_levels[idx]Alternatively, use
zipto iterate both lists safely:orders = [] - for idx, bid_spread in enumerate(self.config.bid_spread_levels): - ask_spread = self.config.ask_spread_levels[idx] + for idx, (bid_spread, ask_spread) in enumerate(zip(self.config.bid_spread_levels, self.config.ask_spread_levels)):
217-222: Callbudget_checker()before using it.Line 219 stores the bound method
connector.budget_checkerinstead of the checker instance, sobudget_checker.adjust_candidates(...)at line 221 raisesAttributeErrorat runtime. Invoke the method to obtain the checker before callingadjust_candidates.Apply this diff:
def adjust_proposal_to_budget(self, proposals: List[PerpetualOrderCandidate]) -> List[PerpetualOrderCandidate]: connector = self.connectors[self.config.exchange] - budget_checker = connector.budget_checker + budget_checker = connector.budget_checker() proposals_adjusted = budget_checker.adjust_candidates(proposals, all_or_none=True) return proposals_adjusted
🧹 Nitpick comments (1)
scripts/pmm_kodiak.py (1)
26-27: Minor: Inconsistent comment spacing.Lines 26, 27, and 31 use
#10 bpswithout a space after#, while standard Python style is# 10 bps.Apply this diff for consistency:
- bid_spread_levels: List[Decimal] = Field(default=[Decimal("0.001")]) #10 bps - ask_spread_levels: List[Decimal] = Field(default=[Decimal("0.001")]) #10 bps + bid_spread_levels: List[Decimal] = Field(default=[Decimal("0.001")]) # 10 bps + ask_spread_levels: List[Decimal] = Field(default=[Decimal("0.001")]) # 10 bps order_refresh_time: int = Field(10) order_cooldown: int = Field(0) # Cooldown in seconds after an order fill max_inventory: Decimal = Field(0.01) # 1k usd - max_price_adjustment: Decimal = Field(default=Decimal("0.001")) # 10 bps + max_price_adjustment: Decimal = Field(default=Decimal("0.001")) # 10 bpsAlso applies to: 31-31
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
scripts/pmm_kodiak.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
scripts/pmm_kodiak.py (6)
hummingbot/client/config/config_data_types.py (1)
BaseClientModel(26-70)hummingbot/core/data_type/common.py (6)
OrderType(8-15)PriceType(50-58)PositionAction(31-34)PositionSide(38-41)TradeType(61-64)get(122-125)hummingbot/core/event/events.py (1)
OrderFilledEvent(161-219)hummingbot/strategy/script_strategy_base.py (1)
ScriptStrategyBase(28-256)hummingbot/connector/client_order_tracker.py (2)
fetch_order(172-184)current_timestamp(122-126)hummingbot/connector/derivative/orderly_perpetual/orderly_perpetual_derivative.py (3)
batch_order_create(876-1153)name(96-98)batch_order_cancel(1155-1402)
🪛 Flake8 (7.3.0)
scripts/pmm_kodiak.py
[error] 140-140: unexpected indentation (comment)
(E116)
🪛 Ruff (0.14.4)
scripts/pmm_kodiak.py
299-299: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (python)
| # best_bid_price = connector.get_price_by_type(self.config.trading_pair, PriceType.BestBid) | ||
| # best_ask_price = connector.get_price_by_type(self.config.trading_pair, PriceType.BestAsk) | ||
|
|
||
| # Get order book |
There was a problem hiding this comment.
Fix comment indentation.
The comment has incorrect indentation (4 spaces instead of 8), which static analysis flagged.
Apply this diff:
- # Get order book
+ # Get order book
order_book = connector.get_order_book(self.config.trading_pair)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Get order book | |
| # Get order book | |
| order_book = connector.get_order_book(self.config.trading_pair) |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 140-140: unexpected indentation (comment)
(E116)
🤖 Prompt for AI Agents
In scripts/pmm_kodiak.py around line 140, the inline comment "# Get order book"
is indented with 4 spaces but should use 8 spaces to match surrounding block
style; update the line to use 8 leading spaces so the comment aligns with
adjacent code and satisfies the linter's indentation rules.
| bid_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same | ||
| ask_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid usedonly to keep the order size same |
There was a problem hiding this comment.
Fix typo in comments.
Line 183 has "usedonly" which should be "used only".
Apply this diff:
- bid_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same
- ask_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid usedonly to keep the order size same
+ bid_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same
+ ask_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bid_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same | |
| ask_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid usedonly to keep the order size same | |
| bid_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same | |
| ask_amount = self.config.order_amount_quote[idx] / reservation_price_mid # reservation_price_mid used only to keep the order size same |
🤖 Prompt for AI Agents
In scripts/pmm_kodiak.py around lines 182 to 183, the inline comment on line 183
contains a typo "usedonly"; update that comment to "used only" so both lines
consistently read "...reservation_price_mid used only to keep the order size
same". Make only the comment text change (no code logic modifications).
…p into feature/orderly
3 participants
Before submitting this PR, please make sure:
A description of the changes proposed in the pull request:
Tests performed by the developer:
Tips for QA testing:
PR-Codex overview
This PR introduces support for the
orderly_perpetualexchange in the Hummingbot connector, implementing mark price functionality and enhancing trading strategies for perpetual futures.Detailed summary
MarkPricetocommon.py.orderly_perpetualexchange.Summary by CodeRabbit
New Features
Bug Fixes
Documentation