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: bump keyring controller 7.5.0 #8035

Merged
merged 24 commits into from
Jan 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f19a8ca
feat: bump @metamask/keyring-controller to v7.5.0
gantunesr Dec 7, 2023
7789cf0
chore: update keyring-controller patch
gantunesr Dec 7, 2023
d63f79c
fix: update keyring-controller constructor
gantunesr Dec 7, 2023
55add8e
chore: update data type from string to Hex
gantunesr Dec 8, 2023
f195f33
test: update keyring-controller mock state
gantunesr Dec 8, 2023
be50074
feat: udpate ledger core methods to not use ethKeyringController dire…
gantunesr Dec 8, 2023
522b894
test: update initial background state
gantunesr Dec 9, 2023
63bd755
Merge branch 'main' of https://github.com/MetaMask/metamask-mobile in…
gantunesr Dec 14, 2023
141a001
chore: update keyring-controller patch
gantunesr Dec 18, 2023
aee0a66
chore: revert core ledger
gantunesr Dec 18, 2023
b873c19
test: fix keyring-controller initial state
gantunesr Dec 18, 2023
4d04a35
test: fix keyring-controller initial state
gantunesr Dec 18, 2023
f81c657
Merge branch 'main' into feat/bump-keyring-controller-7.5.0
gantunesr Dec 18, 2023
9c28c84
chore: dedupe dependencies
gantunesr Dec 18, 2023
4cfb49b
Merge branch 'main' of https://github.com/MetaMask/metamask-mobile in…
gantunesr Dec 18, 2023
494b0e2
chore: deduplicate dependencies
gantunesr Dec 18, 2023
85cc326
chroe: udpate keyring-controller patch
gantunesr Dec 18, 2023
74f2806
chore: remove references to eth-keyring-controller
gantunesr Dec 19, 2023
f89cf0b
chore: lint
gantunesr Dec 21, 2023
647ab69
Merge branch 'main' of https://github.com/MetaMask/metamask-mobile in…
gantunesr Dec 21, 2023
379b483
Merge branch 'main' into feat/bump-keyring-controller-7.5.0
gantunesr Jan 4, 2024
840658e
chore: update KeyringController construction params
gantunesr Jan 4, 2024
315fe04
fix: use setSelectedAddress from PreferencesController instead of Key…
gantunesr Jan 4, 2024
28f05a6
Merge branch 'feat/bump-keyring-controller-7.5.0' of https://github.c…
gantunesr Jan 4, 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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Alert, ListRenderItem, Platform, View } from 'react-native';
import { FlatList } from 'react-native-gesture-handler';
import { useSelector } from 'react-redux';
import { KeyringTypes } from '@metamask/keyring-controller';
import type { Hex } from '@metamask/utils';

// External dependencies.
import Cell, {
Expand Down Expand Up @@ -80,7 +81,7 @@ const AccountSelectorList = ({
isSelected,
index,
}: {
address: string;
address: Hex;
imported: boolean;
isSelected: boolean;
index: number;
Expand Down
1 change: 0 additions & 1 deletion app/core/BackupVault/backupVault.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ describe('backupVault', () => {
vault: undefined,
keyrings: [],
isUnlocked: false,
keyringTypes: [],
};
const response = await backupVault(keyringState);
expect(response.success).toBe(false);
Expand Down
8 changes: 8 additions & 0 deletions app/core/Engine.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ describe('Engine', () => {
},
},
},
KeyringController: {
...backgroundState.KeyringController,
vault: {
cipher: 'mock-cipher',
iv: 'mock-iv',
lib: 'original',
},
},
};

expect(backgroundState).toStrictEqual(initialState);
Expand Down
1 change: 1 addition & 0 deletions app/core/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,7 @@ class Engine {
allowedActions: ['KeyringController:getState'],
}),
state: initialKeyringState || initialState.KeyringController,
// @ts-expect-error To Do: Update the type of QRHardwareKeyring to Keyring<Json>
keyringBuilders: [qrKeyringBuilder],
});

Expand Down
75 changes: 23 additions & 52 deletions app/core/Ledger/Ledger.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import Engine from '../Engine';
import {
KeyringController,
SignTypedDataVersion,
} from '@metamask/keyring-controller';
import { SignTypedDataVersion } from '@metamask/keyring-controller';
import ExtendedKeyringTypes from '../../constants/keyringTypes';

// Mock interface for the SerializationOptions
interface SerializationOptions {
vault: any;
keyrings: any[];
Expand All @@ -13,6 +11,7 @@ interface SerializationOptions {
encryptionSalt: string;
}

// Mock interface for the LedgerKeyring
interface LedgerKeyring {
setTransport: (transport: any, deviceId: string) => void;
getAppAndVersion: () => Promise<{ appName: string }>;
Expand All @@ -25,52 +24,27 @@ interface LedgerKeyring {
getName: () => string;
}

/**
* Get EthKeyringController from KeyringController
*
* @returns The EthKeyringController
*/
const getEthKeyringController = () => {
const keyringController: KeyringController = Engine.context.KeyringController;
return keyringController.getEthKeyringController();
};

/**
* A method replicate #fullupdate from the keyring controller.
*/
export const syncKeyringState = async (): Promise<void> => {
const keyringController = Engine.context.KeyringController;
const ethKeyringController = getEthKeyringController();
const { vault } = ethKeyringController.store.getState();
const { keyrings, isUnlocked, encryptionKey, encryptionSalt } =
ethKeyringController.memStore.getState();

keyringController.update(() => ({
vault,
keyrings,
isUnlocked,
encryptionKey,
encryptionSalt,
}));
};

/**
* Add LedgerKeyring.
*
* @returns The Ledger Keyring
*/
export const addLedgerKeyring = async (): Promise<LedgerKeyring> =>
(await getEthKeyringController().addNewKeyring(
export const addLedgerKeyring = async (): Promise<LedgerKeyring> => {
const keyringController = Engine.context.KeyringController;
return (await keyringController.addNewKeyring(
ExtendedKeyringTypes.ledger,
)) as unknown as LedgerKeyring;
};

/**
* Retrieve the existing LedgerKeyring or create a new one.
*
* @returns The stored Ledger Keyring
*/
export const getLedgerKeyring = async (): Promise<LedgerKeyring> => {
const keyring = getEthKeyringController().getKeyringsByType(
const keyringController = Engine.context.KeyringController;
// There should only be one ledger keyring.
const keyring = keyringController().getKeyringsByType(
ExtendedKeyringTypes.ledger,
)[0] as unknown as LedgerKeyring;

Expand All @@ -90,11 +64,9 @@ export const restoreLedgerKeyring = async (
keyringSerialized: SerializationOptions,
): Promise<void> => {
const keyringController = Engine.context.KeyringController;
const ethKeyringController = getEthKeyringController();

(await getLedgerKeyring()).deserialize(keyringSerialized);
keyringController.updateIdentities(await ethKeyringController.getAccounts());
await syncKeyringState();
keyringController.updateIdentities(await keyringController.getAccounts());
};

/**
Expand Down Expand Up @@ -124,11 +96,11 @@ export const unlockLedgerDefaultAccount = async (): Promise<{
balance: string;
}> => {
const keyringController = Engine.context.KeyringController;
const ethKeyringController = getEthKeyringController();
const preferencesController = Engine.context.PreferencesController;
const keyring = await getLedgerKeyring();
const oldAccounts = await ethKeyringController.getAccounts();
await ethKeyringController.addNewAccount(keyring);
const newAccounts = await ethKeyringController.getAccounts();
const oldAccounts = await keyringController.getAccounts();
await keyringController.addNewAccountForKeyring(keyring);
const newAccounts = await keyringController.getAccounts();

keyringController.updateIdentities(newAccounts);
newAccounts.forEach((address: string) => {
Expand All @@ -137,11 +109,10 @@ export const unlockLedgerDefaultAccount = async (): Promise<{
// The first ledger account is always returned.
keyringController.setAccountLabel(address, `${keyring.getName()} 1`);
}
keyringController.setSelectedAddress(address);
preferencesController.setSelectedAddress(address);
}
});
await ethKeyringController.persistAllKeyrings();
await syncKeyringState();
await keyringController.persistAllKeyrings();

const address = await keyring.getDefaultAccount();
return {
Expand Down Expand Up @@ -171,15 +142,14 @@ export const closeRunningAppOnLedger = async (): Promise<void> => {
*/
export const forgetLedger = async (): Promise<void> => {
const keyringController = Engine.context.KeyringController;
const ethKeyringController = getEthKeyringController();
const preferencesController = Engine.context.PreferencesController;
const keyring = await getLedgerKeyring();
keyring.forgetDevice();

const accounts: string[] = await ethKeyringController.getAccounts();
keyringController.setSelectedAddress(accounts[0]);
const accounts: string[] = await keyringController.getAccounts();
preferencesController.setSelectedAddress(accounts[0]);

await ethKeyringController.persistAllKeyrings();
await syncKeyringState();
await keyringController.persistAllKeyrings();
};

/**
Expand All @@ -205,7 +175,8 @@ export const ledgerSignTypedMessage = async (
version: SignTypedDataVersion,
): Promise<string> => {
await getLedgerKeyring();
return await getEthKeyringController().signTypedMessage(
const keyringController = Engine.context.KeyringController;
return await keyringController.signTypedMessage(
{
from: messageParams.from,
data: messageParams.data as
Expand Down
8 changes: 6 additions & 2 deletions app/util/test/initial-background-state.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
{
"KeyringController": {
"isUnlocked": false,
"keyringTypes": [],
"keyrings": []
"keyrings": [],
"vault": {
"cipher": "mock-cipher",
"iv": "mock-iv",
"lib": "original"
}
},
"LoggingController": {
"logs": {}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@
"@metamask/etherscan-link": "^2.0.0",
"@metamask/gas-fee-controller": "^5.0.0",
"@metamask/key-tree": "^9.0.0",
"@metamask/keyring-controller": "^6.0.0",
"@metamask/keyring-controller": "^7.5.0",
"@metamask/logging-controller": "^1.0.1",
"@metamask/network-controller": "^8.0.0",
"@metamask/permission-controller": "^4.0.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
diff --git a/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts b/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts
index dff777b..a61b5ed 100644
index a05f2a9..1291081 100644
--- a/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts
+++ b/node_modules/@metamask/keyring-controller/dist/KeyringController.d.ts
@@ -1,3 +1,4 @@
@@ -1,4 +1,5 @@
import type { TxData, TypedTransaction } from '@ethereumjs/tx';
+import { KeyringController as EthKeyringController } from '@metamask/eth-keyring-controller';
import { MetaMaskKeyring as QRKeyring, IKeyringState as IQRKeyringState } from '@keystonehq/metamask-airgapped-keyring';
import { BaseControllerV2, RestrictedControllerMessenger } from '@metamask/base-controller';
import { PreferencesController } from '@metamask/preferences-controller';
@@ -141,6 +142,18 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
import { type MetaMaskKeyring as QRKeyring, type IKeyringState as IQRKeyringState } from '@keystonehq/metamask-airgapped-keyring';
import type { RestrictedControllerMessenger } from '@metamask/base-controller';
import { BaseControllerV2 } from '@metamask/base-controller';
@@ -167,6 +168,19 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
* @param opts.state - Initial state to set on this controller.
*/
constructor({ removeIdentity, syncIdentities, updateIdentities, setSelectedAddress, setAccountLabel, encryptor, keyringBuilders, cacheEncryptionKey, messenger, state, }: KeyringControllerOptions);
Expand All @@ -20,16 +21,17 @@ index dff777b..a61b5ed 100644
+ * ===============================================================================
+ */
+ /**
+ * Gets the internal keyring controller.
+ */
+ * Gets the internal keyring controller.
+ */
+ getEthKeyringController(): typeof EthKeyringController;
+
/**
* Adds a new account to the default (first) HD seed phrase keyring.
*
@@ -342,7 +355,10 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
@@ -407,7 +421,10 @@ export declare class KeyringController extends BaseControllerV2<typeof name, Key
}[]>;
unlockQRHardwareWalletAccount(index: number): Promise<void>;
getAccountKeyringType(account: string): Promise<KeyringTypes>;
getAccountKeyringType(account: string): Promise<string>;
- forgetQRDevice(): Promise<void>;
+ forgetQRDevice(): Promise<{
+ removedAccounts: string[];
Expand All @@ -38,14 +40,16 @@ index dff777b..a61b5ed 100644
}
export default KeyringController;
//# sourceMappingURL=KeyringController.d.ts.map
\ No newline at end of file
diff --git a/node_modules/@metamask/keyring-controller/dist/KeyringController.js b/node_modules/@metamask/keyring-controller/dist/KeyringController.js
index c905cc0..16f35e4 100644
index c4dd1d0..ddc4155 100644
--- a/node_modules/@metamask/keyring-controller/dist/KeyringController.js
+++ b/node_modules/@metamask/keyring-controller/dist/KeyringController.js
@@ -139,6 +139,19 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
this.setSelectedAddress = setSelectedAddress;
@@ -154,6 +154,21 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
this.setAccountLabel = setAccountLabel;
__classPrivateFieldGet(this, _KeyringController_instances, "m", _KeyringController_registerMessageHandlers).call(this);
}
+
+ /**
+ * ============================== PATCH INFORMATION ==============================
+ * This patch was added for ledger integration. It adds a new method to the
Expand All @@ -54,25 +58,28 @@ index c905cc0..16f35e4 100644
+ * ===============================================================================
+ */
+ /**
+ * Gets the internal keyring controller.
+ */
+ * Gets the internal keyring controller.
+ */
+ getEthKeyringController() {
+ return __classPrivateFieldGet(this, _KeyringController_keyring, "f");
+ }
+
/**
* Adds a new account to the default (first) HD seed phrase keyring.
*
@@ -678,13 +691,21 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
@@ -745,13 +760,23 @@ class KeyringController extends base_controller_1.BaseControllerV2 {
}
forgetQRDevice() {
return __awaiter(this, void 0, void 0, function* () {
+ /**
+ * ============================== PATCH INFORMATION ==============================
+ * This patch addresses an issue regarding the forget device functionality. It
+ * improves the logic to correctly remove the QR accounts and update the
+ * identities as needed.
+ * ===============================================================================
+ */
+ * ============================== PATCH INFORMATION ==============================
+ * This patch addresses an issue regarding the forget device functionality. It
+ * improves the logic to correctly remove the QR accounts and update the
+ * identities as needed.
+ *
+ * Solved in https://github.com/MetaMask/core/pull/3641
+ * ===============================================================================
+ */
const keyring = yield this.getOrAddQRKeyring();
+ const allAccounts = (yield __classPrivateFieldGet(this, _KeyringController_keyring, "f").getAccounts());
keyring.forgetDevice();
Expand Down
Loading
Loading