Skip to content

refactor(perps): address core PR #7941 review and cleanup#27035

Merged
abretonc7s merged 14 commits into
mainfrom
feat/perps/controller-cleanup
Mar 6, 2026
Merged

refactor(perps): address core PR #7941 review and cleanup#27035
abretonc7s merged 14 commits into
mainfrom
feat/perps/controller-cleanup

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Description

Addresses all actionable review comments from the Core perps-controller PR (MetaMask/core#7941).

Bug fixes:

  • Fix cancellation detection inconsistency — add 'User ' prefix to match the pattern used elsewhere
  • Fix memoization regression — pass timestamp as selector input so re-renders fire correctly
  • Resolve circular dependency in perps-types.ts
  • Extract hardcoded withdrawal fee and fee estimate to named constants

Refactors (per core review feedback):

  • Replace 5 barrel export * wildcards with explicit named exports in index.ts
  • Extract 34 method action types to PerpsController-method-action-types.ts
  • Extract inline provider instantiation into private #createProviders() factory
  • Move registerMethodActionHandlers from constructor to initialize() with an idempotency guard
  • Extract magic numbers to config constants (DEPOSIT_GAS_LIMIT, SPOT_ASSET_ID_OFFSET, MYX_TRADING_DEFAULTS, MYX_FEE_CONFIG)
  • Add fetchHistoricalCandles to PerpsProvider interface (removes a type cast in PerpsController)

Changelog

CHANGELOG entry: null

Related issues

Fixes:

Manual testing steps

Feature: Perps controller cleanup

  Scenario: user opens Perps and trades normally
    Given the app is running with Perps enabled
    When user navigates to Perps, connects, and places an order
    Then all perps features work as before — no regressions

Screenshots/Recordings

Before

After

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.

Note

Medium Risk
Touches Perps controller initialization/messenger wiring and public exports, which could cause runtime or build-time breakages if any consumers rely on previous timing or wildcard exports. Behavior changes are mostly bounded (constants, cancellation detection, candle fetching interface), but span multiple core perps modules.

Overview
Perps controller refactor and API cleanup. Extracts PerpsController method action type definitions into PerpsController-method-action-types.ts, moves registerMethodActionHandlers from the constructor into init() with an idempotency guard, and factors provider instantiation/active-provider selection into a private #createProviders().

Provider/API surface updates. Replaces export * barrels in controllers/perps/index.ts with explicit named exports, adds optional fetchHistoricalCandles to the PerpsProvider interface and implements/tests it in HyperLiquidProvider, and removes a circular dependency by relocating OrderType into types/perps-types.ts.

Bug fixes and config/constants. Updates deposit cancellation detection strings to match User cancelled/canceled, fixes a selector memoization ordering regression around timestamp, drops unsupported hex-wei handling in convertPerpsAmountToUSD (now returns '$0'), and centralizes fee/gas/ID constants (e.g., DEPOSIT_CONFIG.EstimatedGasLimit, WITHDRAWAL_CONSTANTS.DefaultFeeAmount, ESTIMATED_FEE_RATE, SPOT_ASSET_ID_OFFSET).

Written by Cursor Bugbot for commit 29b3e69. This will update automatically on new commits. Configure here.

@github-actions

github-actions Bot commented Mar 5, 2026

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 the team-perps Perps team label Mar 5, 2026
@abretonc7s abretonc7s marked this pull request as ready for review March 5, 2026 04:05
@abretonc7s abretonc7s requested a review from a team as a code owner March 5, 2026 04:05
Comment thread app/controllers/perps/services/DepositService.ts
Comment thread app/controllers/perps/selectors.ts Outdated
Comment thread app/controllers/perps/PerpsController.ts
- Restore EstimatedGasLimit to 100000 (was incorrectly set to 150000
  during constant extraction in prior refactor)
- Move Date.now() back into createSelector result function body to
  restore memoization; default param in input selector fired on every
  call, defeating the cache
- Add fetchHistoricalCandles tests for HyperLiquidProvider (95% new
  code coverage)
Comment thread app/components/UI/Perps/utils/amountConversion.ts Outdated
Perps deposits use ERC-20 USDC so totalFiat is always populated; the
0x fallback path was unreachable and broken (ethPriceUSD was never
passed by the caller, making it always return $0). Remove the BN
import and ethPriceUSD parameter; update tests to expect '$0' for any
hex wei input.
@abretonc7s

Copy link
Copy Markdown
Contributor Author

@metamaskbot update-mobile-fixture

@abretonc7s abretonc7s enabled auto-merge March 5, 2026 05:03

@cursor cursor Bot 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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Comment thread app/controllers/perps/PerpsController.ts
@abretonc7s abretonc7s changed the title refactor(perps): address core PR #7941 review — cleanup and correctness fixes refactor(perps): address core PR #7941 review and cleanup Mar 5, 2026
@github-actions

github-actions Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeWalletPlatform, SmokeConfirmations
  • Selected Performance tags: @PerformancePreps
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR makes significant changes to the PerpsController and related perps infrastructure:

  1. PerpsController changes (CRITICAL):

    • Refactored method action handler registration from constructor to init() method with a guard flag
    • Extracted provider creation logic into a new #createProviders() method
    • Changed error message detection for cancellation flows
    • These changes affect controller initialization and could impact perps functionality
  2. Provider interface changes:

    • Added fetchHistoricalCandles() method to PerpsProvider interface
    • Updated HyperLiquidProvider to implement this method
    • Refactored MarketDataService to use the new provider method directly
  3. Constants and configuration changes:

    • Reduced gas limit from 150000 to 100000 for deposits
    • Added new constants (ESTIMATED_FEE_RATE, SPOT_ASSET_ID_OFFSET)
    • Replaced hardcoded values with constants throughout services
  4. Export refactoring:

    • Changed from export * to explicit named exports in index.ts and types
    • This is a significant change that could affect imports
  5. amountConversion.ts:

    • Removed hex wei conversion support (returns '$0' for hex values)
    • Removed bnjs4 dependency

Tag Selection Rationale:

  • SmokePerps: Primary tag - all changes are in the perps controller and related services. Tests Add Funds flow, balance verification, and perps interface functionality.
  • SmokeWalletPlatform: Required per tag description when selecting SmokePerps (Perps is a section inside Trending tab). Also covers activity display which may be affected by transaction changes.
  • SmokeConfirmations: Required per tag description when selecting SmokePerps (Add Funds deposits are on-chain transactions). The gas limit change (150000 → 100000) could affect deposit transaction confirmations.

The changes are isolated to the perps domain (Phase 6 controller) with no direct impact on core Engine initialization or other controllers. The risk is medium due to the controller initialization refactoring and gas limit changes.

Performance Test Selection:
The changes directly affect the PerpsController initialization, provider creation, and trading services. The refactoring of handler registration from constructor to init() method, the gas limit reduction for deposits, and the new fetchHistoricalCandles method could all impact perps performance. The @PerformancePreps tag covers perps market loading, position management, add funds flow, and order execution - all areas touched by these changes.

View GitHub Actions results

@github-actions

github-actions Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ E2E Fixture Validation — Structural changes detected

Category Count
New keys 68
Missing keys 11
Type mismatches 0
Value mismatches 7 (informational)

The committed fixture schema is out of date. To update, comment:

@metamaskbot update-mobile-fixture

View full details | Download diff report

@sonarqubecloud

sonarqubecloud Bot commented Mar 5, 2026

Copy link
Copy Markdown

@abretonc7s abretonc7s added this pull request to the merge queue Mar 6, 2026
Merged via the queue into main with commit 29d3005 Mar 6, 2026
98 of 99 checks passed
@abretonc7s abretonc7s deleted the feat/perps/controller-cleanup branch March 6, 2026 00:50
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 6, 2026
@metamaskbot metamaskbot added the release-7.70.0 Issue or pull request that will be included in release 7.70.0 label Mar 6, 2026
@abretonc7s abretonc7s changed the title refactor(perps): address core PR #7941 review and cleanup refactor(perps): address core PR #7941 review and cleanup cp-7.69.0 Mar 10, 2026
@abretonc7s abretonc7s changed the title refactor(perps): address core PR #7941 review and cleanup cp-7.69.0 refactor(perps): address core PR #7941 review and cleanup Mar 10, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.70.0 Issue or pull request that will be included in release 7.70.0 size-XL team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants