Open
Conversation
fa7e554 to
e90008a
Compare
…ility This commit implements proper fee reporting in the GMX CCXT wrapper to enable freqtrade to calculate and display fee rates. Problem: - Freqtrade was logging "rate: None" for GMX trades - The wrapper was reporting execution fees (ETH gas) instead of trading fees - ETH is neither base nor quote currency, so freqtrade couldn't compute rate Solution: - Added _build_trading_fee() helper method - Replace execution fees with trading fees (0.06% in USDC/USD) - Updated all fee construction paths: * parse_trade() - Added rate calculation for Subsquid trades * _parse_order_result_to_ccxt() - Order creation uses trading fees * fetch_order() - Verification replaces with trading fees * _parse_sltp_result_to_ccxt() - Bundled SL/TP uses trading fees * _create_standalone_sltp_order() - Standalone SL/TP uses trading fees - Preserved execution fees in info["execution_fee_eth"] Fee rate uses 0.06% (0.0006) as per GMX docs: - 0.04% for positive impact trades (improve balance) - 0.06% for negative impact trades (common case) - Matches existing calculate_fee() method Result: Before: Fee for Trade ... [sell]: 1.8650141e-05 ETH - rate: None After: Fee for Trade ... [sell]: 0.01680000 USDC - rate: 0.0006 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Apply the same fee rate implementation to the async version: - Added _build_trading_fee() helper method - Updated _parse_sltp_result_to_ccxt() to use trading fees - Updated fetch_order() to replace execution fee with trading fee on verification - Preserved execution fees in info["execution_fee_eth"] Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Extract actual fees from PositionIncrease/PositionDecrease events instead of
using fixed 0.06% rate. Calculate real fee rates based on actual paid amounts.
Changes:
- Add _convert_token_fee_to_usd() to convert token fees to USD
- Add _extract_actual_fee() to get actual fees from verification events
- Add _build_fee_breakdown() for detailed fee reporting
- Update fetch_order() to use actual fees from events
- Add fees_breakdown in order["info"] with:
* position_fee_usd, borrowing_fee_usd, funding_fee_usd
* execution_fee_eth (keeper gas fees)
- Change logging precision from %.2f to %s for full float precision
- Enhanced ORDER_TRACE logs to show:
* trading_fee and rate from actual events
* execution_fee separately
Fee structure remains CCXT-compliant:
- order["fee"] = {cost, currency, rate} with actual values
- Detailed breakdown in order["info"]["fees_breakdown"]
Applied to both sync and async GMX CCXT wrappers.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Replaced string "N/A" fallbacks with numeric 0.0 defaults in fetch_order()
logging statements to prevent potential issues with freqtrade's fee processing.
Using numeric types ensures type safety and CCXT compliance.
Changes:
- order["fee"].get("cost/rate") fallbacks now use 0.0 instead of "N/A"
- execution_fee_eth fallback now uses 0.0 instead of "N/A"
- Applied to both sync and async exchange implementations
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…nIncrease
CRITICAL BUG FIX: GMX V2 emits fees in a separate PositionFeesCollected event,
not in PositionIncrease/PositionDecrease events. The previous implementation
tried to extract positionFeeAmount, borrowingFeeAmount, etc. from position
events where these fields don't exist, resulting in all fees being zero.
Changes:
- Updated extract_order_execution_result() in eth_defi/gmx/events.py
- Added separate loop to find PositionFeesCollected event by orderKey
- Fees now correctly extracted from the right event
Test Results (Transaction 0x8aec04ad...):
- Before: Position Fee = 0, Total = 0
- After: Position Fee = 9709 tokens ($0.00970900 USDC)
Test Results (Transaction 0xb8599635...):
- Before: All fees = 0
- After: Position Fee = $0.00970800, Borrowing = $0.00100600,
Funding = $0.00248100, Total = $0.01319500
This fix is critical for accurate fee reporting to freqtrade and other clients.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
CRITICAL FIX: PnL decimal precision issue causing 5.3x error in freqtrade - order["info"]["verification"]["pnl_usd"] was storing RAW 30-decimal int - Freqtrade likely reading this field, resulting in wrong PnL display - Now converts: pnl_usd / GMX_USD_PRECISION for human-readable USD - Also converts price_impact_usd for consistency USER REQUEST: Print full order IDs in logs - Changed all logger statements from id[:16] to id - Allows seeing complete 66-character order keys for debugging - Applied to both sync and async implementations (38 locations) Test case from screenshots: - GMX Web UI shows: +$0.67 (+4.17%) PnL ✅ - Freqtrade showed: $0.126 (0.80%) PnL ❌ (5.3x error) - Root cause: Reading raw int 670000000000000000000000000000 instead of converted 0.67 - Fix: Now stores 0.67 (converted) in verification block Changes: - eth_defi/gmx/ccxt/exchange.py: - Line 6644: Convert pnl_usd and price_impact_usd to USD - 19 locations: id[:16] → id - eth_defi/gmx/ccxt/async_support/exchange.py: - Line 38: Add GMX_USD_PRECISION import - Line 1586: Convert pnl_usd and price_impact_usd to USD - 19 locations: id[:16] → id Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
After bot restart, fetch_order() reconstructs orders from blockchain and previously set side=None because the CCXT side was not stored on-chain. This caused an infinite fee update loop in freqtrade since update_fee() and fee_updated() require "buy"/"sell" to match. Adds _derive_side_from_trade_action() that maps orderType + isLong back to the original CCXT side. Also extracts orderType from EventEmitter events and adds diagnostic logging. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
timeout_seconds=0 caused the Subsquid query loop to never execute (while 0 < 0 is False), so all blockchain-path fetch_order calls fell through to EventEmitter which lacks orderType/isLong fields. Changed to timeout_seconds=5 and bumped fallback logs to info level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Path B (post-restart blockchain fetch) was reporting only gas cost in ETH as the fee, causing freqtrade to record rate: None (ETH→USDC conversion fails). Now extracts positionFeeAmount from trade_action (Subsquid) and converts to USDC with proper rate, matching Path A's design. Gas cost is preserved in info["execution_fee_eth"] for reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…urate fee conversion When Subsquid lacks collateralToken/collateralTokenPriceMax fields (which it always does), fetch the keeper tx receipt and call extract_order_execution_result() to get real collateral data from PositionFeesCollected events. Remove the broken execution_price fallback that used index token price with collateral token decimals, producing garbage values for non-stablecoin collateral. Changes: - Add collateral enrichment from on-chain events in blockchain fallback (sync + async) - Remove broken execution_price parameter from _convert_token_fee_to_usd - Add extract_order_execution_result import to async exchange - Add dynamic collateral detection (collateral_token, collateral_token_price fields) - Extract fees from PositionFeesCollected event with proper collateral price Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ain PositionFeesCollected Re-enable the tradeActions Subsquid query with account_eq + eventName_eq filters to get real fee breakdown (positionFee, borrowingFee, fundingFee) without 504 timeouts. When tradeActions is unavailable, fall back to on-chain enrichment from PositionFeesCollected events via the keeper execution receipt. - Add tradeActions query with eventName_eq:"OrderExecuted" filter to client.py - Add on-chain fee enrichment (Option B) in _extract_fee_from_trade_action - Create shared _extract_fee_from_trade_action, used by both sync and async exchange - Pass account=self.wallet_address to all Subsquid query call sites - Fix dead backup Subsquid endpoint (@cc00ce -> :prod) in contracts.py - Increase tradeActions query timeout from 10s to 30s - Add detailed TRADING FEE BREAKDOWN INFO logging across all fee paths - Add async get_trade_action_by_order_key to async_graphql.py Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…re exit_reason When GMX keeper cancels or freezes an order, create_order now raises ccxt.InvalidOrder instead of returning a cancelled order dict. This prevents freqtrade from setting exit_reason on the trade before the order is confirmed. Also distinguishes frozen orders (status="expired") from hard cancels in fetch_order, and adds sold_on_exchange hint to synthetic position-already-closed orders. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ecreaseOrderSize Pass position_size_usd_raw (exact 30-decimal int) through the full close path so _format_size_info() uses it as-is, avoiding the lossy int(float * 10^30) round-trip that intermittently causes GMX to revert with InvalidDecreaseOrderSize. Also: - Skip token approval for close/reduce orders (no collateral deposit needed) - Convert raw int to float for gas monitoring log display - Use raw int when partial close clamps to full position size
8ffcf1e to
0827439
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: #718