feat: cancel orders(limit)

[Aviksaikat]

ref: #661

[Aviksaikat]

@claude review this PR

[claude]

Claude finished @Aviksaikat's task in 3m 8s —— View job

---

PR #770 review: feat: cancel orders (limit)

• Gather context and read PR diff
• Review core order cancellation logic
• Review CCXT exchange changes
• Review trading module changes
• Review Freqtrade integration changes
• Review GraphQL client changes
• Review test files
• Review scripts and remaining files
• Post final review

---

Summary

This PR adds cancellation support for pending limit orders (stop loss, take profit, limit increase) on GMX V2. It spans the full stack: low-level transaction building, CCXT adapter integration, Freqtrade exchange class, tests, and example scripts. It also includes several bug fixes (nonce recovery, gas pricing, Subsquid endpoint updates, execution buffer adjustment, liquidation guard).

The architecture is well thought out — CancelOrder builds unsigned transactions, pending_orders.py reads from the DataStore, and the CCXT layer wraps everything in CCXT-compatible dicts. The approach of batching cancels via multicall is gas-efficient and correct.

Issues

1. Duplicate code: cancel_order / cancel_orders logic repeated in sync and async CCXT

eth_defi/gmx/ccxt/exchange.py and eth_defi/gmx/ccxt/async_support/exchange.py contain near-identical implementations of cancel_order(), cancel_orders(), and the tx-hash → order-key resolution logic. This is ~500 lines of copy-pasted code that will be a maintenance burden. Consider extracting the shared logic (order key resolution, validation, result formatting) into a private helper or mixin that both sync and async classes delegate to.

Fix this →

2. _pending_order_to_ccxt uses "stop" instead of "stop_loss" to avoid Telegram parse errors

In eth_defi/gmx/ccxt/exchange.py:3492-3496, the stop_loss type is remapped to "stop" with a comment about Telegram MarkdownV1. This silently diverges from the CCXT standard "stop_loss" type. A better fix would be to escape the underscore at the Telegram notification layer rather than changing the order type at the data model level. This could cause subtle bugs for code checking order.get("type") == "stop_loss".

3. cancel_orders calls config.get_chain() inside the loop

In eth_defi/gmx/ccxt/exchange.py (sync cancel_orders() and async variant), chain = self.config.get_chain() is called inside the for raw_id in ids: loop. This is a minor inefficiency — the chain doesn't change per order. Move it before the loop.

Fix this →

4. Three example scripts with significant overlap

scripts/gmx/cancel_gmx_orders.py (321 lines), gmx_ccxt_limit_order_cancel.py (315 lines), and gmx_limit_order_cancel.py (373 lines) total 1009 lines with substantial overlap. Consider keeping only the most polished one (cancel_gmx_orders.py) as the canonical example and referencing it from the others, or consolidating them.

5. debug_ccxt_cancel_order.py is 630 lines — should it be in the repo?

tests/gmx/debug_ccxt_cancel_order.py is a large debug script, not a pytest test. It's useful for development but adds 630 lines to the test directory. Consider either converting it to proper pytest tests or moving it to scripts/.

6. _send_signed_tx_with_nonce_recovery mutates transaction dict

In eth_defi/gmx/ccxt/exchange.py, _send_signed_tx_with_nonce_recovery() does del transaction["nonce"] on the caller's dict. The callers already do del transaction["nonce"] before calling this helper, but if the nonce recovery path fires, it mutates the same dict again. This is fragile — consider working on a copy.

7. PendingOrder.trigger_price_usd assumes 18-decimal tokens

eth_defi/gmx/order/pending_orders.py:121 hardcodes division by 10**12:

return self.trigger_price / 10**12

This is only correct for 18-decimal index tokens (ETH). For BTC (8 decimals), the divisor should be 10**22. The comment acknowledges this but the code doesn't handle it. Consider accepting token_decimals or documenting the limitation explicitly in the property name (e.g., trigger_price_usd_18dec).

Fix this →

Suggestions (non-blocking)

8. Freqtrade gmx_exchange.py module docstring is outdated

The docstring at lines 20-23 says "No limit orders (all orders execute immediately or revert)" and "No order cancellation (orders execute atomically)". This is no longer true after this PR — limit orders and cancellation are now supported.

Fix this →

9. GraphQL _GMX_ORDER_TYPE_NAMES duplicates OrderType enum

eth_defi/gmx/graphql/client.py defines _GMX_ORDER_TYPE_NAMES as a plain dict mapping ints to strings. This duplicates the OrderType(IntEnum) from eth_defi/gmx/constants.py. Consider using OrderType(value).name.lower() instead.

10. order/__init__.py now has no re-exports

After this PR, eth_defi/gmx/order/__init__.py only contains the module docstring — no re-exports. The old from eth_defi.gmx.order import SLTPEntry, SLTPOrder, SLTPParams import path is broken (though all callers were updated). Consider adding re-exports for the new public types (CancelOrder, PendingOrder, fetch_pending_orders, etc.) for discoverability, or documenting the explicit submodule import pattern.

11. Comment in graphql/client.py mentions "ChatGPT"

eth_defi/gmx/graphql/client.py:29:

# Just Random values ChatGPT gave

This should be cleaned up — either provide real references for the threshold values or remove them if they're placeholders.

What looks good

• Core design: CancelOrder + PendingOrder dataclasses with slots=True follow project conventions. Clean separation between transaction building and signing.
• Gas pricing fix: Using web3.eth.gas_price instead of estimate_gas_fees().max_fee_per_gas for cancel transactions avoids the 100x overestimation on Arbitrum. Well-documented reasoning.
• Batch cancellation: multicall batching with linear gas scaling (CANCEL_ORDER_GAS_LIMIT * N) is efficient and correct.
• Nonce recovery: The _send_signed_tx_with_nonce_recovery helper handles both "nonce too high" and "nonce too low" cases, which addresses a real production issue.
• Test coverage: Lifecycle tests (open → SL → verify → cancel → verify gone) cover the happy path well, and edge cases (empty account, nonexistent order) are tested.
• Execution buffer reduction: DEFAULT_SLTP_EXECUTION_FEE_BUFFER from 3.0 → 1.0 to match GMX UI is a good fix.
• Liquidation guard: Returning 0.0 for negative max_loss in calculate_estimated_liquidation_price prevents nonsensical prices.
• Subsquid position close lookup: Using real Subsquid data instead of synthetic orders when a position was closed by SL is a significant improvement for accuracy.

Verdict

Solid feature addition. The core cancellation implementation is correct and well-tested. The main concerns are code duplication between sync/async CCXT adapters (issue #1), the trigger_price_usd hardcoded decimal assumption (issue #7), and the three overlapping scripts (issue #4). I'd suggest addressing issues #1-3 and #7 before merge.

[claude]

PR Review: feat: cancel orders(limit)

Good work on a substantial feature. The cancel order lifecycle is well thought out with proper CCXT compatibility, batch support, and freqtrade integration. Here is detailed feedback:

---

Strengths

• Well-structured architecture: Clean separation between CancelOrder (transaction builder), PendingOrder (DataStore reader), and the CCXT/freqtrade layers.
• Batch cancellation: The cancel_orders() multicall approach is gas-efficient and well-designed.
• Defensive programming: The duplicate SL guard in create_stoploss(), the nonce recovery helper, and the gas-critical pause mechanism all address real operational edge cases.
• Good test coverage: Integration tests cover the full lifecycle (create → fetch → cancel → verify), empty account, and nonexistent order cases.
• Thorough documentation: Docstrings are detailed with Sphinx-compatible RST format, parameter descriptions, and cross-references.

---

Issues to address

1. Code duplication between sync and async cancel_order / cancel_orders

The async cancel_order() and cancel_orders() in async_support/exchange.py duplicate ~90% of the sync versions in exchange.py. The id-resolution logic, validation, CCXT response construction, and error handling are copied verbatim. Consider extracting the shared logic (id resolution, validation, response building) into private helper methods on a mixin or the sync class, and have the async class call into them with only the async I/O parts differing.

2. _pending_order_to_ccxt: hardcoded price divisor 10**12

In PendingOrder.trigger_price_usd, the conversion divides by 10**12, which is correct only for 18-decimal index tokens. For tokens with different decimals (e.g., WBTC with 8 decimals), this would produce wrong prices. Consider accepting token decimals or using the PRECISION constant more explicitly:

# Currently:
return self.trigger_price / 10**12
# Should be:
return self.trigger_price / 10**(PRECISION - token_decimals)

At minimum, document this limitation clearly.

3. _pending_order_to_ccxt maps stop_loss → "stop" but test_ccxt_fetch_orders_after_sl_creation asserts type == "stop_loss"

In _pending_order_to_ccxt (exchange.py line ~3497), stop-loss orders are mapped to order_type = "stop" (with a comment about Telegram MarkdownV1). But the test test_ccxt_fetch_orders_after_sl_creation asserts sl.get("type") == "stop_loss". The _pending_order_to_ccxt returns "stop" not "stop_loss", so this assertion should fail. Please reconcile — either the mapping or the test assertion needs updating.

4. Deprecated asyncio.get_event_loop() usage

In async_support/exchange.py, cancel_order() and cancel_orders() use asyncio.get_event_loop() which is deprecated since Python 3.10. Use asyncio.get_running_loop() instead:

# Before:
loop = asyncio.get_event_loop()
# After:
loop = asyncio.get_running_loop()

5. cancel_orders validates each order sequentially with RPC calls

In both cancel_order() and cancel_orders(), each order key is validated via is_order_pending() which makes an RPC call. For batch cancellation of N orders, this means N sequential RPC calls before the batch transaction is even built. Consider:

• Batching the validation into a single multicall read, or
• At minimum, document that batch cancel has O(N) RPC overhead for validation.

6. _send_signed_tx_with_nonce_recovery mutates the input transaction dict

The helper deletes transaction["nonce"] on the retry path, mutating the caller's dict. While the callers currently use dict(result.transaction) copies, this is fragile. Consider making the copy inside the helper:

def _send_signed_tx_with_nonce_recovery(self, transaction, signed_tx, context="transaction"):
    # ...on retry:
    retry_tx = {k: v for k, v in transaction.items() if k != "nonce"}
    signed_tx = self.wallet.sign_transaction_with_new_nonce(retry_tx)

7. order/__init__.py — breaking change removing re-exports

The __init__.py of eth_defi/gmx/order/ previously re-exported all order classes (IncreaseOrder, DecreaseOrder, SLTPOrder, OrderResult, etc.) and now exports nothing. Any downstream code doing from eth_defi.gmx.order import OrderResult will break. The fix in order.py (from eth_defi.gmx.order.base_order import OrderResult) and lagoon/test_gmx_lagoon_integration.py confirm this is intentional, but:

• Are there other consumers not updated? A grep for from eth_defi.gmx.order import across the codebase would be prudent.
• Consider a deprecation path — keep the re-exports with a deprecation warning for one release cycle.

8. execution_buffer parameter inconsistency in async vs sync cancel_order

The async cancel_order reads execution_buffer from params or self.options, while the sync version uses self.execution_buffer. These should be consistent.

9. time.sleep() in get_latest_position_close polling loop

The get_latest_position_close method uses time.sleep(poll_interval) in a polling loop. Since this is called from the sync CCXT exchange, it blocks the thread for up to max_wait_seconds (default 10s). This is acceptable for the sync path but should be documented as blocking.

10. Test test_ccxt_cancel_order_lifecycle depends on type == "stop_loss" from _pending_order_to_ccxt

As noted in point 3, the test at line 3956 (assert sl.get("type") == "stop_loss") appears inconsistent with the "stop" mapping in _pending_order_to_ccxt. The debug script debug_ccxt_cancel_order.py line 4886 also asserts type == "stop_loss". These should all be aligned.

---

Minor / style

• UK English: The code consistently uses "cancelled" (UK spelling) — good.
• Emoji in log strings: Lines like "⛽ Gas pause active for %s" and "🚫 Gas critical" use emoji in logger calls. While this aids readability in Telegram alerts, it is unusual in structured log output. Consider keeping emoji only in the Telegram message strings.
• _strip_json_comments in scripts: The regex ^\s*//.*$ only strips full-line comments. Inline trailing comments like "key": "value", // comment are not handled. This is fine given the documented scope but worth a brief comment noting the limitation.
• cancel_gmx_orders.py uses rich as a hard dependency — consider guarding the import since rich is not in the main project dependencies.
• Decimal import removed from trading.py: Verify it is not used elsewhere in the file.

---

Summary

The core cancel order feature is solid and well-tested. The main concerns are:

1. Type inconsistency between _pending_order_to_ccxt ("stop") and test assertions ("stop_loss") — this looks like a bug.
2. Breaking change in order/__init__.py re-exports — needs verification that all consumers are updated.
3. Code duplication between sync/async cancel implementations — could be reduced.

The rest are improvements that can be addressed in follow-up PRs. Good work overall.

[Aviksaikat]

@claude review now

[claude]

Claude finished @Aviksaikat's task in 4m 48s —— View job

---

PR #770 review: feat: cancel orders (limit)

• Gather context and read PR diff
• Review core order modules (cancel_order.py, pending_orders.py)
• Review CCXT exchange changes (sync and async)
• Review trading module and freqtrade integration
• Review GraphQL client changes
• Review tests and scripts
• Post final review

---

Summary

This PR adds cancellation support for pending limit orders (stop loss, take profit, limit increase) on GMX V2 across the full stack: low-level transaction building, CCXT adapter, Freqtrade exchange class, GraphQL client, tests, and example scripts. It also bundles several important fixes: nonce recovery, gas pricing, execution buffer correction, Subsquid endpoint swap, and a liquidation guard.

The core architecture is solid — CancelOrder builds unsigned transactions, PendingOrder reads from the DataStore, the CCXT layer wraps everything in CCXT-compatible dicts, and multicall batching is gas-efficient and correct. The stoploss_adjust() change enabling trailing stop-losses is a significant functional improvement.

---

Issues

1. _send_signed_tx_with_nonce_recovery mutates the caller's transaction dict

eth_defi/gmx/ccxt/exchange.py:6607-6609 — The helper does del transaction["nonce"] on the retry path, mutating the caller's dict. Although callers currently pass a dict(result.transaction) copy, this is fragile — future callers may not. The helper should work on its own copy:

retry_tx = {k: v for k, v in transaction.items() if k != "nonce"}
signed_tx = self.wallet.sign_transaction_with_new_nonce(retry_tx)

Fix this →

2. cancel_orders calls config.get_chain() inside the loop

eth_defi/gmx/ccxt/exchange.py:6809 and eth_defi/gmx/ccxt/async_support/exchange.py:1429 — chain = self.config.get_chain() is called inside the for raw_id in ids: loop. The chain never changes between iterations; move it before the loop.

Fix this →

3. trigger_price_usd hardcodes 18-decimal assumption

eth_defi/gmx/order/pending_orders.py:121 — self.trigger_price / 10**12 is only correct for 18-decimal index tokens (ETH, most alts). For tokens like WBTC (8 decimals), the divisor should be 10**22. The docstring acknowledges this but the property silently returns wrong values for non-18-decimal tokens. Consider either:

• Accepting token_decimals in the constructor and using 10**(PRECISION - token_decimals), or
• Renaming to trigger_price_usd_18dec to make the assumption explicit.

Fix this →

4. Deprecated asyncio.get_event_loop() usage

eth_defi/gmx/ccxt/async_support/exchange.py:1320,1457 — asyncio.get_event_loop() is deprecated since Python 3.10. Use asyncio.get_running_loop() instead, which is the correct call within an async function:

# Before:
loop = asyncio.get_event_loop()
# After:
loop = asyncio.get_running_loop()

Fix this →

5. Code duplication between sync and async cancel implementations

The async cancel_order() and cancel_orders() in async_support/exchange.py duplicate ~90% of the sync versions — id resolution, validation, response building, error handling are near-identical (~500 lines duplicated). The only async-specific parts are the run_in_executor calls. Consider extracting shared logic (id resolution, validation, CCXT response construction) into a common mixin or private helper module.

6. order/__init__.py now exports nothing — potential downstream breakage

eth_defi/gmx/order/__init__.py previously re-exported IncreaseOrder, DecreaseOrder, SLTPOrder, OrderResult, etc. It now only has a module docstring. I verified that from eth_defi.gmx.order import is only used in a commented-out test line (tests/gmx/test_order.py:11), so internal consumers appear updated. But any external code using from eth_defi.gmx.order import OrderResult will break. Consider adding re-exports for the most common types (CancelOrder, PendingOrder, fetch_pending_orders) for discoverability.

7. Three overlapping example scripts

scripts/gmx/cancel_gmx_orders.py (321 lines), gmx_ccxt_limit_order_cancel.py (315 lines), and gmx_limit_order_cancel.py (373 lines) total 1009 lines with significant overlap. Consider keeping only the most polished one (cancel_gmx_orders.py) and removing or substantially trimming the others.

---

Suggestions (non-blocking)

8. Freqtrade module docstring is outdated

eth_defi/gmx/freqtrade/gmx_exchange.py:20-21 still says "No limit orders (all orders execute immediately or revert)" and "No order cancellation (orders execute atomically)". This PR makes both statements false.

Fix this →

9. _GMX_ORDER_TYPE_NAMES duplicates OrderType enum

eth_defi/gmx/graphql/client.py:979-988 defines a plain dict mapping ints to strings that mirrors OrderType(IntEnum) from eth_defi/gmx/constants.py. Consider using OrderType(value).name.lower() to avoid maintaining parallel definitions.

10. Comment references "ChatGPT"

eth_defi/gmx/graphql/client.py:28: # Just Random values ChatGPT gave — should be cleaned up with a proper reference or removed if the values are placeholders.

11. cancel_orders validates each order with sequential RPC calls

In both sync and async cancel_orders(), each order key is validated via is_order_pending() which makes an individual RPC call. For a batch of N orders, this means N sequential RPC calls before the batch transaction is even built. For large batches, consider batching validation into a single multicall read.

12. debug_ccxt_cancel_order.py is 630 lines — belongs in scripts/

tests/gmx/debug_ccxt_cancel_order.py is a development/debug script, not a pytest test. Consider moving it to scripts/gmx/ to keep the test directory focused on automated tests.

---

What looks good

• Core design: CancelOrder + PendingOrder dataclasses with slots=True follow project conventions. Clean separation between transaction building and signing.
• Gas pricing fix: Using web3.eth.gas_price instead of estimate_gas_fees().max_fee_per_gas for both cancel and SL/TP transactions avoids the 100x overestimation on Arbitrum. Well-documented reasoning in both cancel_order.py:258-262 and sltp_order.py.
• DEFAULT_SLTP_EXECUTION_FEE_BUFFER 3.0 → 1.0: Aligns with GMX UI behaviour — justified change.
• Batch cancellation: multicall batching with linear gas scaling is efficient and correct.
• Nonce recovery: Handles both "nonce too high" and "nonce too low" cases with appropriate use of "latest" vs "pending" counts.
• stoploss_adjust(): Now properly enables trailing stop-loss by returning True when the new SL tightens the stop. Clean implementation with clear direction logic.
• Duplicate SL guard in create_stoploss(): Prevents wasting ETH on duplicate keeper execution fees.
• Gas-critical pause mechanism: Exponential backoff with Telegram alerts is a well-designed production safety feature.
• Telegram utils extraction: Clean, reusable, non-fatal wrapper.
• Test coverage: Lifecycle tests (open → SL → verify → cancel → verify gone) cover the happy path; edge cases (empty account, nonexistent order) are tested.
• Liquidation guard: Returning 0.0 for negative max_loss prevents nonsensical prices.
• "stopLoss" type: Using camelCase consistently throughout tests and code avoids the Telegram MarkdownV1 underscore-as-italic issue. Tests assert type == "stopLoss" which matches _pending_order_to_ccxt.

Verdict

Solid feature addition with good test coverage and thoughtful production hardening. The main items I'd address before merge are:

1. Issue This Front Run Bot working good #2 (get_chain() inside loop) — trivial fix
2. Issue Investigate Uniswap v3 and getting historical price data in good format #4 (deprecated asyncio.get_event_loop()) — trivial fix
3. Issue Uniswap v3: price #8 (outdated docstring) — trivial fix

Issues #1, #3, #5, and #6 are worth tracking but can be addressed in follow-up PRs.

---

View job

[claude]

Code review: feat: cancel orders(limit) (#770)

Overall this is a substantial feature addition (~4300 lines added) that implements GMX limit order cancellation across the core order module, CCXT wrappers (sync/async), freqtrade integration, a new GraphQL/Subsquid client, and several scripts/tests. Below are the findings.

---

Potential bugs

1. is_long logic may be inverted in position close detection
In eth_defi/gmx/ccxt/exchange.py, the Subsquid close-detection path has:

is_long = side != "sell"  # we were trying to close a long (sell side)

The comment says "we were trying to close a long (sell side)" — but that logic sets is_long = True when side != "sell". If we are selling to close a long, then the original position was long, so is_long should be side == "sell". The comment and code are contradictory. Please verify which is correct.

2. asyncio.get_event_loop() deprecated
In eth_defi/gmx/ccxt/async_support/exchange.py, cancel_order and cancel_orders use:

loop = asyncio.get_event_loop()

This is deprecated since Python 3.10 and raises DeprecationWarning in some contexts. Since these are async methods with a running loop, use asyncio.get_running_loop() instead.

3. Timestamp inconsistency in cancel_order return dict
In eth_defi/gmx/ccxt/exchange.py, the sync cancel_order calls self.milliseconds() twice:

"timestamp": self.milliseconds(),
"datetime": self.iso8601(self.milliseconds()),

These could yield slightly different values. Capture once in a local variable (as is already done correctly in cancel_orders).

4. trigger_price_usd assumes 18-decimal tokens
In eth_defi/gmx/order/pending_orders.py:

return self.trigger_price / 10**12

The docstring acknowledges this is for "standard 18-decimal index tokens", but for BTC (8 decimals) the divisor should be 10**22. Since PendingOrder doesn't store the index token's decimal count, this will silently produce incorrect prices for non-18-decimal tokens. Consider either storing the decimal count or adding a more prominent warning.

5. Race condition in nonce recovery
_send_signed_tx_with_nonce_recovery() directly sets self.wallet.current_nonce. In a threaded environment (which the project prefers), two concurrent calls detecting nonce errors could both reset to the same on-chain nonce and sign with the same value, causing one to fail again. Consider adding a lock around nonce recovery.

---

Code quality

6. Significant duplication between sync and async exchange
cancel_order() and cancel_orders() are copy-pasted (~260 lines each) between exchange.py and async_support/exchange.py. A shared mixin or helper would reduce the maintenance burden and risk of the two drifting apart.

7. Inconsistent execution_buffer handling
The async cancel_order reads execution_buffer from params:

execution_buffer = params.get("execution_buffer", self.options.get("executionBuffer", 2.2))

The sync version uses self.execution_buffer directly, ignoring params. These should be consistent.

8. Inconsistent order_type naming in _pending_order_to_ccxt

order_type = "stopLoss"      # camelCase
order_type = "take_profit"   # snake_case

The comment explains camelCase for stop-loss is to avoid Telegram MarkdownV1 escaping issues, but this inconsistency forces downstream code to check for both "stopLoss" and "stop_loss" variants (which it already does). Handle the Telegram escaping at the Telegram output layer instead.

9. __init__.py removal is a breaking change
eth_defi/gmx/order/__init__.py had all re-exports removed. Any external consumer importing from eth_defi.gmx.order directly will break. This should be documented as a breaking change.

10. chain = self.config.get_chain() called inside loop
In both cancel_orders() methods, self.config.get_chain() is called on every iteration but always returns the same value. Move it before the loop.

11. Duplicated utility functions across scripts
_extract_order_keys() and _fetch_eth_spot_price() are duplicated across gmx_ccxt_limit_order_cancel.py and gmx_limit_order_cancel.py. Factor these into a shared utility module.

---

Security

12. Telegram token could leak in error logs
In eth_defi/gmx/freqtrade/telegram_utils.py, if the urllib request fails, the exception message may include the full URL containing the bot token. The except Exception as err: logger.warning(...) handler could log it. Sanitise the URL in the error handler.

13. Direct nonce manipulation bypasses wallet safeguards
_send_signed_tx_with_nonce_recovery sets self.wallet.current_nonce directly, bypassing sync_nonce(). The comment acknowledges this, but if sync_nonce gains additional side effects in future, this could be dangerous. Consider using the wallet's public API.

---

Performance

14. _market_address_to_symbol is O(n) per call
This linear scan over all markets is called for every pending order in _pending_order_to_ccxt. Build a reverse-lookup dict ({market_token_lower: symbol}) during load_markets() instead.

15. is_order_pending called per order in batch cancel
Each order in cancel_orders() makes a separate RPC call to is_order_pending(). For large batches, consider fetching all pending order keys once and checking membership locally.

---

Test coverage gaps

16. No tests for async cancel_order/cancel_orders — Given the duplicated code, bugs in one version could easily exist in the other undetected.

17. No tests for nonce recovery logic — _send_signed_tx_with_nonce_recovery directly manipulates wallet state with different behaviour for "nonce too high" vs "nonce too low". This critical code path needs targeted tests.

18. No tests for gas-critical throttling — The sliding window, pause mechanism, and Telegram alerting in gmx_exchange.py are complex state machine logic that should have tests.

19. No tests for get_latest_position_close — The Subsquid polling and response parsing are untested.

20. No tests for stoploss_adjust new logic — The old implementation always returned False; the new directional logic (long vs short) should have unit tests.

21. Test removed without replacement — test_fetch_ticker_raise_exception_action was removed from test_ccxt_price_sanity.py. If the underlying functionality is still supported, its coverage should be preserved.

---

Style issues (per CLAUDE.md conventions)

22. Missing type hints

• _send_signed_tx_with_nonce_recovery: signed_tx parameter has no type hint (should be SignedTransaction)
• _market_address_to_symbol: market_address should use HexAddress not str
• get_latest_position_close: account and market_address should use HexAddress

23. Emojis in logger calls
gmx_exchange.py uses emojis in logger.debug(), logger.info(), and logger.error() calls. Per project conventions, emojis should only be used when explicitly requested. Move emoji usage to the Telegram message layer only.

24. cancel_gmx_orders.py uses sys.argv instead of environment variable
Per project conventions, command-line scripts should "always use environment variables" rather than command-line argument parsers.

25. DEFAULT_SLTP_EXECUTION_FEE_BUFFER changed from 3.0 to 1.0
This is a significant behavioural change that will affect existing users. Should be documented as a potential breaking change and ideally mentioned in the changelog.

---

Summary

The core cancellation logic looks solid and well-structured. The main concerns are:

1. The possible is_long inversion bug (finding Figure out API doc issues #1) needs immediate verification
2. The sync/async code duplication creates maintenance risk
3. Several new code paths lack test coverage (nonce recovery, gas throttling, async cancel, Subsquid polling)
4. A few style/convention issues per project standards

Nice work on the comprehensive implementation — the cancel order flow, pending order fetching, and freqtrade integration are well thought out. The test fixtures for Anvil fork-based testing are well designed.