Skip to content

feat(perps): align PerpsController with core PR #8633 (isInternal flag)#30413

Merged
abretonc7s merged 3 commits into
mainfrom
feat/perps-controller-parity
May 20, 2026
Merged

feat(perps): align PerpsController with core PR #8633 (isInternal flag)#30413
abretonc7s merged 3 commits into
mainfrom
feat/perps-controller-parity

Conversation

@abretonc7s

@abretonc7s abretonc7s commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

Mirrors the change from MetaMask/core#8633 into mobile ahead of the full migration to core, so the mobile copy of PerpsController stays in sync with the upstream controller. The single substantive change forwards isInternal: true in the options payload when PerpsController submits a transaction via the TransactionController messenger, matching the new core behavior that flags controller-originated transactions as internal.

This keeps both codebases behaviorally identical until mobile retires its local PerpsController in favor of the core package.

Changelog

CHANGELOG entry: null

Related issues

Fixes:

Manual testing steps

Feature: Perps internal transaction flag parity

  Scenario: user submits a perps transaction routed through PerpsController
    Given the Perps feature is enabled and the wallet is unlocked

    When user initiates a perps action that triggers PerpsController to submit a transaction
    Then the resulting TransactionController entry is marked as an internal transaction (isInternal: true)
    And the transaction completes with the same UX as before this change

Screenshots/Recordings

Before

N/A — behavioral parity change, no user-visible UI difference.

After

N/A — behavioral parity change, no user-visible UI difference.

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
  • I've tested with a power user scenario
  • I've instrumented key operations with Sentry traces for production performance metrics

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
Changes the options passed to TransactionController:addTransaction for perps flows, which could affect how transactions are classified/handled downstream (e.g., UI filtering or telemetry). Scope is small and covered by updated unit tests.

Overview
Perps transactions submitted via PerpsController now always forward isInternal: true when calling TransactionController:addTransaction, aligning mobile behavior with the upstream core controller.

Tests for depositWithConfirmation (including the perpsDeposit and perpsDepositAndOrder paths) are updated to assert the new isInternal flag in the messenger call options.

Reviewed by Cursor Bugbot for commit 93a877e. Bugbot is set up for automated code reviews on this repo. Configure here.

@abretonc7s abretonc7s requested a review from a team as a code owner May 20, 2026 00:29
@github-actions

github-actions Bot commented May 20, 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.

@metamaskbotv2 metamaskbotv2 Bot added the team-perps Perps team label May 20, 2026
@abretonc7s abretonc7s enabled auto-merge May 20, 2026 00:30
@github-actions

github-actions Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeConfirmations, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
The change is a single-line modification in PerpsController.ts that adds isInternal: true to the transaction options when submitting perps deposit transactions. The test file is updated accordingly to assert this new flag.

Why these tags:

  1. SmokePerps: Directly affected - the Add Funds (deposit) flow in Perps now passes isInternal: true to the transaction system. This flag could affect how the transaction is processed, displayed, or confirmed. The perps deposit flow must be validated end-to-end.
  2. SmokeConfirmations: Per SmokePerps tag description, "Add Funds deposits are on-chain transactions" so SmokeConfirmations must be included. Additionally, the isInternal flag may affect the confirmation UI (there is a useIsInternalConfirmation hook in the confirmations system, though it checks origin not isInternal directly - the flag may still affect transaction metadata display or routing).
  3. SmokeWalletPlatform: Per SmokePerps tag description, Perps is a section inside the Trending tab, so changes to Perps views affect SmokeWalletPlatform.

Risk level: Medium - The change is small and targeted, but it modifies transaction submission behavior in a financial flow (perps deposits), which warrants careful validation. The isInternal flag could affect transaction routing, confirmation display, or activity history.

Performance Test Selection:
The change is a single-line addition of isInternal: true to transaction options in PerpsController. This is a metadata flag change with no impact on rendering performance, data loading, UI components, or app startup. No performance tests are warranted.

View GitHub Actions results

@abretonc7s

abretonc7s commented May 20, 2026

Copy link
Copy Markdown
Contributor Author
Run Duration Model Nudges Grade Cost
c7480c82 ? opus / claude 1 ungraded $46.4620
Worker report

Comments Report — PR #30413

# Source Author File Triage Action
1 conversation github-actions[bot] — OUT OF SCOPE CLA signature bot — no reply target (issue-level, not review thread)
2 conversation github-actions[bot] — OUT OF SCOPE Smart E2E selection bot — no reply target (issue-level, not review thread)

No inline review comments. No CHANGES_REQUESTED reviews. No human reviewer comments.

CI Failure (Unit tests shard 8) — Fixed

CI run 26133739904 shard 8 had 2 failing assertions in app/controllers/perps/PerpsController.test.ts:

  • depositWithConfirmation › calls TransactionController:addTransaction with prepared transaction (line 2710)
  • depositWithConfirmation › uses addTransaction when placeOrder is true (line 3101)

Both expected the addTransaction options object without the new isInternal: true flag introduced by this PR. Updated both expectations to include isInternal: true. Verified locally via yarn jest app/controllers/perps/PerpsController.test.ts -t "depositWithConfirmation" — 25 passed, 0 failed.

Totals

  • Total comments: 2 (0 REAL, 0 FALSE POSITIVE, 2 OUT OF SCOPE)
  • Test fix commit: 93a877ed95
  • Files changed: app/controllers/perps/PerpsController.test.ts
  • Recipe re-validation: SKIPPED (HAS_RECIPE=no, RECIPE_SOURCE=pr-body-llm-llm-failed)
  • Merge-main status: clean (merge commit 00aafb18cd, no conflicts)

@sonarqubecloud

sonarqubecloud Bot commented May 20, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@abretonc7s abretonc7s added this pull request to the merge queue May 20, 2026
Merged via the queue into main with commit 1919f0f May 20, 2026
237 of 255 checks passed
@abretonc7s abretonc7s deleted the feat/perps-controller-parity branch May 20, 2026 23:30
@github-actions github-actions Bot locked and limited conversation to collaborators May 20, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.79.0 Issue or pull request that will be included in release 7.79.0 label May 20, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@geositta geositta geositta approved these changes

Assignees

No one assigned

Labels

release-7.79.0 Issue or pull request that will be included in release 7.79.0 size-XS team-perps Perps team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@abretonc7s @geositta