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.

Sign up for GitHub

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

Integrate KeyringController v10 #17056

Merged
merged 9 commits into from
Jan 20, 2023
Merged

Integrate KeyringController v10 #17056

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0ce2727
pulling in latest eth-keyring-controller and @metamask/scure-bip39
adonesky1 Aug 24, 2022
b822771
update policy files
adonesky1 Dec 22, 2022
8e0a03f
cleanup
adonesky1 Dec 22, 2022
48a3dc3
fix test
adonesky1 Dec 23, 2022
d2bde20
fix + add test
adonesky1 Jan 6, 2023
5c49f55
pull in @metamask/eth-keyring-controller
adonesky1 Jan 18, 2023
f886657
fix rebase
adonesky1 Jan 18, 2023
187e701
cleanup/revert
adonesky1 Jan 18, 2023
8cc60b2
cleanup
adonesky1 Jan 18, 2023
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
7 changes: 4 additions & 3 deletions app/scripts/lib/seed-phrase-verifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import KeyringController from 'eth-keyring-controller';
import { KeyringController } from '@metamask/eth-keyring-controller';
import log from 'loglevel';

import { HardwareKeyringTypes } from '../../../shared/constants/hardware-wallets';
Expand All @@ -22,15 +22,16 @@ const seedPhraseVerifier = {
}

const keyringController = new KeyringController({});
const Keyring = keyringController.getKeyringClassForType(
const keyringBuilder = keyringController.getKeyringBuilderForType(
HardwareKeyringTypes.hdKeyTree,
);
const keyring = keyringBuilder();
const opts = {
mnemonic: seedPhrase,
numberOfAccounts: createdAccounts.length,
};

const keyring = new Keyring(opts);
await keyring.deserialize(opts);
const restoredAccounts = await keyring.getAccounts();
log.debug(`Created accounts: ${JSON.stringify(createdAccounts)}`);
log.debug(`Restored accounts: ${JSON.stringify(restoredAccounts)}`);
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/lib/seed-phrase-verifier.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* https://github.com/facebook/jest/issues/7780
*/
import { cloneDeep } from 'lodash';
import KeyringController from 'eth-keyring-controller';
import { KeyringController } from '@metamask/eth-keyring-controller';
import firstTimeState from '../first-time-state';
import mockEncryptor from '../../../test/lib/mock-encryptor';
import { HardwareKeyringTypes } from '../../../shared/constants/hardware-wallets';
Expand Down
21 changes: 17 additions & 4 deletions app/scripts/metamask-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { JsonRpcEngine } from 'json-rpc-engine';
import { debounce } from 'lodash';
import createEngineStream from 'json-rpc-middleware-stream/engineStream';
import { providerAsMiddleware } from 'eth-json-rpc-middleware';
import KeyringController from 'eth-keyring-controller';
import {
KeyringController,
keyringBuilderFactory,
} from '@metamask/eth-keyring-controller';
import {
errorCodes as rpcErrorCodes,
EthereumRpcError,
Expand Down Expand Up @@ -57,6 +60,8 @@ import {
} from '@metamask/snaps-controllers';
///: END:ONLY_INCLUDE_IN

import { wordlist as englishWordlist } from '@metamask/scure-bip39/dist/wordlists/english';

import browser from 'webextension-polyfill';
import {
AssetType,
Expand Down Expand Up @@ -641,15 +646,19 @@ export default class MetamaskController extends EventEmitter {

let additionalKeyrings = [];
if (!isManifestV3) {
additionalKeyrings = [
const additionalKeyringTypes = [
TrezorKeyring,
LedgerBridgeKeyring,
LatticeKeyring,
QRHardwareKeyring,
];
additionalKeyrings = additionalKeyringTypes.map((keyringType) =>
keyringBuilderFactory(keyringType),
);
Comment on lines +649 to +657
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The KeyringController constructor now takes keyring builder functions rather than classes.

keyringBuilderFactory takes the class types and makes them into these builder functions.

}

this.keyringController = new KeyringController({
keyringTypes: additionalKeyrings,
keyringBuilders: additionalKeyrings,
initState: initState.KeyringController,
encryptor: opts.encryptor || undefined,
cacheEncryptionKey: isManifestV3,
Expand Down Expand Up @@ -2568,7 +2577,11 @@ export default class MetamaskController extends EventEmitter {
if (!keyring.mnemonic) {
throw new Error('Primary keyring mnemonic unavailable.');
}
return keyring.mnemonic;

const recoveredIndices = Array.from(
new Uint16Array(new Uint8Array(keyring.mnemonic).buffer),
);
return recoveredIndices.map((i) => englishWordlist[i]).join(' ');
Copy link
Contributor Author

@adonesky1 adonesky1 Jan 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

getPrimaryKeyringMnemonic is used by Snaps which still passes mnemonics around as strings. They will be refactoring to pass mnemonics as Uint8Arrays but until then we must recover and return the string version from the Uint8Array format that is now on the keyring value returned by the KeyringController as of this latest version bump.

}

//
Expand Down
50 changes: 37 additions & 13 deletions app/scripts/metamask-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@ import { strict as assert } from 'assert';
import sinon from 'sinon';
import { cloneDeep } from 'lodash';
import nock from 'nock';
import { pubToAddress, bufferToHex } from 'ethereumjs-util';
import { obj as createThoughStream } from 'through2';
import EthQuery from 'eth-query';
import proxyquire from 'proxyquire';
import browser from 'webextension-polyfill';
import { wordlist as englishWordlist } from '@metamask/scure-bip39/dist/wordlists/english';
import { TransactionStatus } from '../../shared/constants/transaction';
import createTxMeta from '../../test/lib/createTxMeta';
import { NETWORK_TYPES } from '../../shared/constants/network';
import {
HardwareDeviceNames,
HardwareKeyringTypes,
} from '../../shared/constants/hardware-wallets';
import { addHexPrefix, deferredPromise } from './lib/util';
import { deferredPromise } from './lib/util';

const Ganache = require('../../test/e2e/ganache');

Expand Down Expand Up @@ -209,13 +209,15 @@ describe('MetaMaskController', function () {
metamaskController.keyringController.getKeyringsByType(
HardwareKeyringTypes.imported,
);
const privKeyBuffer = simpleKeyrings[0].wallets[0].privateKey;
const pubKeyBuffer = simpleKeyrings[0].wallets[0].publicKey;
const addressBuffer = pubToAddress(pubKeyBuffer);
const privKey = bufferToHex(privKeyBuffer);
const pubKey = bufferToHex(addressBuffer);
assert.equal(privKey, addHexPrefix(importPrivkey));
assert.equal(pubKey, '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc');
const pubAddressHexArr = await simpleKeyrings[0].getAccounts();
const privKeyHex = await simpleKeyrings[0].exportAccount(
pubAddressHexArr[0],
);
Comment on lines +212 to +215
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the new pattern for getting public and private keys for an account.

assert.equal(privKeyHex, importPrivkey);
assert.equal(
pubAddressHexArr[0],
'0xe18035bf8712672935fdb4e5e431b1a0183d2dfc',
);
});

it('adds 1 account', async function () {
Expand Down Expand Up @@ -511,6 +513,31 @@ describe('MetaMaskController', function () {
});
});

describe('getPrimaryKeyringMnemonic', function () {
it('should return a mnemonic as a string', function () {
const mockMnemonic =
'above mercy benefit hospital call oval domain student sphere interest argue shock';
const mnemonicIndices = mockMnemonic
.split(' ')
.map((word) => englishWordlist.indexOf(word));
const uint8ArrayMnemonic = new Uint8Array(
new Uint16Array(mnemonicIndices).buffer,
);

const mockHDKeyring = {
type: 'HD Key Tree',
mnemonic: uint8ArrayMnemonic,
};
sinon
.stub(metamaskController.keyringController, 'getKeyringsByType')
.returns([mockHDKeyring]);
Comment on lines +518 to +533
Copy link
Contributor Author

@adonesky1 adonesky1 Jan 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This mocks the way mnemonics are stored on HD keyrings - as Uint8Arrays of the indices of the words relative to the english wordlist - as of MetaMask/eth-hd-keyring#67. This test then demonstrates that even though the mnemonic is stored in this way, the getPrimaryKeyringMnemonic method on MetamaskController returns the mnemonic in it's string form, since this is required by Snaps for now.


const recoveredMnemonic = metamaskController.getPrimaryKeyringMnemonic();

assert.equal(recoveredMnemonic, mockMnemonic);
});
});

describe('checkHardwareStatus', function () {
it('should throw if it receives an unknown device name', async function () {
try {
Expand Down Expand Up @@ -691,11 +718,8 @@ describe('MetaMaskController', function () {
}
});

beforeEach(async function () {
await metamaskController.createNewVaultAndKeychain('password');
});

it('#addNewAccount', async function () {
await metamaskController.createNewVaultAndKeychain('password');
await metamaskController.addNewAccount(1);
const getAccounts =
await metamaskController.keyringController.getAccounts();
Expand Down
Loading