Skip to content

Commit

Permalink
Merge branch 'main' into fix/default-network
Browse files Browse the repository at this point in the history
  • Loading branch information
kanthesha authored Jul 30, 2024
2 parents 2d801c1 + c00a309 commit ff25c0b
Show file tree
Hide file tree
Showing 16 changed files with 361 additions and 33 deletions.
21 changes: 21 additions & 0 deletions packages/accounts-controller/src/AccountsController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2331,6 +2331,27 @@ describe('AccountsController', () => {
});
});

describe('updateAccountMetadata', () => {
it('updates the metadata of an existing account', () => {
const { accountsController } = setupAccountsController({
initialState: {
internalAccounts: {
accounts: { [mockAccount.id]: mockAccount },
selectedAccount: mockAccount.id,
},
},
});
accountsController.updateAccountMetadata(mockAccount.id, {
lastSelected: 1,
});

expect(
accountsController.getAccountExpect(mockAccount.id).metadata
.lastSelected,
).toBe(1);
});
});

describe('#getNextAccountNumber', () => {
// Account names start at 2 since have 1 HD account + 2 simple keypair accounts (and both
// those keyring types are "grouped" together)
Expand Down
34 changes: 30 additions & 4 deletions packages/accounts-controller/src/AccountsController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,11 @@ export type AccountsControllerGetAccountAction = {
handler: AccountsController['getAccount'];
};

export type AccountsControllerUpdateAccountMetadata = {
type: `${typeof controllerName}:updateAccountMetadata`;
handler: AccountsController['updateAccountMetadata'];
};

export type AllowedActions =
| KeyringControllerGetKeyringForAccountAction
| KeyringControllerGetKeyringsByTypeAction
Expand All @@ -122,7 +127,8 @@ export type AccountsControllerActions =
| AccountsControllerGetSelectedAccountAction
| AccountsControllerGetNextAvailableAccountNameAction
| AccountsControllerGetAccountAction
| AccountsControllerGetSelectedMultichainAccountAction;
| AccountsControllerGetSelectedMultichainAccountAction
| AccountsControllerUpdateAccountMetadata;

export type AccountsControllerChangeEvent = ControllerStateChangeEvent<
typeof controllerName,
Expand Down Expand Up @@ -406,8 +412,6 @@ export class AccountsController extends BaseController<
* @throws An error if an account with the same name already exists.
*/
setAccountName(accountId: string, accountName: string): void {
const account = this.getAccountExpect(accountId);

if (
this.listMultichainAccounts().find(
(internalAccount) =>
Expand All @@ -418,12 +422,29 @@ export class AccountsController extends BaseController<
throw new Error('Account name already exists');
}

this.updateAccountMetadata(accountId, { name: accountName });
}

/**
* Updates the metadata of the account with the given ID.
* Use {@link setAccountName} if you only need to update the name of the account.
*
* @param accountId - The ID of the account for which the metadata will be updated.
* @param metadata - The new metadata for the account.
*/
updateAccountMetadata(
accountId: string,
metadata: Partial<InternalAccount['metadata']>,
): void {
const account = this.getAccountExpect(accountId);

this.update((currentState: Draft<AccountsControllerState>) => {
const internalAccount = {
...account,
metadata: { ...account.metadata, name: accountName },
metadata: { ...account.metadata, ...metadata },
};
// Do not remove this comment - This error is flaky: Comment out or restore the `ts-expect-error` directive below as needed.
// See: https://github.com/MetaMask/utils/issues/168
// // @ts-expect-error Known issue - `Json` causes recursive error in immer `Draft`/`WritableDraft` types
currentState.internalAccounts.accounts[accountId] = internalAccount;
});
Expand Down Expand Up @@ -1093,5 +1114,10 @@ export class AccountsController extends BaseController<
`AccountsController:getAccount`,
this.getAccount.bind(this),
);

this.messagingSystem.registerActionHandler(
`AccountsController:updateAccountMetadata`,
this.updateAccountMetadata.bind(this),
);
}
}
5 changes: 2 additions & 3 deletions packages/assets-controllers/src/TokensController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export class TokensController extends BaseController<

#selectedAccountId: string;

#provider: Provider | undefined;
#provider: Provider;

#abortController: AbortController;

Expand All @@ -209,7 +209,7 @@ export class TokensController extends BaseController<
messenger,
}: {
chainId: Hex;
provider: Provider | undefined;
provider: Provider;
state?: Partial<TokensControllerState>;
messenger: TokensControllerMessenger;
}) {
Expand Down Expand Up @@ -750,7 +750,6 @@ export class TokensController extends BaseController<

#getProvider(networkClientId?: NetworkClientId): Web3Provider {
return new Web3Provider(
// @ts-expect-error TODO: remove this annotation once the `Eip1193Provider` class is released
networkClientId
? this.messagingSystem.call(
'NetworkController:getNetworkClientById',
Expand Down
13 changes: 12 additions & 1 deletion packages/keyring-controller/src/KeyringController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ export type KeyringControllerSignUserOperationAction = {
handler: KeyringController['signUserOperation'];
};

export type KeyringControllerAddNewAccountAction = {
type: `${typeof name}:addNewAccount`;
handler: KeyringController['addNewAccount'];
};

export type KeyringControllerStateChangeEvent = {
type: `${typeof name}:stateChange`;
payload: [KeyringControllerState, Patch[]];
Expand Down Expand Up @@ -212,7 +217,8 @@ export type KeyringControllerActions =
| KeyringControllerPersistAllKeyringsAction
| KeyringControllerPrepareUserOperationAction
| KeyringControllerPatchUserOperationAction
| KeyringControllerSignUserOperationAction;
| KeyringControllerSignUserOperationAction
| KeyringControllerAddNewAccountAction;

export type KeyringControllerEvents =
| KeyringControllerStateChangeEvent
Expand Down Expand Up @@ -1796,6 +1802,11 @@ export class KeyringController extends BaseController<
`${name}:signUserOperation`,
this.signUserOperation.bind(this),
);

this.messagingSystem.registerActionHandler(
`${name}:addNewAccount`,
this.addNewAccount.bind(this),
);
}

/**
Expand Down
4 changes: 4 additions & 0 deletions packages/network-controller/src/NetworkController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,10 @@ export class NetworkController extends BaseController<
);
}

if (networkConfigurationId === this.state.selectedNetworkClientId) {
throw new Error(`The selected network configuration cannot be removed`);
}

const autoManagedNetworkClientRegistry =
this.#ensureAutoManagedNetworkClientRegistryPopulated();
const networkClientId = networkConfigurationId;
Expand Down
23 changes: 23 additions & 0 deletions packages/network-controller/tests/NetworkController.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3766,6 +3766,29 @@ describe('NetworkController', () => {
},
);
});

it('throws an error if the given ID corresponds to the selected network', async () => {
await withController(
{
state: {
networkConfigurations: {
'AAAA-AAAA-AAAA-AAAA': {
rpcUrl: 'https://test.network',
ticker: 'TICKER',
chainId: toHex(111),
id: 'AAAA-AAAA-AAAA-AAAA',
},
},
selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA',
},
},
async ({ controller }) => {
expect(() =>
controller.removeNetworkConfiguration('AAAA-AAAA-AAAA-AAAA'),
).toThrow('The selected network configuration cannot be removed');
},
);
});
});

describe('given an ID that does not identify a network configuration in state', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ import * as OnChainNotifications from './services/onchain-notifications';
import type { UserStorage } from './types/user-storage/user-storage';
import * as Utils from './utils/utils';

// Mock type used for testing purposes
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type MockVar = any;

const featureAnnouncementsEnv = {
spaceId: ':space_id',
accessToken: ':access_token',
Expand Down Expand Up @@ -664,6 +668,7 @@ function mockNotificationMessenger() {
name: 'NotificationServicesController',
allowedActions: [
'KeyringController:getAccounts',
'KeyringController:getState',
'AuthenticationController:getBearerToken',
'AuthenticationController:isSignedIn',
'NotificationServicesPushController:disablePushNotifications',
Expand All @@ -676,6 +681,8 @@ function mockNotificationMessenger() {
],
allowedEvents: [
'KeyringController:stateChange',
'KeyringController:lock',
'KeyringController:unlock',
'NotificationServicesPushController:onNewNotifications',
],
});
Expand Down Expand Up @@ -729,6 +736,10 @@ function mockNotificationMessenger() {
return mockListAccounts();
}

if (actionType === 'KeyringController:getState') {
return { isUnlocked: true } as MockVar;
}

if (actionType === 'AuthenticationController:getBearerToken') {
return mockGetBearerToken();
}
Expand Down Expand Up @@ -774,12 +785,9 @@ function mockNotificationMessenger() {
return mockPerformSetStorage(params[0], params[1]);
}

const exhaustedMessengerMocks = (action: never) => {
return new Error(
`MOCK_FAIL - unsupported messenger call: ${action as string}`,
);
};
throw exhaustedMessengerMocks(actionType);
throw new Error(
`MOCK_FAIL - unsupported messenger call: ${actionType as string}`,
);
});

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import { toChecksumHexAddress } from '@metamask/controller-utils';
import type {
KeyringControllerGetAccountsAction,
KeyringControllerStateChangeEvent,
KeyringControllerGetStateAction,
KeyringControllerLockEvent,
KeyringControllerUnlockEvent,
} from '@metamask/keyring-controller';
import type {
AuthenticationController,
Expand Down Expand Up @@ -192,6 +195,7 @@ export type Actions =
export type AllowedActions =
// Keyring Controller Requests
| KeyringControllerGetAccountsAction
| KeyringControllerGetStateAction
// Auth Controller Requests
| AuthenticationController.AuthenticationControllerGetBearerToken
| AuthenticationController.AuthenticationControllerIsSignedIn
Expand All @@ -214,7 +218,11 @@ export type NotificationServicesControllerMessengerEvents =

// Allowed Events
export type AllowedEvents =
// Keyring Events
| KeyringControllerStateChangeEvent
| KeyringControllerLockEvent
| KeyringControllerUnlockEvent
// Push Notification Events
| NotificationServicesPushControllerOnNewNotification;

// Type for the messenger of NotificationServicesController
Expand Down Expand Up @@ -244,6 +252,34 @@ export default class NotificationServicesController extends BaseController<
// Temporary boolean as push notifications are not yet enabled on mobile
#isPushIntegrated = true;

// Flag to check is notifications have been setup when the browser/extension is initialized.
// We want to re-initialize push notifications when the browser/extension is refreshed
// To ensure we subscribe to the most up-to-date notifications
#isPushNotificationsSetup = false;

#isUnlocked = false;

#keyringController = {
setupLockedStateSubscriptions: (onUnlock: () => Promise<void>) => {
const { isUnlocked } = this.messagingSystem.call(
'KeyringController:getState',
);
this.#isUnlocked = isUnlocked;

this.messagingSystem.subscribe('KeyringController:unlock', () => {
this.#isUnlocked = true;
// messaging system cannot await promises
// we don't need to wait for a result on this.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
onUnlock();
});

this.messagingSystem.subscribe('KeyringController:lock', () => {
this.#isUnlocked = false;
});
},
};

#auth = {
getBearerToken: async () => {
return await this.messagingSystem.call(
Expand Down Expand Up @@ -338,6 +374,12 @@ export default class NotificationServicesController extends BaseController<
if (!this.state.isNotificationServicesEnabled) {
return;
}
if (this.#isPushNotificationsSetup) {
return;
}
if (!this.#isUnlocked) {
return;
}

const storage = await this.#getUserStorage();
if (!storage) {
Expand All @@ -346,6 +388,7 @@ export default class NotificationServicesController extends BaseController<

const uuids = Utils.getAllUUIDs(storage);
await this.#pushNotifications.enablePushNotifications(uuids);
this.#isPushNotificationsSetup = true;
},
};

Expand Down Expand Up @@ -463,10 +506,13 @@ export default class NotificationServicesController extends BaseController<
});

this.#isPushIntegrated = env.isPushIntegrated ?? true;

this.#featureAnnouncementEnv = env.featureAnnouncements;
this.#registerMessageHandlers();
this.#clearLoadingStates();

this.#keyringController.setupLockedStateSubscriptions(
this.#pushNotifications.initializePushNotifications,
);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this.#accounts.initialize();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Expand Down
2 changes: 2 additions & 0 deletions packages/profile-sync-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"devDependencies": {
"@lavamoat/allow-scripts": "^3.0.4",
"@metamask/auto-changelog": "^3.4.4",
"@metamask/keyring-controller": "^17.1.2",
"@metamask/snaps-controllers": "^9.3.1",
"@types/jest": "^27.4.1",
"deepmerge": "^4.2.2",
Expand All @@ -66,6 +67,7 @@
"typescript": "~5.0.4"
},
"peerDependencies": {
"@metamask/keyring-controller": "^17.0.0",
"@metamask/snaps-controllers": "^9.3.0"
},
"engines": {
Expand Down
Loading

0 comments on commit ff25c0b

Please sign in to comment.