fix(perps): enforce geo-block and compliance gate on Market Insights Long/Short actions cp-7.72.1#28678
Conversation
…erps actions The MarketInsightsView Long/Short buttons navigated directly to PerpsOrderRedirect without checking geo-eligibility or running the compliance gate, allowing restricted users to open positions. Adds the same guards used by PerpsMarketDetailsView and AssetOverviewContent so the geo-block modal is shown for ineligible users.
|
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. |
| contentKey="geo_block" | ||
| testID="market-insights-geo-block-tooltip" | ||
| /> | ||
| </Modal> |
There was a problem hiding this comment.
Extra Modal wrapper inconsistent with reference pattern
Medium Severity
The PerpsBottomSheetTooltip is wrapped in a React Native Modal here, but the reference implementation in PerpsMarketDetailsView (line ~1574) renders PerpsBottomSheetTooltip directly without a Modal wrapper. Since PerpsBottomSheetTooltip likely already provides its own modal/bottom-sheet overlay internally, the extra Modal wrapper can cause double backdrops, z-index conflicts, or touch-event interception issues that break the dismiss behavior.
Reviewed by Cursor Bugbot for commit f9a9f99. Configure here.
There was a problem hiding this comment.
Some seem to be using modal and some not, this is likely just due to the animation
|
@metamaskbot update-mobile-fixture |
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->
## **Description**
<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
## **Changelog**
<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`
If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`
(This helps the Release Engineer do their job more quickly and
accurately)
-->
CHANGELOG entry:
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: my feature name
Scenario: user [verb for user action]
Given [describe expected initial app state]
When user [verb for user action]
Then [describe expected outcome]
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!-- [screenshots/recordings] -->
### **After**
<!-- [screenshots/recordings] -->
## **Pre-merge author checklist**
- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I've included tests if applicable
- [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
## **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.
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Low Risk**
> Low risk test-only change that adjusts component-view rendering
behavior; main risk is breaking navigation assertions if provider
wrapping alters route props or stack behavior.
>
> **Overview**
> Updates `renderPerpsView` test renderer so **all `extraRoutes`
screens** (both root stack and `Routes.PERPS.ROOT` nested stack) are
wrapped with the same Perps providers (`AccessRestrictedProvider`,
`PerpsConnectionContext`, `PerpsStreamProvider`) as the primary view.
This ensures extra routes can render real Perps views (not just probes)
without missing-context errors during component-view tests.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
b3dae39. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
| isVisible | ||
| onClose={closeEligibilityModal} | ||
| contentKey="geo_block" | ||
| testID="market-insights-geo-block-tooltip" |
There was a problem hiding this comment.
Geo-block testID missing from selectors enum, uses raw string
Low Severity
The new testID="market-insights-geo-block-tooltip" is hardcoded as a raw string in the component and queried with raw strings in the test (queryByTestId('market-insights-geo-block-tooltip'), getByTestId('market-insights-geo-block-tooltip')). Every other testID in this component references the MarketInsightsSelectorsIDs enum from MarketInsights.testIds.ts. This new testID needs to be added to that enum and referenced via the constant, consistent with the rest of the file. Violates the unit testing guideline to prefer testID constants over hardcoded strings.
Additional Locations (1)
Triggered by project rule: Unit Testing Guidelines
Reviewed by Cursor Bugbot for commit 40d8e04. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ 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 fac7ff6. Configure here.
…Mask/metamask-mobile into fix/perps-market-insights-geo-block
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag Selection Rationale:
The changes are scoped to Perps/MarketInsights interaction with no impact on accounts, networks, swaps, or other unrelated areas. Performance Test Selection: |
|
|
✅ E2E Fixture Validation — Schema is up to date |
Description
The
MarketInsightsView(AI summaries page) Long/Short buttons navigated directly toPerpsOrderRedirectwithout checking geo-eligibility or running the compliance gate. This allowed users in restricted regions to bypass restrictions and open perps positions from the Market Insights entry point, even though every other entry point (Market Details, Order Book, Token Details) correctly blocked them.Changes:
handlePerpsDirectionPressin the compliancegate()and added theisEligiblegeo-check before navigation, matching the pattern inPerpsMarketDetailsViewandAssetOverviewContentPerpsBottomSheetTooltipgeo-block modal ("Perps unavailable in your region") when the user is ineligibleMARKET_INSIGHTStoPERPS_EVENT_VALUE.SOURCEfor proper geo-block analytics attributionChangelog
CHANGELOG entry: Fixed a bug where users in restricted regions could open perps positions from the Market Insights page
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
N/A — behavior matches existing geo-block modal shown on other Perps entry points.
Before
Tapping Long/Short on Market Insights in a restricted region navigated to the order screen (bypassing restrictions).
After
Tapping Long/Short on Market Insights in a restricted region shows the geo-block modal and blocks navigation.
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Touches Perps eligibility/compliance gating and navigation from
MarketInsightsView, so mistakes could reintroduce a restriction bypass or block eligible users; changes are localized and covered by updated tests.Overview
Prevents the Market Insights Perps
Long/ShortCTAs from navigating directly to order flow without restrictions by wrapping the action in the compliancegate()and checkingselectPerpsEligibilityfirst.When ineligible, it now tracks a geo-block screen view with source
PERPS_EVENT_VALUE.SOURCE.MARKET_INSIGHTSand shows thePerpsBottomSheetTooltipin a modal instead of navigating. Tests were updated to handle async gated presses and a new test asserts the ineligible geo-block behavior; the Perps view test renderer also ensures extra routes get the same Perps providers.Reviewed by Cursor Bugbot for commit 8ed8908. Bugbot is set up for automated code reviews on this repo. Configure here.