Skip to content

feat: perps improve protocol abstraction and handle edge order edge cases#17809

Merged
abretonc7s merged 21 commits into
mainfrom
feat/perps-edge-cases
Aug 6, 2025
Merged

feat: perps improve protocol abstraction and handle edge order edge cases#17809
abretonc7s merged 21 commits into
mainfrom
feat/perps-edge-cases

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Aug 1, 2025

Copy link
Copy Markdown
Contributor

Description

This PR implements critical edge case handling for perpetual trading (JIRA TAT-1280) while
establishing a robust protocol-agnostic architecture. The key achievement is moving all
validation logic from the UI layer to the protocol provider level, ensuring that
protocol-specific rules are properly encapsulated and maintainable.

Key improvements:

  1. Protocol-agnostic architecture: All validation logic now resides at the provider level
    (HyperLiquidProvider), not in UI components
  2. Async validation infrastructure: Made all validation methods async to enable real-time,
    protocol-specific validation
  3. Fixed critical edge cases:
    • Proper USD value calculation from coin amounts (was comparing apples to oranges)
    • Asset-specific leverage limits (e.g., 3x for some assets, 50x for others)
    • Partial position close validation to prevent positions below minimum tradeable size
    • Removed incorrect maximum order size validation (HyperLiquid has no fixed max)

The architecture now supports easy addition of new protocols without touching UI code - just
implement the IPerpsProvider interface.

Changelog

CHANGELOG entry: Fixed perpetual trading edge cases and improved protocol-agnostic validation
architecture to properly enforce minimum order sizes and asset-specific limits

Related issues

Fixes: TAT-1280

Manual testing steps

  1. Navigate to the Perps trading interface
  2. Select any trading pair (e.g., BTC-USD)
  3. Try to place an order below the minimum amount ($10 mainnet, $11 testnet) - should show
    validation error
  4. Try to set leverage above the asset's maximum (varies by asset) - should show validation
    error with correct max
  5. Open a position and try to partially close it
  6. Verify that partial close is prevented if remaining position would be below minimum order
    size
  7. Switch between different assets and verify each has its own leverage limits
  8. Verify all validation messages appear immediately without delay

Screenshots/Recordings

Before

  • Validation logic scattered across UI components
  • Used hardcoded leverage limits (20x for all assets)
  • Minimum order validation incorrectly compared coin amounts to USD values
  • No validation for partial position closes
  • Deprecated validation functions in utils

After

  • All validation logic centralized in protocol providers
  • Asset-specific leverage limits properly enforced
  • Correct USD value calculation for minimum order validation
  • Partial close validation ensures remaining position meets minimums
  • Clean, protocol-agnostic architecture
image

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being
    changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it
    closes and includes the necessary testing evidence such as recordings and or screenshots.

@github-actions

github-actions Bot commented Aug 1, 2025

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@metamaskbot metamaskbot added INVALID-PR-TEMPLATE PR's body doesn't match template and removed INVALID-PR-TEMPLATE PR's body doesn't match template labels Aug 1, 2025
@abretonc7s abretonc7s changed the title feat: perps improve protocol agonis feat: perps improve protocol abstraction and handle edge order edge cases Aug 1, 2025
@abretonc7s abretonc7s marked this pull request as ready for review August 4, 2025 03:31
@abretonc7s abretonc7s requested a review from a team as a code owner August 4, 2025 03:31
@abretonc7s abretonc7s added No QA Needed Apply this label when your PR does not need any QA effort. team-perps Perps team labels Aug 4, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

nickewansmith
nickewansmith previously approved these changes Aug 5, 2025

@nickewansmith nickewansmith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Matt561

Matt561 commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

@abretonc7s Just a quick heads-up that we'll probably want to stop skipping E2E tests if there isn't an active issue with them blocking us. I know Perps is mostly (if not fully self-contained) but the mobile-platform team may dislike that we're skipping them often.

Matt561
Matt561 previously approved these changes Aug 5, 2025
@abretonc7s abretonc7s dismissed stale reviews from Matt561 and nickewansmith via 942ce5e August 6, 2025 01:33
@abretonc7s abretonc7s enabled auto-merge August 6, 2025 01:33
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.90323% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.43%. Comparing base (4bb4d0f) to head (942ce5e).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...Perps/controllers/providers/HyperLiquidProvider.ts 63.79% 12 Missing and 9 partials ⚠️
...s/UI/Perps/Views/PerpsOrderView/PerpsOrderView.tsx 41.37% 16 Missing and 1 partial ⚠️
.../UI/Perps/hooks/usePerpsClosePositionValidation.ts 85.71% 2 Missing and 4 partials ⚠️
...mponents/UI/Perps/hooks/usePerpsOrderValidation.ts 87.87% 2 Missing and 2 partials ⚠️
.../PerpsMarketDetailsView/PerpsMarketDetailsView.tsx 50.00% 0 Missing and 2 partials ⚠️
app/components/UI/Perps/hooks/usePerpsOrderForm.ts 95.34% 0 Missing and 2 partials ⚠️
...omponents/UI/Perps/hooks/usePerpsOrderExecution.ts 97.29% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #17809      +/-   ##
==========================================
+ Coverage   74.38%   74.43%   +0.05%     
==========================================
  Files        2985     2991       +6     
  Lines       67286    67504     +218     
  Branches    11289    11342      +53     
==========================================
+ Hits        50051    50249     +198     
+ Misses      14006    14002       -4     
- Partials     3229     3253      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sonarqubecloud

sonarqubecloud Bot commented Aug 6, 2025

Copy link
Copy Markdown

@nickewansmith nickewansmith left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving after conflict resolution

@abretonc7s abretonc7s added this pull request to the merge queue Aug 6, 2025
Merged via the queue into main with commit ed52dbd Aug 6, 2025
43 of 44 checks passed
@abretonc7s abretonc7s deleted the feat/perps-edge-cases branch August 6, 2025 02:17
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 6, 2025
@metamaskbot metamaskbot added the release-7.55.0 Issue or pull request that will be included in release 7.55.0 label Aug 6, 2025
@tommasini tommasini added release-7.54.0 Issue or pull request that will be included in release 7.54.0 and removed release-7.55.0 Issue or pull request that will be included in release 7.55.0 labels Aug 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

No QA Needed Apply this label when your PR does not need any QA effort. release-7.54.0 Issue or pull request that will be included in release 7.54.0 team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants