-
Notifications
You must be signed in to change notification settings - Fork 5.4k
test: Fix bip44 failures #38425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
davibroc
wants to merge
87
commits into
main
Choose a base branch
from
fix-bip44-failures
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
test: Fix bip44 failures #38425
+1,899
−1,473
Conversation
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
<!-- 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. --> This PR turns BIP44 by default and make changes to the fixtures so that Solana account is already created when the Extension starts [](https://codespaces.new/MetaMask/metamask-extension/pull/37365?quickstart=1) <!-- 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: Fixes: 1. Go to this page... 2. 3. <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> <!-- [screenshots/recordings] --> - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. - [ ] 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] > Enables BIP-44 state 2 by default across e2e, adds multichain/Solana defaults to fixtures, and updates page objects and tests to the new multichain flows and feature-flag mocks. > > - **E2E Framework**: > - Default `withFixtures` now enables BIP-44 stage 2 via feature flag (boolean `forceBip44Version`, true by default) and applies only state-2 mocks when enabled. > - Updated selectors and flows in benchmarks (use `HeaderNavbar`/`AccountListPage`; activity selector for confirmed tx). > - **Fixtures & Mocks**: > - `default-fixture` seeds `AccountTreeController` with a multichain wallet (Ethereum + Solana account) and updates `AccountsController` metadata; enables Solana in network enablement. > - Introduces `BIP44_STAGE_TWO` and injects it into feature-flag responses; various bridge fixtures/mocks include it. > - **Page Objects**: > - `AccountListPage.checkPageIsLoaded` simplified to multichain UI (always expects "Create account" and options menu; removes state arg); ensures "Syncing" not shown. > - `HeaderNavbar` adds `clickNetworkAddresses`. > - **Bridge Tests**: > - Adjusted expectations to ETH balances/units; explicitly disable BIP-44 where needed; minor skips added; hardforks normalized; duplicate mocks removed. > - **Multichain Accounts Tests**: > - Updated to new `checkPageIsLoaded()` signature and multichain flows; remove per-test state flags; minor assertions streamlined. > - Unskips and simplifies wallet details scenario (validate presence in account list). > - **Solana Test Utilities**: > - Add native price mock; tweak enabled networks and initial navigation (network filter steps); remove per-account creation loop. > - **Misc**: > - Numerous tests now pass `forceBip44Version: false` where legacy behavior is required; balance constants adjusted. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f8bb587. 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** <!-- 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? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/37406?quickstart=1) ## **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** 1. Go to this page... 2. 3. ## **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 Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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] > Hardcodes local-node balance to 24.998 in homepage tests and enables the sendRedesign feature flag in multichain accounts mocks. > > - **E2E Tests**: > - **Homepage balance check** (`test/e2e/page-objects/pages/home/homepage.ts`): > - For local nodes, replace dynamic balance fetch with fixed `"24.998"` in `checkLocalNodeBalanceIsDisplayed`. > - **Feature flags** (`test/e2e/tests/multichain-accounts/feature-flag-mocks.ts`): > - Set `sendRedesign.enabled` to `true` in `BIP44_STAGE_TWO`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit aea2615. 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** <!-- 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? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/38154?quickstart=1) ## **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** 1. Go to this page... 2. 3. ## **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 Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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] > Stabilizes and refactors multichain and snap account E2E tests by standardizing login flows, simplifying setup, adjusting delays/mocks/fixtures, and reworking the hardware wallet test to use direct fixtures with local balance setup. > > - **E2E: Snap account transfers (`test/e2e/tests/account/snap-account-transfers.spec.ts`)** > - Standardize on `loginWithBalanceValidation`; remove `loginWithoutBalanceValidation` usage. > - Adjust timing: add pre-send delay for sync flow; keep post-send delay for async approve. > - Update fixtures (enable networks/prefs) and mocks wiring; add `ignoredConsoleErrors` for reject case. > - Update balance assertions; comment out flaky post-tx balance check in async approve. > - **E2E helper refactor (`test/e2e/tests/multichain-accounts/common.ts`)** > - Remove `NetworkManager` navigation; open account menu directly after login. > - Split fixtures by `AccountType` (Ledger vs MultiSRP) with prefs/network enabled; tweak mock aggregation. > - Use `loginWithoutBalanceValidation` only for hardware wallets; others use `loginWithBalanceValidation`. > - **E2E: Multichain account list menu (`test/e2e/tests/multichain-accounts/multichain-account-list-menu.spec.ts`)** > - Rework hardware wallet test to use `withFixtures`, set local node balance, and log in with balance validation; add necessary mocks/imports. > - Minor cleanup and consistency updates across tests (headers/navigation, balances). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit d506922. 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** <!-- 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? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/38207?quickstart=1) ## **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** 1. Go to this page... 2. 3. ## **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 Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/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] > Replace modified vault fixtures and update E2E tests (snaps labels, network counts, metrics), refactor websocket tests, and revise state-logs schema and spec. > > - **Fixtures**: > - Replace encrypted `vault` payloads in `test/e2e/default-fixture.js` and `fixture-builder.js` (`withKeyringControllerAdditionalAccountVault`, `withKeyringControllerMultiSRP`, default vault). > - **Snaps E2E**: > - Update entropy source labels from long IDs to `SRP 1 (primary)` / `SRP 2` in `test-snap-bip-32.spec.ts`, `test-snap-bip-44.spec.ts`, and `test-snap-get-entropy.spec.ts`. > - **Connections/Permissions E2E**: > - Adjust expected connected networks count from `12` to `11` in `edit-networks-permissions.spec.ts` and `multiple-provider-connections.spec.ts`. > - **Metrics E2E**: > - Update `DappViewed` assertions for `number_of_accounts` from `3` to `1` in `dapp-viewed.spec.ts`. > - **WebSocket E2E**: > - Remove `tests/multichain-accounts/web-socket-connection.spec.ts`. > - Refactor `tests/multichain/web-socket-connection.spec.ts` to use `waitForWebsocketConnections` helper and eliminate fixed delays. > - **State Logs**: > - Expand `state-logs.json` schema (e.g., `accountsAssets`, `assetsMetadata`, `balances`, `nonEvmTransactions`) and adjust internal account structures. > - Update `state-logs.spec.ts` to normalize dynamic account IDs before type-map comparison. > - **Misc**: > - Remove a debug `console.log` in hardware wallets helper. > - Fix minor comment typo in snap account transfers test. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit a413e55. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
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. |
Collaborator
✨ Files requiring CODEOWNER review ✨🧪 @MetaMask/qa (12 files, +193 -155)
👨🔧 @MetaMask/wallet-integrations (6 files, +9 -14)
|
Collaborator
Builds ready [ab454df]
UI Startup Metrics (1305 ± 103 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Overhauls E2E tests and helpers to run with BIP44 stage 2 and multichain accounts, updating selectors, mocks, balances, and flows across pages and specs.
BIP44_STAGE_TWOfeature flag and wire into mocks/manifests.forceBip44Versionper test where needed.transaction-status-label--confirmed).Written by Cursor Bugbot for commit ab454df. This will update automatically on new commits. Configure here.