Skip to content

Commit ca4425e

Browse files
fix: cp-7.53.0 e2e misusing checksum addresses (MetaMask#17896)
<!-- 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** After investigating state on both mobile and extension, the AccountsController and other state fixtures were incorrectly using checksummed addresses when we should be using lowercase addresses. This was uncovered since the new AccountTreeInit links together the `KeyringController` and the `AccountsController` https://github.com/MetaMask/metamask-mobile/blob/390dee8bb650c0f1f7a1fed3b8d15e67fa670093/app/multichain-accounts/AccountTreeInitService/index.ts#L26-L29 Fixes Failing Tests: ``` e2e/specs/assets/import-tokens-via-asset-watcher.spec.ts e2e/specs/assets/multichain/asset-sort.spec.ts e2e/specs/assets/defi/view-defi-details.spec.ts e2e/specs/assets/defi/view-defi-tab.spec.ts ``` <!-- 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** 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** - [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** - [ ] 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.
1 parent 0514f2c commit ca4425e

6 files changed

Lines changed: 36 additions & 19 deletions

File tree

e2e/api-mocking/mock-config/mock-events.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
suggestedGasFeesApiGanache,
99
} from '../mock-responses/gas-api-responses.json';
1010
import defiPositionsWithData from '../mock-responses/defi-api-response-data.json';
11+
import { DEFAULT_FIXTURE_ACCOUNT } from '../../framework/fixtures/FixtureBuilder';
1112

1213
export const mockEvents = {
1314
/**
@@ -164,22 +165,19 @@ export const mockEvents = {
164165
},
165166

166167
defiPositionsWithNoData: {
167-
urlEndpoint:
168-
'https://defiadapters.api.cx.metamask.io/positions/0x76cf1CdD1fcC252442b50D6e97207228aA4aefC3',
168+
urlEndpoint: `https://defiadapters.api.cx.metamask.io/positions/${DEFAULT_FIXTURE_ACCOUNT}`,
169169
response: { data: [] },
170170
responseCode: 200,
171171
},
172172

173173
defiPositionsError: {
174-
urlEndpoint:
175-
'https://defiadapters.api.cx.metamask.io/positions/0x76cf1CdD1fcC252442b50D6e97207228aA4aefC3',
174+
urlEndpoint: `https://defiadapters.api.cx.metamask.io/positions/${DEFAULT_FIXTURE_ACCOUNT}`,
176175
response: { error: 'Internal server error' },
177176
responseCode: 500,
178177
},
179178

180179
defiPositionsWithData: {
181-
urlEndpoint:
182-
'https://defiadapters.api.cx.metamask.io/positions/0x76cf1CdD1fcC252442b50D6e97207228aA4aefC3',
180+
urlEndpoint: `https://defiadapters.api.cx.metamask.io/positions/${DEFAULT_FIXTURE_ACCOUNT}`,
183181
response: { data: defiPositionsWithData },
184182
responseCode: 200,
185183
},

e2e/fixtures/utils.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ export function buildPermissions(chainIds) {
7878
caveats: [
7979
{
8080
type: Caip25CaveatType,
81+
/** @type {import('@metamask/chain-agnostic-permission').Caip25CaveatValue} */
8182
value: {
8283
optionalScopes,
8384
requiredScopes: {},

e2e/framework/fixtures/FixtureBuilder.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,14 @@ import {
1818
import { BackupAndSyncSettings, RampsRegion } from '../types';
1919
import { MULTIPLE_ACCOUNTS_ACCOUNTS_CONTROLLER } from './constants';
2020

21-
export const DEFAULT_FIXTURE_ACCOUNT =
21+
export const DEFAULT_FIXTURE_ACCOUNT_CHECKSUM =
2222
'0x76cf1CdD1fcC252442b50D6e97207228aA4aefC3';
2323

24+
export const DEFAULT_FIXTURE_ACCOUNT =
25+
DEFAULT_FIXTURE_ACCOUNT_CHECKSUM.toLowerCase() as Lowercase<
26+
typeof DEFAULT_FIXTURE_ACCOUNT_CHECKSUM
27+
>;
28+
2429
export const DEFAULT_FIXTURE_ACCOUNT_2 =
2530
'0xcdd74c6eb517f687aa2c786bc7484eb2f9bae1da';
2631

@@ -128,12 +133,12 @@ class FixtureBuilder {
128133
AccountTrackerController: {
129134
accountsByChainId: {
130135
64: {
131-
[DEFAULT_FIXTURE_ACCOUNT]: {
136+
[DEFAULT_FIXTURE_ACCOUNT_CHECKSUM]: {
132137
balance: '0x0',
133138
},
134139
},
135140
1: {
136-
[DEFAULT_FIXTURE_ACCOUNT]: {
141+
[DEFAULT_FIXTURE_ACCOUNT_CHECKSUM]: {
137142
balance: '0x0',
138143
},
139144
},
@@ -818,7 +823,7 @@ class FixtureBuilder {
818823
const permittedEthAccounts =
819824
incomingEthAccounts.length > 0
820825
? incomingEthAccounts
821-
: [DEFAULT_FIXTURE_ACCOUNT];
826+
: [DEFAULT_FIXTURE_ACCOUNT_CHECKSUM];
822827

823828
// Cast addresses to the required 0x${string} format
824829
const typedAddresses = permittedEthAccounts.map(
@@ -932,7 +937,7 @@ class FixtureBuilder {
932937
);
933938
const caip25CaveatValueWithDefaultAccount = setEthAccounts(
934939
caip25CaveatValueWithChains,
935-
[DEFAULT_FIXTURE_ACCOUNT],
940+
[DEFAULT_FIXTURE_ACCOUNT_CHECKSUM],
936941
);
937942
const chainPermission = {
938943
[Caip25EndowmentPermissionName]: {

e2e/specs/assets/import-tokens-via-asset-watcher.spec.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { SmokeNetworkAbstractions } from '../../tags';
22
import TestHelpers from '../../helpers';
33
import { loginToApp } from '../../viewHelper';
4-
import FixtureBuilder from '../../framework/fixtures/FixtureBuilder';
4+
import FixtureBuilder, {
5+
DEFAULT_FIXTURE_ACCOUNT,
6+
} from '../../framework/fixtures/FixtureBuilder';
57
import { withFixtures } from '../../framework/fixtures/FixtureHelper';
68
import { SMART_CONTRACTS } from '../../../app/util/test/smart-contracts';
79

@@ -12,6 +14,7 @@ import AssetWatchBottomSheet from '../../pages/Transactions/AssetWatchBottomShee
1214
import WalletView from '../../pages/wallet/WalletView';
1315
import { buildPermissions } from '../../fixtures/utils';
1416
import { DappVariants } from '../../framework/Constants';
17+
import { setEthAccounts } from '@metamask/chain-agnostic-permission';
1518

1619
const ERC20_CONTRACT = SMART_CONTRACTS.HST;
1720

@@ -23,6 +26,16 @@ describe(SmokeNetworkAbstractions('Asset Watch:'), () => {
2326
await TestHelpers.reverseServerPort();
2427
});
2528

29+
const buildERC20PermsForAddress = () => {
30+
const perms = buildPermissions(['0x539']);
31+
perms['endowment:caip25'].caveats[0].value = setEthAccounts(
32+
perms['endowment:caip25'].caveats[0].value,
33+
[DEFAULT_FIXTURE_ACCOUNT],
34+
);
35+
36+
return perms;
37+
};
38+
2639
it('Should Import ERC20 Token via Dapp', async () => {
2740
await withFixtures(
2841
{
@@ -34,7 +47,7 @@ describe(SmokeNetworkAbstractions('Asset Watch:'), () => {
3447
fixture: new FixtureBuilder()
3548
.withGanacheNetwork()
3649
.withPermissionControllerConnectedToTestDapp(
37-
buildPermissions(['0x539']),
50+
buildERC20PermsForAddress(),
3851
)
3952
.build(),
4053
restartDevice: true,

e2e/specs/multichain-accounts/common.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { mockEvents } from '../../api-mocking/mock-config/mock-events';
22
import FixtureBuilder, {
3-
DEFAULT_FIXTURE_ACCOUNT,
3+
DEFAULT_FIXTURE_ACCOUNT_CHECKSUM,
44
} from '../../framework/fixtures/FixtureBuilder';
55
import { withFixtures } from '../../framework/fixtures/FixtureHelper';
66
import AccountListBottomSheet from '../../pages/wallet/AccountListBottomSheet';
@@ -16,13 +16,13 @@ export interface Account {
1616
export const HD_ACCOUNT: Account = {
1717
name: 'Account 1',
1818
index: 0,
19-
address: DEFAULT_FIXTURE_ACCOUNT,
19+
address: DEFAULT_FIXTURE_ACCOUNT_CHECKSUM,
2020
};
2121

2222
export const SIMPLE_KEYPAIR_ACCOUNT: Account = {
2323
name: 'Account 4',
2424
index: 4,
25-
address: DEFAULT_FIXTURE_ACCOUNT,
25+
address: DEFAULT_FIXTURE_ACCOUNT_CHECKSUM,
2626
};
2727

2828
export const goToAccountDetails = async (account: Account) => {

e2e/specs/notifications/notification-settings-flow.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import Assertions from '../../framework/Assertions';
44
import { mockNotificationServices } from './utils/mocks';
55
import { withFixtures } from '../../framework/fixtures/FixtureHelper';
66
import FixtureBuilder, {
7-
DEFAULT_FIXTURE_ACCOUNT,
7+
DEFAULT_FIXTURE_ACCOUNT_CHECKSUM,
88
} from '../../framework/fixtures/FixtureBuilder';
99
import { loginToApp } from '../../viewHelper';
1010
import TabBarComponent from '../../pages/wallet/TabBarComponent';
@@ -66,11 +66,11 @@ describe(SmokeNetworkAbstractions('Notification Onboarding'), () => {
6666

6767
// Test account notifications toggle functionality
6868
await NotificationSettingsView.tapAccountNotificationsToggleAndVerifyState(
69-
DEFAULT_FIXTURE_ACCOUNT,
69+
DEFAULT_FIXTURE_ACCOUNT_CHECKSUM,
7070
'off',
7171
);
7272
await NotificationSettingsView.tapAccountNotificationsToggleAndVerifyState(
73-
DEFAULT_FIXTURE_ACCOUNT,
73+
DEFAULT_FIXTURE_ACCOUNT_CHECKSUM,
7474
'on',
7575
);
7676

0 commit comments

Comments
 (0)