Skip to content
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

feat(NOTIFY-1096): add account syncing #11190

Open
wants to merge 53 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
631dbe9
feat(NOTIFY-1096): add account syncing
mathieuartu Sep 13, 2024
04780e0
fix: import path
mathieuartu Sep 13, 2024
771ad50
fix: useEffect
mathieuartu Sep 13, 2024
2f20cdb
fix: temporarily remove keyring-controller patch
mathieuartu Sep 13, 2024
249a1ba
fix: yarn.lock
mathieuartu Sep 13, 2024
6e2b5b5
fix: remove Gemfile.lock
mathieuartu Sep 13, 2024
888f01d
fix: add @ts-expect-error for controller version mismatch
mathieuartu Sep 13, 2024
b9bb37a
fix: remove unused import
mathieuartu Sep 13, 2024
6c9b2ce
feat: group profile-sync effects under a layout effect + AppState cha…
mathieuartu Sep 13, 2024
80af81c
add layouteffect deps
mathieuartu Sep 13, 2024
ab17435
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 13, 2024
95563d9
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 16, 2024
c8e8e91
chore: remove preview packages and use latest deps
mathieuartu Sep 16, 2024
c3a3cb6
fix: yarn.lock
mathieuartu Sep 16, 2024
c0be57e
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 16, 2024
9aca646
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 17, 2024
8efc079
fix: update package.json & yarn.lock
mathieuartu Sep 17, 2024
8836b0f
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 17, 2024
970f65e
fix: update yarn.lock
mathieuartu Sep 18, 2024
63708ef
feat: add analytics
mathieuartu Sep 18, 2024
b6d891e
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 18, 2024
c821134
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 19, 2024
5d17b5d
fix: yarn.lock
mathieuartu Sep 19, 2024
b7af253
fix: bump snaps packages
mathieuartu Sep 19, 2024
91fa607
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 19, 2024
01d2304
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 19, 2024
a3fc2cc
fix: remove unused ts-error statements
mathieuartu Sep 19, 2024
a971856
fix: snaps imports
mathieuartu Sep 20, 2024
fd5615d
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 20, 2024
cdca605
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 20, 2024
92d9839
fix: yarn.lock
mathieuartu Sep 20, 2024
40e8294
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 23, 2024
c157056
fix: yarn.lock
mathieuartu Sep 23, 2024
5d708da
fix: yarn.lock
mathieuartu Sep 23, 2024
77c9c31
Merge branch 'main' into feat/add_account_syncing
mathieuartu Sep 25, 2024
0210192
fix: yarn.lock
mathieuartu Sep 25, 2024
2be206f
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 3, 2024
f5a2aa9
fix: bump profile-sync-controller version
mathieuartu Oct 3, 2024
1050e86
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 7, 2024
66aff4c
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 8, 2024
22cfc1a
fix: update analytics events names
mathieuartu Oct 9, 2024
bcfd1bb
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 9, 2024
307b449
Merge branch 'feat/add_account_syncing' of github.com:MetaMask/metama…
mathieuartu Oct 9, 2024
7a4dc64
fix: update yarn.lock
mathieuartu Oct 9, 2024
b03fd6a
fix: build issues
mathieuartu Oct 9, 2024
ddf4c9e
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 15, 2024
2fee02f
fix: update yarn.lock
mathieuartu Oct 15, 2024
0fa03b7
fix: yarn dedupe
mathieuartu Oct 15, 2024
730fb38
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 15, 2024
c953e4f
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 16, 2024
1ba5618
Merge branch 'main' into feat/add_account_syncing
mathieuartu Oct 16, 2024
5752f14
fix: add missing semi-colon
mathieuartu Oct 16, 2024
d859175
fix: move account syncing to its own hook file
mathieuartu Oct 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion app/actions/notification/helpers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,15 @@
return getErrorMessage(error);
}
};

export const syncInternalAccountsWithUserStorage = async () => {
try {
await Engine.context.UserStorageController.syncInternalAccountsWithUserStorage();
} catch (error) {
return getErrorMessage(error);
}
};

/**
* Perform the deletion of the notifications storage key and the creation of on chain triggers to reset the notifications.
*
Expand All @@ -187,4 +196,4 @@
} catch (error) {
return getErrorMessage(error);
}
};
}

Check warning on line 199 in app/actions/notification/helpers/index.ts

View workflow job for this annotation

GitHub Actions / scripts (lint)

Missing semicolon
mathieuartu marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 11 additions & 1 deletion app/components/Views/Wallet/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ import {
} from '../../../selectors/notifications';
import { ButtonVariants } from '../../../component-library/components/Buttons/Button';
import { useListNotifications } from '../../../util/notifications/hooks/useNotifications';
import { useAccountSyncing } from '../../../util/notifications/hooks/useProfileSyncing';

import { isObject } from 'lodash';

const createStyles = ({ colors, typography }: Theme) =>
Expand Down Expand Up @@ -169,6 +171,7 @@ const Wallet = ({
const appState = useRef(AppState.currentState);
const { navigate } = useNavigation();
const { listNotifications } = useListNotifications();
const { dispatchAccountSyncing } = useAccountSyncing();
const walletRef = useRef(null);
const theme = useTheme();
const { toastRef } = useContext(ToastContext);
Expand Down Expand Up @@ -443,13 +446,17 @@ const Wallet = ({
[navigation, providerConfig.chainId],
);

// Layout effect when component/view is visible
// - fetches notifications
// - dispatches account syncing
useLayoutEffect(() => {
const handleAppStateChange = (nextAppState: AppStateStatus) => {
if (
appState.current.match(/inactive|background/) &&
nextAppState === 'active'
) {
listNotifications();
dispatchAccountSyncing();
mathieuartu marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Can we test this effect in the unit tests?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a pain to locally test using jest for app/components/Views/Wallet/index.test.tsx, do you have a trick in mind so I can bypass this error?

Happy to explore that for both listNotifications and dispatchAccountSyncing in a subsequent PR as well. LMK

Capture d’écran 2024-10-16 à 11 29 24

}

appState.current = nextAppState;
Expand All @@ -459,11 +466,14 @@ const Wallet = ({
'change',
handleAppStateChange,
);

listNotifications();
dispatchAccountSyncing();

return () => {
subscription.remove();
};
}, [listNotifications]);
}, [listNotifications, dispatchAccountSyncing]);

useEffect(() => {
navigation.setOptions(
Expand Down
9 changes: 9 additions & 0 deletions app/core/Analytics/MetaMetrics.events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,9 @@ enum EVENT_NAME {
PRIMARY_CURRENCY_TOGGLE = 'primary_currency_toggle',
LOGIN_DOWNLOAD_LOGS = 'Download State Logs Button Clicked',

// Profile Syncing
ACCOUNTS_SYNC_ADDED = 'Accounts Sync Added',
ACCOUNTS_SYNC_NAME_UPDATED = 'Accounts Sync Name Updated',
// network
MULTI_RPC_MIGRATION_MODAL_ACCEPTED = 'multi_rpc_migration_modal_accepted',
}
Expand Down Expand Up @@ -854,6 +857,12 @@ const events = {
),
PRIMARY_CURRENCY_TOGGLE: generateOpt(EVENT_NAME.PRIMARY_CURRENCY_TOGGLE),
LOGIN_DOWNLOAD_LOGS: generateOpt(EVENT_NAME.LOGIN_DOWNLOAD_LOGS),

// Profile Syncing
ACCOUNTS_SYNC_ADDED: generateOpt(EVENT_NAME.ACCOUNTS_SYNC_ADDED),
ACCOUNTS_SYNC_NAME_UPDATED: generateOpt(
EVENT_NAME.ACCOUNTS_SYNC_NAME_UPDATED,
),
};

/**
Expand Down
31 changes: 27 additions & 4 deletions app/core/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ import {
} from '@metamask/snaps-controllers';

import { WebViewExecutionService } from '@metamask/snaps-controllers/react-native';
import { NotificationArgs } from '@metamask/snaps-rpc-methods/dist/types/restricted/notify';
import { NotificationParameters } from '@metamask/snaps-rpc-methods/dist/restricted/notify.cjs';
mathieuartu marked this conversation as resolved.
Show resolved Hide resolved
import { getSnapsWebViewPromise } from '../lib/snaps';
import {
buildSnapEndowmentSpecifications,
Expand Down Expand Up @@ -896,7 +896,7 @@ class Engine {
type,
requestData: { content, placeholder },
}),
showInAppNotification: (origin: string, args: NotificationArgs) => {
showInAppNotification: (origin: string, args: NotificationParameters) => {
Logger.log(
'Snaps/ showInAppNotification called with args: ',
args,
Expand Down Expand Up @@ -1213,26 +1213,49 @@ class Engine {

const userStorageController = new UserStorageController.Controller({
getMetaMetricsState: () => MetaMetrics.getInstance().isEnabled(),
env: {
isAccountSyncingEnabled: true,
},
config: {
accountSyncing: {
onAccountAdded: (profileId) => {
MetaMetrics.getInstance().trackEvent(
MetaMetricsEvents.ACCOUNTS_SYNC_ADDED,
{
profile_id: profileId,
},
);
},
onAccountNameUpdated: (profileId) => {
MetaMetrics.getInstance().trackEvent(
MetaMetricsEvents.ACCOUNTS_SYNC_NAME_UPDATED,
{
profile_id: profileId,
Comment on lines +1222 to +1233
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C: trackEvent called with legacy way has been deprecated recently. You will find out how to use the new MetricsEventBuilder in the code documentation

},
);
},
},
},
state: initialState.UserStorageController,
messenger: this.controllerMessenger.getRestricted({
name: 'UserStorageController',
allowedActions: [
'SnapController:handleRequest',
'KeyringController:getState',
'KeyringController:addNewAccount',
'AuthenticationController:getBearerToken',
'AuthenticationController:getSessionProfile',
'AuthenticationController:isSignedIn',
'AuthenticationController:performSignOut',
'AuthenticationController:performSignIn',
'NotificationServicesController:disableNotificationServices',
'NotificationServicesController:selectIsNotificationServicesEnabled',
'KeyringController:addNewAccount',
'AccountsController:listAccounts',
'AccountsController:updateAccountMetadata',
],
allowedEvents: [
'KeyringController:lock',
'KeyringController:unlock',
'KeyringController:lock',
'AccountsController:accountAdded',
'AccountsController:accountRenamed',
],
Expand Down
78 changes: 77 additions & 1 deletion app/util/notifications/hooks/useProfileSyncing.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Provider } from 'react-redux';
import createMockStore from 'redux-mock-store';
import * as Actions from '../../../actions/notification/helpers';
import initialRootState from '../../../util/test/initial-root-state';
import { useProfileSyncing } from './useProfileSyncing';
import { useAccountSyncing, useProfileSyncing } from './useProfileSyncing';

function arrangeStore() {
const store = createMockStore()(initialRootState);
Expand Down Expand Up @@ -172,3 +172,79 @@ describe('useDisableProfileSyncing', () => {
);
});
});

describe('useAccountSyncing', () => {
beforeEach(() => {
jest.clearAllMocks();
});

function arrangeHook() {
const store = arrangeStore();
const hook = renderHook(() => useAccountSyncing(), {
wrapper: ({ children }) => <Provider store={store}>{children}</Provider>,
});

return hook;
}

function arrangeActions() {
const syncInternalAccountsWithUserStorageAction = jest
.spyOn(Actions, 'syncInternalAccountsWithUserStorage')
.mockResolvedValue(undefined);

return {
syncInternalAccountsWithUserStorageAction,
};
}

it('dispatches account syncing and error as undefined', async () => {
const mockActions = arrangeActions();

const { result } = arrangeHook();
await act(async () => {
await result.current.dispatchAccountSyncing();
});

expect(
mockActions.syncInternalAccountsWithUserStorageAction,
).toHaveBeenCalledTimes(1);
expect(result.current.error).toBeUndefined();
});

it('sets error message when syncInternalAccountsWithUserStorageAction returns an error', async () => {
const mockActions = arrangeActions();
mockActions.syncInternalAccountsWithUserStorageAction.mockRejectedValueOnce(
new Error('MOCK - failed to sync internal account with user storage'),
);

const { result } = arrangeHook();
await act(async () => {
await result.current.dispatchAccountSyncing();
});

expect(
mockActions.syncInternalAccountsWithUserStorageAction,
).toHaveBeenCalledTimes(1);
expect(result.current.error).toBeDefined();
expect(result.current.error).toEqual(
'MOCK - failed to sync internal account with user storage',
);
});

it('sets error message when an error occurs during dispatchAccountSyncing', async () => {
const mockActions = arrangeActions();
mockActions.syncInternalAccountsWithUserStorageAction.mockRejectedValueOnce(
new Error('MOCK - failed to sync internal account with user storage'),
);

const { result } = arrangeHook();
await act(async () => {
await result.current.dispatchAccountSyncing();
});

expect(result.current.error).toBeDefined();
expect(result.current.error).toEqual(
'MOCK - failed to sync internal account with user storage',
);
});
});
30 changes: 30 additions & 0 deletions app/util/notifications/hooks/useProfileSyncing.ts
mathieuartu marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { getErrorMessage } from '../../../util/errorHandling';
import {
disableProfileSyncing as disableProfileSyncingAction,
enableProfileSyncing as enableProfileSyncingAction,
syncInternalAccountsWithUserStorage as syncInternalAccountsWithUserStorageAction,
} from '../../../actions/notification/helpers';

/**
Expand Down Expand Up @@ -56,3 +57,32 @@ export function useProfileSyncing(): ProfileSyncingReturn {

return { enableProfileSyncing, disableProfileSyncing, loading, error };
}

/**
* Custom hook to dispatch account syncing.
*
* @returns An object containing the `dispatchAccountSyncing` function, and error state.
*/
export const useAccountSyncing = () => {
const [error, setError] = useState<string>();

const dispatchAccountSyncing = useCallback(async () => {
setError(undefined);
try {
const errorMessage = await syncInternalAccountsWithUserStorageAction();
if (errorMessage) {
setError(getErrorMessage(errorMessage));
return errorMessage;
}
} catch (e) {
const errorMessage = getErrorMessage(e);
setError(errorMessage);
return errorMessage;
}
}, []);

return {
dispatchAccountSyncing,
error,
};
};
5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@
"**/xml2js": ">=0.5.0",
"react-native-level-fs/**/bl": "^1.2.3",
"react-native-level-fs/levelup/semver": "^5.7.2",
"@metamask/accounts-controller": "^18.1.0",
"@metamask/contract-metadata": "^2.1.0",
"@metamask/react-native-payments/validator": "^13.7.0",
"**/minimist": "1.2.6",
Expand Down Expand Up @@ -180,8 +179,8 @@
"@metamask/smart-transactions-controller": "^13.0.0",
"@metamask/snaps-controllers": "^9.8.0",
"@metamask/snaps-execution-environments": "^6.7.2",
"@metamask/snaps-rpc-methods": "^9.1.4",
"@metamask/snaps-sdk": "^6.5.0",
"@metamask/snaps-rpc-methods": "^11.1.1",
"@metamask/snaps-sdk": "^6.5.1",
"@metamask/snaps-utils": "^8.1.1",
"@metamask/swappable-obj-proxy": "^2.1.0",
"@metamask/swaps-controller": "^9.0.12",
Expand Down
Loading
Loading