From 8b95f54c1aa490567d13b8a8488f63e0e7d448e8 Mon Sep 17 00:00:00 2001 From: Michele Esposito Date: Thu, 17 Aug 2023 11:27:40 +0200 Subject: [PATCH] refactor: remove remaining eth-keyring-controller use --- .../metamask-controller.actions.test.js | 4 +- app/scripts/metamask-controller.js | 48 +++++++------------ app/scripts/metamask-controller.test.js | 40 +++++++++------- 3 files changed, 40 insertions(+), 52 deletions(-) diff --git a/app/scripts/metamask-controller.actions.test.js b/app/scripts/metamask-controller.actions.test.js index 8c015c9c5c63..bd9c966835cb 100644 --- a/app/scripts/metamask-controller.actions.test.js +++ b/app/scripts/metamask-controller.actions.test.js @@ -177,7 +177,7 @@ describe('MetaMaskController', function () { ]), Promise.resolve(1).then(() => { keyringControllerState1 = JSON.stringify( - metamaskController.keyringController.memStore.getState(), + metamaskController.coreKeyringController.state, ); metamaskController.importAccountWithStrategy('privateKey', [ importPrivkey, @@ -185,7 +185,7 @@ describe('MetaMaskController', function () { }), Promise.resolve(2).then(() => { keyringControllerState2 = JSON.stringify( - metamaskController.keyringController.memStore.getState(), + metamaskController.coreKeyringController.state, ); }), ]); diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 8daf37320605..ef4d3ac847f0 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -903,8 +903,8 @@ export default class MetamaskController extends EventEmitter { permissionSpecifications: { ...getPermissionSpecifications({ getIdentities, - getAllAccounts: this.keyringController.getAccounts.bind( - this.keyringController, + getAllAccounts: this.coreKeyringController.getAccounts.bind( + this.coreKeyringController, ), captureKeyringTypesWithMissingIdentities: ( identities = {}, @@ -1192,8 +1192,8 @@ export default class MetamaskController extends EventEmitter { this.networkController.state.providerConfig.chainId, preferencesStore: this.preferencesController.store, txHistoryLimit: 60, - signTransaction: this.keyringController.signTransaction.bind( - this.keyringController, + signTransaction: this.coreKeyringController.signTransaction.bind( + this.coreKeyringController, ), provider: this.provider, blockTracker: this.blockTracker, @@ -2160,7 +2160,7 @@ export default class MetamaskController extends EventEmitter { * @returns {object} status */ getState() { - const { vault } = this.keyringController.store.getState(); + const { vault } = this.coreKeyringController.state; const isInitialized = Boolean(vault); const flatState = this.memStore.getFlatState(); @@ -2788,7 +2788,7 @@ export default class MetamaskController extends EventEmitter { async exportAccount(address, password) { await this.verifyPassword(password); - return this.keyringController.exportAccount(address, password); + return this.coreKeyringController.exportAccount(password, address); } async getTokenStandardAndDetails(address, userAddress, tokenId) { @@ -2885,19 +2885,10 @@ export default class MetamaskController extends EventEmitter { async createNewVaultAndKeychain(password) { const releaseLock = await this.createVaultMutex.acquire(); try { - let vault; - const accounts = await this.keyringController.getAccounts(); - if (accounts.length > 0) { - vault = await this.keyringController.fullUpdate(); - } else { - vault = await this.keyringController.createNewVaultAndKeychain( - password, - ); - const addresses = await this.keyringController.getAccounts(); - this.preferencesController.setAddresses(addresses); - this.selectFirstIdentity(); - } - + const vault = await this.coreKeyringController.createNewVaultAndKeychain( + password, + ); + this.selectFirstIdentity(); return vault; } finally { releaseLock(); @@ -3123,7 +3114,7 @@ export default class MetamaskController extends EventEmitter { * @param {string} password - The user's password */ async verifyPassword(password) { - await this.keyringController.verifyPassword(password); + await this.coreKeyringController.verifyPassword(password); } /** @@ -3256,7 +3247,7 @@ export default class MetamaskController extends EventEmitter { // Merge with existing accounts // and make sure addresses are not repeated - const oldAccounts = await this.keyringController.getAccounts(); + const oldAccounts = await this.coreKeyringController.getAccounts(); const accountsToTrack = [ ...new Set( oldAccounts.concat(accounts.map((a) => a.address.toLowerCase())), @@ -3372,9 +3363,9 @@ export default class MetamaskController extends EventEmitter { const keyring = await this.getKeyringForDevice(deviceName, hdPath); keyring.setAccountToUnlock(index); - const oldAccounts = await this.keyringController.getAccounts(); + const oldAccounts = await this.coreKeyringController.getAccounts(); const keyState = await this.keyringController.addNewAccount(keyring); - const newAccounts = await this.keyringController.getAccounts(); + const newAccounts = await this.coreKeyringController.getAccounts(); this.preferencesController.setAddresses(newAccounts); newAccounts.forEach((address) => { if (!oldAccounts.includes(address)) { @@ -3542,15 +3533,8 @@ export default class MetamaskController extends EventEmitter { this.custodyController.removeAccount(address); ///: END:ONLY_INCLUDE_IN(build-mmi) - const keyring = await this.coreKeyringController.getKeyringForAccount( - address, - ); // Remove account from the keyring - await this.keyringController.removeAccount(address); - const updatedKeyringAccounts = keyring ? await keyring.getAccounts() : {}; - if (updatedKeyringAccounts?.length === 0) { - keyring.destroy?.(); - } + await this.coreKeyringController.removeAccount(address); return address; } @@ -4480,7 +4464,7 @@ export default class MetamaskController extends EventEmitter { * @returns {boolean} Whether the extension is unlocked. */ isUnlocked() { - return this.keyringController.memStore.getState().isUnlocked; + return this.coreKeyringController.state.isUnlocked; } //============================================================================= diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 20f496abaed0..2712e6662864 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -303,7 +303,7 @@ describe('MetaMaskController', function () { // add sinon method spies sandbox.spy( - metamaskController.keyringController, + metamaskController.coreKeyringController, 'createNewVaultAndKeychain', ); sandbox.spy( @@ -349,7 +349,7 @@ describe('MetaMaskController', function () { it('adds 1 account', async function () { const keyringAccounts = - await metamaskController.keyringController.getAccounts(); + await metamaskController.coreKeyringController.getAccounts(); assert.equal( keyringAccounts[keyringAccounts.length - 1], '0xe18035bf8712672935fdb4e5e431b1a0183d2dfc', @@ -370,7 +370,7 @@ describe('MetaMaskController', function () { metamaskController.preferencesController.store.getState().identities, ); const addresses = - await metamaskController.keyringController.getAccounts(); + await metamaskController.coreKeyringController.getAccounts(); identities.forEach((identity) => { assert.ok( @@ -401,7 +401,7 @@ describe('MetaMaskController', function () { await metamaskController.createNewVaultAndKeychain(password); assert( - metamaskController.keyringController.createNewVaultAndKeychain + metamaskController.coreKeyringController.createNewVaultAndKeychain .calledOnce, ); @@ -611,7 +611,7 @@ describe('MetaMaskController', function () { }); it('should add the Trezor Hardware keyring', async function () { - sinon.spy(metamaskController.keyringController, 'addNewKeyring'); + sinon.spy(metamaskController.coreKeyringController, 'addNewKeyring'); await metamaskController .connectHardware(HardwareDeviceNames.trezor, 0) .catch(() => null); @@ -620,14 +620,15 @@ describe('MetaMaskController', function () { KeyringType.trezor, ); assert.deepEqual( - metamaskController.keyringController.addNewKeyring.getCall(0).args, + metamaskController.coreKeyringController.addNewKeyring.getCall(0) + .args, [KeyringType.trezor], ); assert.equal(keyrings.length, 1); }); it('should add the Ledger Hardware keyring', async function () { - sinon.spy(metamaskController.keyringController, 'addNewKeyring'); + sinon.spy(metamaskController.coreKeyringController, 'addNewKeyring'); await metamaskController .connectHardware(HardwareDeviceNames.ledger, 0) .catch(() => null); @@ -636,7 +637,8 @@ describe('MetaMaskController', function () { KeyringType.ledger, ); assert.deepEqual( - metamaskController.keyringController.addNewKeyring.getCall(0).args, + metamaskController.coreKeyringController.addNewKeyring.getCall(0) + .args, [KeyringType.ledger], ); assert.equal(keyrings.length, 1); @@ -734,13 +736,13 @@ describe('MetaMaskController', function () { windowOpenStub.returns(noop); addNewAccountStub = sinon.stub( - metamaskController.keyringController, + metamaskController.coreKeyringController, 'addNewAccount', ); addNewAccountStub.returns('0x123'); getAccountsStub = sinon.stub( - metamaskController.keyringController, + metamaskController.coreKeyringController, 'getAccounts', ); // Need to return different address to mock the behavior of @@ -767,8 +769,8 @@ describe('MetaMaskController', function () { afterEach(function () { window.open.restore(); - metamaskController.keyringController.addNewAccount.restore(); - metamaskController.keyringController.getAccounts.restore(); + metamaskController.coreKeyringController.addNewAccount.restore(); + metamaskController.coreKeyringController.getAccounts.restore(); metamaskController.preferencesController.setAddresses.restore(); metamaskController.preferencesController.setSelectedAddress.restore(); metamaskController.preferencesController.setAccountLabel.restore(); @@ -783,11 +785,13 @@ describe('MetaMaskController', function () { }); it('should call keyringController.addNewAccount', async function () { - assert(metamaskController.keyringController.addNewAccount.calledOnce); + assert( + metamaskController.coreKeyringController.addNewAccount.calledOnce, + ); }); it('should call keyringController.getAccounts ', async function () { - assert(metamaskController.keyringController.getAccounts.called); + assert(metamaskController.coreKeyringController.getAccounts.called); }); it('should call preferencesController.setAddresses', async function () { @@ -839,7 +843,7 @@ describe('MetaMaskController', function () { await metamaskController.createNewVaultAndKeychain('password'); await metamaskController.addNewAccount(1); const getAccounts = - await metamaskController.keyringController.getAccounts(); + await metamaskController.coreKeyringController.getAccounts(); assert.equal(getAccounts.length, 2); }); }); @@ -906,7 +910,7 @@ describe('MetaMaskController', function () { }; sinon.stub(metamaskController.preferencesController, 'removeAddress'); sinon.stub(metamaskController.accountTracker, 'removeAccount'); - sinon.stub(metamaskController.keyringController, 'removeAccount'); + sinon.stub(metamaskController.coreKeyringController, 'removeAccount'); sinon.stub(metamaskController, 'removeAllAccountPermissions'); sinon .stub( @@ -919,7 +923,7 @@ describe('MetaMaskController', function () { }); afterEach(function () { - metamaskController.keyringController.removeAccount.restore(); + metamaskController.coreKeyringController.removeAccount.restore(); metamaskController.accountTracker.removeAccount.restore(); metamaskController.preferencesController.removeAddress.restore(); metamaskController.removeAllAccountPermissions.restore(); @@ -944,7 +948,7 @@ describe('MetaMaskController', function () { }); it('should call keyringController.removeAccount', async function () { assert( - metamaskController.keyringController.removeAccount.calledWith( + metamaskController.coreKeyringController.removeAccount.calledWith( addressToRemove, ), );