fix(gmx-sol): exclude points-farming volume from GMTrade#8085
Open
mkzung wants to merge 2 commits into
Open
Conversation
Contributor
Summary by CodeRabbit
WalkthroughGMX Solana daily volume now comes from paginated ChangesGMX Solana volume calculation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Adapter
participant GMXGraphQL
participant WalletAggregation
participant DailyVolume
Adapter->>GMXGraphQL: Request paginated tradeEvents
GMXGraphQL-->>Adapter: Return daily trade events
Adapter->>WalletAggregation: Apply position and volume changes
WalletAggregation-->>DailyVolume: Return non-farmer wallet volumes
DailyVolume-->>Adapter: Return dailyVolume
Suggested labels: 🚥 Pre-merge checks | ✅ 13 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (13 passed)
✨ Finishing Touches✨ Simplify code
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 |
|
The gmx-sol.ts adapter exports: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dexs/gmx-sol.ts`:
- Line 109: Update the error message in the events-length check to remove the
stray period after the exclamation mark, preserving the existing error condition
and behavior.
- Around line 101-109: Cap the pagination loop in the trade-event retrieval flow
around the events accumulator and request call by enforcing a maximum number of
pages and a timeout for each GraphQL request. Stop or fail clearly when either
limit is reached, while preserving the existing short-page termination and “Not
found daily data!” behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33888a54-c3ba-488f-866a-fdf1ff2c3e4a
📒 Files selected for processing (1)
dexs/gmx-sol.ts
Author
|
Both fair. Added a cap on the pagination loop so it cannot spin if the subgraph keeps handing back full pages, and reworded the error. The daily numbers are unchanged. |
|
The gmx-sol.ts adapter exports: |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
dexs/gmx-sol.ts (1)
99-151: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftAdapter should be
version: 2— the subgraph supports timestamp filtering.The SQD subgraph accepts
from/totimestamp parameters (line 105), confirming it supports granular time filtering. Per coding guidelines, subgraphs should useversion: 2withpullHourlyexplicitly set. The currentversion: 1means the adapter runs once daily and usesoptions.startOfDay(a v1-only time anchor), which the guidelines say v2 adapters should not use.Migrating to v2 would require:
- Replacing
options.startOfDaywithoptions.startTimestamp/options.fromTimestamp- Adding
pullHourly: true(orfalsewith a comment if hourly pulling is impractical for farmer detection)- Adapting the farmer classification logic to work on hourly windows (a wallet with 20 trades/day may only have 1–2 trades/hour, falling below the
trades >= 20threshold)If hourly farmer detection is genuinely impractical,
version: 2withpullHourly: falseand an inline comment explaining why would still be more guideline-compliant thanversion: 1.Based on learnings, version selection should be based on data source capabilities (timestamp filtering), not on query expense. Subgraphs explicitly fall into the v2 category. As per coding guidelines, "Use Version 2 when the source supports granular time filtering, such as on-chain logs, contract calls, subgraphs."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dexs/gmx-sol.ts` around lines 99 - 151, Update the adapter definition that uses fetch to version 2 because the subgraph supports timestamp filtering. Replace the v1 options.startOfDay anchor in fetch with the v2 start/from timestamp field, and explicitly configure pullHourly; if disabling hourly pulls, add the required rationale comment. Adjust isVolumeFarmer-based classification as needed so farmer detection remains correct for the selected window size.Sources: Coding guidelines, Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@dexs/gmx-sol.ts`:
- Around line 99-151: Update the adapter definition that uses fetch to version 2
because the subgraph supports timestamp filtering. Replace the v1
options.startOfDay anchor in fetch with the v2 start/from timestamp field, and
explicitly configure pullHourly; if disabling hourly pulls, add the required
rationale comment. Adjust isVolumeFarmer-based classification as needed so
farmer detection remains correct for the selected window size.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: af8d10ea-c548-4dc6-8176-0d3a8dbdeecb
📒 Files selected for processing (1)
dexs/gmx-sol.ts
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
2 participants
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.
Closes #7120.
Problem
@treeoflife2 you were right to close #7235.
totalFeesmixes price impact, liquidation, borrowing and funding fees, which this repo already documents infees/gmx-sol.ts("Fees from opening/closing perpetual positions + borrowing fees + liquidation fees"), so fees/volume is not a trading fee rate and a threshold on it is fitted to noise.I also tried the rule you suggested, at least 90% of closes held under a minute. It does not work either. On 2026-05-20 it flags 2 wallets and 0.095% of the volume, because only 1.9% of that day's closes were held under a minute. The variable is right, the scale is off: median hold that day is about 1h against about 10h on a quiet day.
So I went and looked at who actually produces the spike. On 2026-05-20 it is 15 wallets, and 13 of them carry 99.8% of it. They are close to identical:
Thirteen wallets with the same capital band, the same market count, the same turnover, a book that is flat overall, and points ranks in one contiguous block. That reads as one operation rather than a population of traders.
One correction to the premise, before you decide what to do about it. They are not paying fees to do this. On that day they are net +0.63 bps profitable after fees and price impact, while everyone else on the platform is -0.60 bps. They are farming the GT points programme and they are good enough at it to trade for free. And since GMX Solana is pool-based rather than an orderbook, none of this is self-matched wash trading. It is real trading against the pool that only exists because of the points programme.
Whether that volume should count is your call. What follows filters it if you want it filtered.
Fix
Per wallet, per day, from
tradeEvents:|long volume - short volume| / volume.Flagged when turnover >= 20 and imbalance <= 0.15.
Turnover has to be measured against total concurrent capital, not the largest single position. These wallets run 15 to 26 markets at once, so keying off the biggest position understates their capital by about that factor and sweeps in ordinary multi-market traders. I had that wrong at first and it roughly doubled the amount removed.
Quiet days stay at 0.0% removed anywhere in turnover 10-30 and imbalance 0.05-0.25, so the exact thresholds are not load bearing.
Fees are untouched. Those fees were really paid.
Verified
Recomputing volume from
tradeEventsreproducesvolumeRecordDailies.tradeVolumeexactly on all 62 days from 2026-05-10 to 2026-07-10, so with nothing flagged this returns exactly what the adapter returns today.Over the 62 days $78.4B becomes $71.2B, and 52 of the 62 days move by less than 2%. The 2026-03-25 and 2026-04-08 rows are the ones I would check first: real $500-675M days with nothing flagged, so this is not just firing whenever volume is high.
It adds about 4s to the adapter on a normal day and 6s on the heaviest one, since it reads the day's trade events (three requests at 50k rows).
Happy to move the thresholds, or to drop this if you would rather not treat incentive volume differently.