Skip to content

Commit

Permalink
fix: Fix/9662 account section disappears (#9733)
Browse files Browse the repository at this point in the history 
<!--
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?
-->

This PR fixes the issue where the accounts section on the wallet screen
disappears after re-authenticating post app auto lock. The main change
happens in the `useAccounts` hook, where we are now grabbing the
accounts from the `AccountsController` via Redux rather than from the
`KeyringController` from the Engine context. We're also now fetching the
selected account from the `AccountsController` as well. While the root
issue isn't immediately clear, I suspect it has to do with the accounts
temporarily disappearing from the KeyringController between lock/auth
phases while the app is locked for an extended period of time. In any
case, these changes mirror the changes that @owencraston is working on,
which will eventually migrate most accounts data to be derived from the
`AccountsController`. This fix can serve as a POC that the
`AccountsController` is a reliable source of truth when referencing
account information.

## **Related issues**

Fixes: #9662 #9279
## **Manual testing steps**

1. Import wallet via SRP
2. Check that phone is set to auto lock
3. Set MM app to immediate lock
4. Go to wallet screen
5. Wait for app to auto lock
6. Wait 5-10mins
7. Unlock phone and re-authenticate to MM
8. Notice account section is still intact on the wallet screen

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->


https://github.com/MetaMask/metamask-mobile/assets/10508597/27d7823b-1d1f-48a0-89ae-946cdcf6e853


### **After**

<!-- [screenshots/recordings] -->


https://github.com/MetaMask/metamask-mobile/assets/10508597/e8ba2a56-ee2c-48bf-b602-46e62626f1f5


## **Pre-merge author checklist**

- [X] I’ve followed [MetaMask 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.
  • Loading branch information
@Cal-L
Cal-L authored May 23, 2024
1 parent d632331 commit c8d161e
Show file tree
Hide file tree
Showing 13 changed files with 1,693 additions and 231 deletions.
48 changes: 22 additions & 26 deletions app/components/UI/AccountSelectorList/AccountSelector.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,36 +1,30 @@
import React from 'react';
// eslint-disable-next-line @typescript-eslint/no-shadow
import { waitFor, within } from '@testing-library/react-native';
import Engine from '../../../core/Engine';
import renderWithProvider from '../../../util/test/renderWithProvider';
import AccountSelectorList from './AccountSelectorList';
import { useAccounts } from '../../../components/hooks/useAccounts';
import { View } from 'react-native';
import { ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID } from '../../../../wdio/screen-objects/testIDs/Components/AccountListComponent.testIds';
import initialBackgroundState from '../../../util/test/initial-background-state.json';
import { regex } from '../../../../app/util/regex';

const mockEngine = Engine;

const BUSINESS_ACCOUNT = '0xC4955C0d639D99699Bfd7Ec54d9FaFEe40e4D272';
const PERSONAL_ACCOUNT = '0xd018538C87232FF95acbCe4870629b75640a78E7';

jest.mock('../../../core/Engine', () => ({
init: () => mockEngine.init({}),
context: {
KeyringController: {
state: {
keyrings: [
{
type: 'HD Key Tree',
index: 0,
accounts: [BUSINESS_ACCOUNT, PERSONAL_ACCOUNT],
},
],
},
},
},
}));
import { MOCK_ACCOUNTS_CONTROLLER_STATE } from '../../../util/test/accountsControllerTestUtils';
import { toChecksumAddress } from 'ethereumjs-util';

const MOCK_ACCOUNT_ADDRESSES = Object.values(
MOCK_ACCOUNTS_CONTROLLER_STATE.internalAccounts.accounts,
).map((account) => account.address);

const BUSINESS_ACCOUNT = toChecksumAddress(MOCK_ACCOUNT_ADDRESSES[0]);
const PERSONAL_ACCOUNT = toChecksumAddress(MOCK_ACCOUNT_ADDRESSES[1]);

jest.mock('../../../util/address', () => {
const actual = jest.requireActual('../../../util/address');
return {
...actual,
getLabelTextByAddress: jest.fn(),
};
});

const initialState = {
engine: {
Expand All @@ -44,6 +38,7 @@ const initialState = {
chainId: '0x1',
},
},
AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE,
AccountTrackerController: {
accounts: {
[BUSINESS_ACCOUNT]: { balance: '0xDE0B6B3A7640000' },
Expand Down Expand Up @@ -167,6 +162,7 @@ describe('AccountSelectorList', () => {
...initialState.engine.backgroundState.PreferencesController,
isMultiAccountBalancesEnabled: false,
},
AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE,
},
},
});
Expand All @@ -176,12 +172,12 @@ describe('AccountSelectorList', () => {
expect(accounts.length).toBe(1);

const businessAccountItem = await queryByTestId(
`${ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${BUSINESS_ACCOUNT}`,
`${ACCOUNT_BALANCE_BY_ADDRESS_TEST_ID}-${PERSONAL_ACCOUNT}`,
);

expect(within(businessAccountItem).getByText(regex.eth(1))).toBeDefined();
expect(within(businessAccountItem).getByText(regex.eth(2))).toBeDefined();
expect(
within(businessAccountItem).getByText(regex.usd(3200)),
within(businessAccountItem).getByText(regex.usd(6400)),
).toBeDefined();

expect(toJSON()).toMatchSnapshot();
Expand Down
909 changes: 742 additions & 167 deletions app/components/UI/AccountSelectorList/__snapshots__/AccountSelector.test.tsx.snap
View file

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion app/components/UI/Tabs/TabThumbnail/TabThumbnail.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ import React from 'react';
import renderWithProvider from '../../../../util/test/renderWithProvider';
import initialBackgroundState from '../../../../util/test/initial-background-state.json';
import TabThumbnail from './TabThumbnail';
import { MOCK_ACCOUNTS_CONTROLLER_STATE } from '../../../../util/test/accountsControllerTestUtils';

const mockInitialState = {
wizard: {
step: 1,
},
engine: {
backgroundState: initialBackgroundState,
backgroundState: {
...initialBackgroundState,
AccountsController: MOCK_ACCOUNTS_CONTROLLER_STATE,
},
},
};

Expand Down
247 changes: 246 additions & 1 deletion app/components/UI/Tabs/TabThumbnail/__snapshots__/TabThumbnail.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,251 @@ exports[`TabThumbnail should render correctly 1`] = `
}
}
>
<View
style={
{
"paddingRight": 8,
}
}
>
<View
onLayout={[Function]}
style={
{
"alignSelf": "flex-start",
"position": "relative",
}
}
testID="badge-wrapper-badge"
>
<View>
<View
style={
{
"backgroundColor": "#FFFFFF",
"borderRadius": 8,
"height": 16,
"overflow": "hidden",
"width": 16,
}
}
>
<View
style={
[
{
"overflow": "hidden",
},
{
"backgroundColor": "#FC4800",
"borderRadius": 8,
"height": 16,
"width": 16,
},
undefined,
]
}
>
<RNSVGSvgView
bbHeight={16}
bbWidth={16}
focusable={false}
height={16}
style={
[
{
"backgroundColor": "transparent",
"borderWidth": 0,
},
{
"flex": 0,
"height": 16,
"width": 16,
},
]
}
width={16}
>
<RNSVGGroup
fill={
{
"payload": 4278190080,
"type": 0,
}
}
>
<RNSVGRect
fill={
{
"payload": 4278291575,
"type": 0,
}
}
height={16}
matrix={
[
-0.41310442982454204,
-0.910683660806177,
0.910683660806177,
-0.41310442982454204,
4.404802817153955,
20.16808402411075,
]
}
propList={
[
"fill",
]
}
width={16}
x={0}
y={0}
/>
<RNSVGRect
fill={
{
"payload": 4278410587,
"type": 0,
}
}
height={16}
matrix={
[
0.903335292863301,
-0.42893513340314526,
0.42893513340314526,
0.903335292863301,
-9.297010789302583,
3.362634662066926,
]
}
propList={
[
"fill",
]
}
width={16}
x={0}
y={0}
/>
<RNSVGRect
fill={
{
"payload": 4294382337,
"type": 0,
}
}
height={16}
matrix={
[
-0.6921431738704069,
-0.7217602280983622,
0.7217602280983622,
-0.6921431738704069,
-6.169639630140347,
15.20799235933167,
]
}
propList={
[
"fill",
]
}
width={16}
x={0}
y={0}
/>
</RNSVGGroup>
</RNSVGSvgView>
</View>
</View>
</View>
<View
style={
{
"alignItems": "center",
"aspectRatio": 1,
"height": 0,
"justifyContent": "center",
"position": "absolute",
"right": 0,
"top": 0,
"transform": [
{
"translateX": 0,
},
{
"translateY": -0,
},
],
}
}
>
<View
onLayout={[Function]}
style={
{
"alignItems": "center",
"aspectRatio": 1,
"height": "50%",
"justifyContent": "center",
"maxHeight": 24,
"minHeight": 8,
"opacity": 0,
}
}
testID="badge-network"
>
<View
style={
{
"alignItems": "center",
"backgroundColor": "#FFFFFF",
"borderColor": "#FFFFFF",
"borderRadius": 8,
"borderWidth": 1,
"height": 16,
"justifyContent": "center",
"overflow": "hidden",
"shadowColor": "#0000001A",
"shadowOffset": {
"height": 2,
"width": 0,
},
"shadowOpacity": 1,
"shadowRadius": 4,
"transform": [
{
"scale": 1,
},
],
"width": 16,
}
}
>
<Image
onError={[Function]}
resizeMode="contain"
source={
{
"default": {
"uri": "MockImage",
},
}
}
style={
{
"height": 16,
"width": 16,
}
}
testID="network-avatar-image"
/>
</View>
</View>
</View>
</View>
</View>
<Text
accessibilityRole="text"
ellipsizeMode="tail"
Expand All @@ -206,7 +451,7 @@ exports[`TabThumbnail should render correctly 1`] = `
}
}
>
Undefined account - Ethereum Main Network
Account 2 - Ethereum Main Network
</Text>
</View>
</TouchableOpacity>
Expand Down
Loading

0 comments on commit c8d161e

Please sign in to comment.