[pull] main from MetaMask:main#733
Merged
Merged
Conversation
…eystore key invalidation (#29752) --- ## **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? --> **Reason for the change:** On Android, when a user changes their biometric enrollment (or due to an Android 16 Keystore behavioral change), the Keystore key backing the vault backup entry can be permanently invalidated. In that state, `getInternetCredentials(VAULT_BACKUP_KEY)` throws an exception rather than returning `false`/`undefined`. Because that call sat inside the outer `try/catch` in `backupVault`, the thrown error was caught at the top-level catch, which logged "Vault backup failed" to Sentry and returned `{ success: false }` — even though the underlying vault was fine and a fresh backup could have been written successfully. **Solution:** Wrap the initial `getInternetCredentials` read in its own inner `try/catch`. If the read throws (key invalidated), we: 1. Log a non-error warning via `Logger.log` (no Sentry noise) 2. Clear any corrupted keychain entries with `clearAllVaultBackups()` 3. Fall through to write a fresh backup normally This makes `backupVault` resilient to stale Android Keystore state and prevents a false "Vault backup failed" from ever surfacing when the real backup operation can succeed. ## **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: Fixed an issue on Android where vault backup could silently fail after a biometric enrollment change or Keystore key invalidation ## **Related issues** No issue: bug surfaced via Sentry report (`extra.message: "Vault backup failed"`) — Android Keystore key invalidation causing `getInternetCredentials` to throw inside `backupVault` ## **Manual testing steps** ```gherkin Feature: Vault backup resilience on Android Scenario: Vault backup succeeds even when existing backup read throws due to Keystore invalidation Given the user has a MetaMask wallet set up on Android And a vault backup already exists in the keychain And the Android Keystore key has been invalidated (e.g. by adding/removing biometric enrollment) When the vault backup routine is triggered (e.g. on wallet unlock or state change) Then the backup should complete successfully And no "Vault backup failed" error should appear in Sentry And the user should not be prompted with an error or crash ``` > Note: This fix is primarily defensive for an Android Keystore edge case. Manual reproduction requires a device where biometrics have been changed after the backup was created, or an Android 16 device exhibiting the Keystore regression. Unit test coverage added for the throwing path. ## **Screenshots/Recordings** ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** <!-- Every checklist item must be consciously assessed before marking this PR as "Ready for review". A checked box means you deliberately considered that responsibility, not that you literally performed every action listed. Unchecked boxes are ambiguous: they are not an implicit "N/A" and they are not a silent "skip". See `docs/readme/ready-for-review.md` for the full checklist semantics. --> - [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. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [x] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **Pre-merge reviewer checklist** <!-- Reviewer checklist items follow the same semantics as the author checklist: an unchecked box is ambiguous, a checked box means the reviewer consciously assessed that responsibility. See `docs/readme/ready-for-review.md`. --> - [ ] 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. <!-- Generated with the help of the pr-description AI skill --> Made with [Cursor](https://cursor.com) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk, localized change that only alters error handling around reading existing keychain credentials and adds a unit test for the throwing path. Main risk is potentially masking unexpected read failures by proceeding to overwrite the backup. > > **Overview** > Makes `backupVault` resilient to Android Keystore/keychain read exceptions by wrapping the initial `getInternetCredentials(VAULT_BACKUP_KEY)` call in an inner try/catch and proceeding as if no existing backup was found (while logging via `Logger.log`). > > Adds a unit test ensuring vault backup still reports success when the existing-backup read throws (simulating Android Keystore key invalidation). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6a6edf9. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…etection, and collapse animation (#29684) ## **Description** Adds new `TabsIcon` components to `components-temp` and new icons to the component library as part of the hub page discovery tabs A/B test. Extracted into its own PR so it can be reviewed and merged independently before the feature PR lands. --- ### `TabsIcon` Component Family **`TabsIconTab`** Tab item that renders an icon above a label. Icon and label have active/disabled color states. Supports a `fillWidth` prop for equal-width layouts and animated icon collapse via `TabsIconAnimationContext`. **`TabsIconBar`** Tab bar built for icon+label tabs. Features: - Automatic overflow detection — switches between a fixed layout and a horizontally scrollable `ScrollView` - Animated sliding underline indicator - `fillWidth` mode for equal-width tabs - Height collapse animation for hiding the bar on scroll **`TabsIconList`** Lazy-mounting tab list that manages active state, swipe gestures, and `InteractionManager`-based content loading. Per-tab `keepMounted` prop controls whether inactive tab content stays mounted or is unmounted when the tab loses focus. **`TabsIconAnimationContext`** React context that carries an `Animated.Value` (`0` = icons expanded, `1` = collapsed) from a parent provider down to `TabsIconTab` without prop drilling. --- ### Icons Adds three new SVG icons and registers them in `Icon.assets.ts` / `Icon.types.ts`: | Icon | `IconName` | |---|---| | Portfolio | `IconName.Portfolio` | | Predict | `IconName.Predict` | | Candlestick | `IconName.Candlestick` | ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TMCU-591 ## **Manual testing steps** ```gherkin Feature: Tabs component library Scenario: user navigates between tabs Given the app is open on any screen using TabsList When user taps each tab Then the animated underline slides to the active tab And the correct tab content is displayed Scenario: tabs overflow to scrollable mode Given a TabsBar with more tabs than fit the container width When the screen renders Then the tab bar becomes horizontally scrollable And the active tab is scrolled into view automatically Scenario: new icons render correctly Given any view that uses the Icon component When IconName.Portfolio or IconName.Predict is passed Then the correct SVG icon is displayed ``` ## **Screenshots/Recordings** Take from the feature branch using these components [here](#29193) ### iOS Dark Mode https://github.com/user-attachments/assets/7df6a46d-b3b3-44cc-a697-b796581dd759 Light Mode (No Gradient) https://github.com/user-attachments/assets/97b1f901-6092-42f9-926c-e8e6785c6f4e ### Android Dark Mode https://github.com/user-attachments/assets/4232f3fc-a47c-4516-93c2-104e4a3fb5cb Light Mode (No Gradient) https://github.com/user-attachments/assets/3f11dd8f-696f-438e-9867-1b08dc69200d ### **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. #### Performance checks (if applicable) - [x] I've tested on Android - Ideally on a mid-range device; emulator is acceptable - [x] I've tested with a power user scenario - Use these [power-user SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93) to import wallets with many accounts and tokens - [x] I've instrumented key operations with Sentry traces for production performance metrics - See [`trace()`](/app/util/trace.ts) for usage and [`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274) for an example For performance guidelines and tooling, see the [Performance Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers). ## **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. [TMCU-591]: https://consensyssoftware.atlassian.net/browse/TMCU-591?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Adds new tab UI components plus shared layout/list hooks involving animations, gesture handling, and InteractionManager-based lazy loading; regressions would primarily impact navigation/UX in screens adopting these new components. Also extends the global `IconName` enum/assets, which can affect consumers if mis-registered. > > **Overview** > Adds a new `TabsIcon` component family under `components-temp`: > > `TabsIconTab` renders icon+label tabs with active/disabled states and optional icon collapse via `TabIconAnimationContext`, `TabsIconBar` manages underline animation plus *auto scroll overflow detection* and optional height collapse, and `TabsIconList` composes the bar with lazy-mounted tab content, disabled-tab handling, swipe gestures, and a small imperative ref API. > > Introduces reusable hooks `useTabsBarLayout` (measures tab/container layouts, toggles scroll mode, drives underline animations, and resets/cleans up on layout/tab changes) and `useTabsList` (active index normalization, InteractionManager-backed lazy loading with timeout fallback, key-preserving tab updates, and swipe-to-switch). > > Extends the icon library by registering new SVGs and `IconName` entries (`Group`, `Portfolio`, `Predictions`) and updates the `candlestick.svg` asset. Comprehensive unit tests are added for the new components and hooks. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit ffa57ba. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/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.
2 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 : )