[pull] main from MetaMask:main#400
Merged
Merged
Conversation
… to be incorrectly geo-blocked (#23895) <!-- 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** Fix race condition causing users to be geo-blocked incorrectly. The Bug: `refreshEligibility()` is called with fallback regions but isn't awaited — so the constructor continues, updates to remote config, and starts new eligibility checks. The fallback check finishes last and overwrites the correct remote result with false. Workaround: a version counter ensures stale checks (started before the config changed) are discarded when they complete. <!-- 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: fix perps refreshEligibility race condition causing users to be geo-blocked ## **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** - [x] 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). - [x] 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 - [x] 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] > Guards async eligibility updates with a versioned blocked-region list to avoid stale fallback results overwriting remote-config checks. > > - **PerpsController (`app/components/UI/Perps/controllers/PerpsController.ts`)** > - Add `blockedRegionListVersion` to track changes to `blockedRegionList`. > - Increment version when setting blocked regions in `setBlockedRegionList` and in `refreshEligibilityOnFeatureFlagChange`. > - Update `refreshEligibility()` to: > - Capture version at start and skip state updates if version changed while awaiting. > - Only set default eligible on error if version is unchanged. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1fa308b. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Nicholas Gambino <nicholas.gambino@consensys.net>
#23904) ## **Description** remove background from more button in multichain account selector ## **Changelog** CHANGELOG entry:null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/MDP-277 ## **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** | before | after | | -------- | ------- | |  |  | ### **Before** `~` ### **After** `~` ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > Removes background, border radius, and fixed dimensions from the multichain account cell "more" menu button, updating snapshots accordingly. > > - **UI (Multichain Accounts)** > - `AccountCell.styles.ts`: Remove `menuButton` `backgroundColor`, `borderRadius`, `height`, and `width` to make the "more" button icon-only. > - **Tests** > - Update snapshots in `MultichainAccountsConnectedList.test.tsx.snap` to reflect the menu button style change. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 5b2e2cc. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## **Description** Updating most instances of selected item background color to `colors.background.muted` ## **Changelog** CHANGELOG entry:null ## **Related issues** Fixes: Region Picker: https://consensyssoftware.atlassian.net/browse/MDP-280 Network Picker: https://consensyssoftware.atlassian.net/browse/MDP-239 Sort: https://consensyssoftware.atlassian.net/browse/MDP-243 ## **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** ### Network Manager | before | after | | -------- | ------- | |  |  | ### RPC Selection | before | after | | -------- | ------- | |  |  | ### Sort | before | after | | -------- | ------- | |  |  | ### Payment Selector | before | after | | -------- | ------- | |  |  | ### Region Picker | before | after | | -------- | ------- | |  |  | ### Select Network | before | after | | -------- | ------- | |  |  | ### **Before** `~` ### **After** `~` ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] 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** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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] > Updates selected item visuals across lists/selectors to use `colors.background.muted` and drops the side underlay bar indicator, with snapshots adjusted. > > - **Styling/UX**: > - Use `colors.background.muted` for selected states in `ListItemMultiSelect`, `ListItemSelect`, `ListItemMultiSelectButton`, and downstream UI (account/network/region/token selectors, permissions screens). > - Remove underlay side bar visuals (`underlayBar`, width strip) and related props/tests; retain simple full-row underlay where applicable. > - **Components**: > - `components-temp/ListItemMultiSelectButton`: selected container background updated; removed underlay view rendering; tests simplified (icon/press only). > - `components/List/ListItemSelect`: underlay color changed; removed `underlayBar`; kept minimal `underlay` view. > - `components/List/ListItemMultiSelect`: underlay color changed. > - Remove unused constant `SELECTABLE_ITEM_UNDERLAY_ID`. > - **Tests/Snapshots**: > - Snapshot updates to reflect color change (`#4459ff1a` → `#3c4d9d0f`) and removal of side bar indicator across numerous selector/modal views. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9dbd116. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
<!--
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**
This PR refactors wallet deletion logic by consolidating the
`useDeleteWallet` hook into the `Authentication` service class. The
refactoring improves code organization, ensures consistent wallet
deletion behavior, and simplifies the codebase by removing redundant
hook logic.
**Reason for the change:**
1. The `useDeleteWallet` hook duplicated logic that should be
centralized in the `Authentication` service
2. `resetWalletState` and `deleteUser` were always used together, but
there was no enforcement of this pattern
3. Having wallet deletion logic in a hook created unnecessary coupling
between React components and core authentication logic
4. The hook pattern was not appropriate for core service operations that
don't depend on React lifecycle
**Improvement/Solution:**
1. **Moved Logic to Authentication Service**: Consolidated
`resetWalletState` and `deleteUser` methods into
`app/core/Authentication/Authentication.ts` as protected methods
2. **Created Public API**: Added `deleteWallet()` as the main public
method that ensures `resetWalletState()` is always called before
`deleteUser()`
3. **Protected Methods**: Made `resetWalletState` and `deleteUser`
protected to prevent direct external access and enforce the correct
usage pattern through `deleteWallet()`
4. **Updated All Call Sites**: Refactored `usePromptSeedlessRelogin` and
`DeleteWalletModal` to use the new `Authentication.deleteWallet()`
method
5. **Comprehensive Test Coverage**: Added tests for the new
`deleteWallet()` method and maintained test coverage for the protected
methods
**Key Technical Improvements:**
- **Removed Hook**: Deleted
`app/components/hooks/DeleteWallet/useDeleteWallet.ts` and related test
files
- **Centralized Logic**: All wallet deletion logic is now in the
`Authentication` service, following the single responsibility principle
- **Enforced Usage Pattern**: The protected methods ensure that wallet
reset and user deletion always happen in the correct sequence
- **Better Encapsulation**: Protected methods prevent accidental misuse
and make the API surface clearer
- **Consistent Behavior**: All wallet deletion flows now use the same
underlying implementation
## **Changelog**
CHANGELOG entry: Refactored wallet deletion logic by consolidating
useDeleteWallet hook into Authentication service
## **Related issues**
Fixes:
## **Manual testing steps**
```gherkin
Feature: Wallet Deletion
Scenario: Delete wallet from seedless relogin prompt
Given the user encounters a seedless controller error
And the error prompt is displayed
When the user taps the primary button to delete wallet
Then the wallet state should be reset
And user data should be deleted
And the user should be navigated to onboarding
And no errors should occur during the deletion process
Scenario: Delete wallet from settings
Given the user is viewing the Security Settings screen
And the user navigates to the Delete Wallet modal
When the user confirms wallet deletion
Then the wallet state should be reset
And user data should be deleted
And cookies should be cleared
And the user should be navigated to onboarding
And analytics should track the deletion event
Scenario: Wallet deletion error handling
Given the user attempts to delete the wallet
When an error occurs during wallet reset
Then the error should be logged
And the loading state should be reset
And the user should see appropriate error feedback
And the app should remain in a stable state
```
## **Screenshots/Recordings**
<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->
### **Before**
<!--
Before: Wallet deletion logic was split between a hook and the
Authentication service
- useDeleteWallet hook managed resetWalletState and deleteUser
separately
- Components had to call both methods in sequence manually
- No enforcement that both methods were always called together
-->
### **After**
https://github.com/user-attachments/assets/7dcb9cc3-cb52-4c50-8f1c-009fa402d1de
<!--
After: Wallet deletion is centralized in Authentication service
- Single deleteWallet() method ensures correct sequence
- Protected methods prevent misuse
- Cleaner component code with single method call
-->
## **Pre-merge author checklist**
- [x] 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).
- [x] I've completed the PR template to the best of my ability
- [x] I've included tests if applicable
- [x] 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]
> Centralizes wallet deletion into `Authentication.deleteWallet()` and
refactors consumers/tests to use it, removing the old `useDeleteWallet`
hook.
>
> - **Core (Authentication)**:
> - Add `Authentication.deleteWallet()` as the public API; implements
sequential `resetWalletState` → `deleteUser`.
> - `resetWalletState` (protected): clears vault backups,
disables/re-enables `EngineClass.disableAutomaticVaultBackup`, creates
temp wallet, clears seedless state, resets rewards via
`Engine.controllerMessenger`, clears provider token, locks app (no
navigation).
> - `deleteUser` (protected): dispatches `setExistingUser(false)` and
triggers `MetaMetrics.createDataDeletionTask`.
> - Also removes `OPTIN_META_METRICS_UI_SEEN` and dispatches
`setCompletedOnboarding(false)`.
> - **UI**:
> - `DeleteWalletModal`: replace hook flow with
`Authentication.deleteWallet()`, keep sign-out, clear cookies, dispatch
`clearHistory`, track event, then navigate to onboarding.
> - `usePromptSeedlessRelogin`: invoke `Authentication.deleteWallet()`
and simplify flow; maintain loading/error state and `clearHistory` +
sign-out + navigation.
> - **Hooks**:
> - Remove `app/components/hooks/DeleteWallet/*` (hook and tests).
> - **Tests**:
> - Add comprehensive tests for `deleteWallet`, `resetWalletState`, and
`deleteUser` in `Authentication.test.ts`.
> - Update component/hook tests to assert calls to
`Authentication.deleteWallet()` and revised action sequences; remove
assertions tied to deleted hook/storage calls.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
b71e98d. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
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.
3 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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )