fix(perps): update perps controller 3.1.1 cp-13.28.0#41558
Merged
Conversation
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. |
Contributor
Builds ready [4c47afc]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
Contributor
✨ Files requiring CODEOWNER review ✨🕵️ @MetaMask/extension-privacy-reviewers (1 files, +1 -1)
👨🔧 @MetaMask/perps (2 files, +219 -27)
📜 @MetaMask/policy-reviewers (8 files, +8 -0)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🧪 @MetaMask/qa (1 files, +1 -1)
|
Contributor
Author
Automated fix-bug run — TAT-2699
Worker reportFix Report: TAT-2699 — Decimal logic issuesSummaryPerps UI screens displayed prices with hardcoded 2-decimal formatting instead of the adaptive formatting used on mobile. Entry prices, liquidation prices, TP/SL prices, funding payments, and token quantities were either missing thousands separators or showing the wrong number of decimal places. Root Cause
Data flow: position price fields (entryPrice, liquidationPrice, takeProfitPrice, stopLossPrice, cumulativeFunding.sinceOpen) arrive as raw API strings like Changes
Test PlanAutomated:
Manual Gherkin: Evidence
Ticket |
Contributor
Builds ready [ef7ff31]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs
|
aganglada
reviewed
Apr 15, 2026
aganglada
left a comment
Contributor
There was a problem hiding this comment.
@abretonc7s would you mind fixing the sonar issues?
They are highlighted in the files
This was referenced Apr 15, 2026
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
Collaborator
|
Policies updated. Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 👀 lavamoat/browserify/beta/policy.json changes differ from main/policy.json policy changes |
Contributor
Builds ready [f23eaa5]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Address the CI/performance review feedback by removing the broad Jest transformIgnorePatterns exceptions and isolating the affected infrastructure test behind a local @metamask/perps-controller mock. Constraint: Keep the branch diff minimal and avoid paying a repo-wide CI performance cost for a perps-specific test need Rejected: Keep the node_modules transform allowlists in unit/integration Jest config | slows extension CI and broadens the maintenance surface for little value Confidence: high Scope-risk: narrow Directive: Prefer local test mocks over global Jest transform exceptions when only a small number of tests need package isolation Tested: targeted Jest for perps infrastructure/init; ESLint on touched files; yarn lint:tsc; yarn circular-deps:check Not-tested: Full CI rerun after push
…ogic' into fix/tat-2699-controller-decimal-base
Remove the integration-specific node_modules transform allowlist now that the affected path is covered by local package mocking and the representative integration path no longer needs the broader transform exception. Constraint: Keep the change scoped to the integration Jest config only Rejected: Keep the transitive ESM allowlist in integration Jest | adds CI cost and maintenance overhead for little value Confidence: high Scope-risk: narrow Directive: If a future integration failure needs perps-controller isolation, prefer targeted mocking over broad transformIgnorePatterns exceptions Tested: local integration webpack build; representative integration Jest path; targeted non-UI Jest; yarn lint:tsc; yarn circular-deps:check Not-tested: Full branch CI rerun after push
…ogic' into fix/tat-2699-controller-decimal-base
Contributor
Builds ready [0472ef0]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Update the perps disk cache adapter so in-memory state changes only after StorageService writes/removes succeed, and add regression tests for rejected persistence calls. Constraint: Keep the fix scoped to the cache consistency bug reported on the controller branch Rejected: Keep optimistic memory updates without rollback | can return stale in-memory state after failed persistence and mislead the current session Confidence: high Scope-risk: narrow Directive: When the disk cache has both memory and persistent layers, treat the persistent operation as the source of truth for mutation ordering Tested: Jest app/scripts/controllers/perps/infrastructure.test.ts --no-coverage; ESLint on touched files Not-tested: Full branch CI rerun after push
Contributor
Builds ready [7cc8149]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8a651da. Configure here.
The smart-transactions init file only differed from main because of a widened unknown-cast workaround that is not part of the perps controller change. Reverting it keeps the PR focused on the perps surface and removes a review distraction from inherited code. Constraint: Do not widen this PR beyond perps-owned changes Rejected: Keep the cast and dismiss the review thread | leaves unrelated churn in the live PR diff Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep non-perps typing cleanups out of this PR unless they are required by the perps change itself Tested: eslint on smart-transactions-controller-init.ts; verify-locales --quiet; circular-deps:check Not-tested: yarn lint (blocked by unrelated .agent/fixture-state-test.json workspace file)
Contributor
Builds ready [dc5f930]
⚡ Performance Benchmarks (Total: 🟢 6 pass · 🟡 10 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Contributor
Builds ready [b56f431]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
GeolocationController initializes lastFetchedAt as null and never eager-fetches, causing the flat state merge to produce null where snapshots expected a number.
Contributor
Builds ready [1d6c71a] [reused from b56f431]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 12 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
|
Contributor
Builds ready [428562d]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
MajorLift
approved these changes
Apr 17, 2026
Contributor
There was a problem hiding this comment.
Hallucination isn't the only failure mode to guard against when using autonomous agents. Tangentiality (not addressing the core ask or thesis) and Circumstantiality (excessive, unnecessary digressions) can be destructive as well.
Looking at the PR's end state, this could have been a quick, routine approval with maybe only the StorageService adoption being a requested change. But a lot of reviewer and author time was taken up dealing with churn caused by the inclusion of:
- a) Tangential changes that are unrelated to the PR's scope or thesis.
- b) Circumstantial changes that are wasteful, not useful, and even actively destructive.
- c) Erratic changes that disrupt or degrade existing patterns and processes for little to no added value.
We should equip agents with universal principles:
- Scope lock skill
- Specifications (tests, types, shared configs) as guardrails skill
- Diff audit skill
.agents/knowledgefiles with repo-specific onventions, patterns, processes, architecture (the platform team will work on this).
PR changes LGTM
Comment on lines
1920
to
+1923
| "@metamask/perps-controller": { | ||
| "globals": { | ||
| "AbortController": true, | ||
| "Intl.NumberFormat": true, |
Contributor
There was a problem hiding this comment.
Benign policy change for internal package
Contributor
Author
There was a problem hiding this comment.
Yes those are domain knowledge expertise that I don't have on extension yet, but now I do. Will integrate those skills.
Contributor
There was a problem hiding this comment.
The 3 skills should be applied org-wide not just for extension. The principles are global not domain-specific, although we might need to add wider examples and pitfall scenarios. There are probably more "ai-collboration" skills that we need to encode for all autonomous agents. The domain knowledge files specific to extension is something platform team will continue to work on.
aganglada
approved these changes
Apr 17, 2026
7 tasks
Contributor
Builds ready [428562d]
⚡ Performance Benchmarks (Total: 🟢 7 pass · 🟡 8 warn · 🔴 0 fail)
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 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.
Description
This PR is the base branch for the perps controller update and decimal-logic work in TAT-2699.
It upgrades the extension to
@metamask/perps-controller@3.1.1, keeps the decimal/calculation logic changes, and includes the required extension integration support for that controller update (StorageServicewiring, Jest support, and LavaMoat updates). A few perps UI files still appear here only where the retained hunks are logic-owned rather than presentation-only.The UI-facing display/formatting parity changes were split out into stacked PR #41853.
Changelog
CHANGELOG entry: null
Related issues
Fixes: TAT-2699
Related:
Manual testing steps
temp/agentic/recipes/teams/perps/recipes/pr-41558-decimal-formatting.json.Screenshots/Recordings
Before
N/A for this split PR. UI-facing screenshot evidence lives in stacked PR #41853.
After
N/A for this split PR. UI-facing screenshot evidence lives in stacked PR #41853.
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Medium risk because it upgrades a core dependency and changes perps infrastructure wiring (formatting, Sentry breadcrumbs, and persistent cache via StorageService), which can affect order-entry calculations and cached market data behavior.
Overview
Upgrades
@metamask/perps-controllerto3.1.1and updates perps infrastructure to use the package-providedformatPerpsFiat/formatPercentageand exposePRICE_RANGES_UNIVERSAL, aligning formatting behavior with the controller.Adds a
diskCacheimplementation tocreatePerpsInfrastructurebacked byStorageService(with an in-memory read-through cache) and wires the requiredStorageService:getItem/setItem/removeItemactions through the perps messenger/init path.Extends tracing to forward
addBreadcrumbcalls to Sentry, addsperpsCalculateLiquidationPriceto the background API wiring/tests, and updates LavaMoat policies to allowIntl.NumberFormatfor the perps controller.Reviewed by Cursor Bugbot for commit 428562d. Bugbot is set up for automated code reviews on this repo. Configure here.