Skip to content

Commit

Permalink
refactor: remove remaining eth-keyring-controller use
Browse files Browse the repository at this point in the history
  • Loading branch information
@mikesposito
mikesposito committed Aug 17, 2023
1 parent 6b72316 commit 8b95f54
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 52 deletions.
4 changes: 2 additions & 2 deletions app/scripts/metamask-controller.actions.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,15 @@ describe('MetaMaskController', function () {
]),
Promise.resolve(1).then(() => {
keyringControllerState1 = JSON.stringify(
metamaskController.keyringController.memStore.getState(),
metamaskController.coreKeyringController.state,
);
metamaskController.importAccountWithStrategy('privateKey', [
importPrivkey,
]);
}),
Promise.resolve(2).then(() => {
keyringControllerState2 = JSON.stringify(
metamaskController.keyringController.memStore.getState(),
metamaskController.coreKeyringController.state,
);
}),
]);
Expand Down
48 changes: 16 additions & 32 deletions app/scripts/metamask-controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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())),
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

//=============================================================================
Expand Down
40 changes: 22 additions & 18 deletions app/scripts/metamask-controller.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ describe('MetaMaskController', function () {

// add sinon method spies
sandbox.spy(
metamaskController.keyringController,
metamaskController.coreKeyringController,
'createNewVaultAndKeychain',
);
sandbox.spy(
Expand Down Expand Up @@ -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',
Expand All @@ -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(
Expand Down Expand Up @@ -401,7 +401,7 @@ describe('MetaMaskController', function () {
await metamaskController.createNewVaultAndKeychain(password);

assert(
metamaskController.keyringController.createNewVaultAndKeychain
metamaskController.coreKeyringController.createNewVaultAndKeychain
.calledOnce,
);

Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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();
Expand All @@ -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 () {
Expand Down Expand Up @@ -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);
});
});
Expand Down Expand Up @@ -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(
Expand All @@ -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();
Expand All @@ -944,7 +948,7 @@ describe('MetaMaskController', function () {
});
it('should call keyringController.removeAccount', async function () {
assert(
metamaskController.keyringController.removeAccount.calledWith(
metamaskController.coreKeyringController.removeAccount.calledWith(
addressToRemove,
),
);
Expand Down

0 comments on commit 8b95f54

Please sign in to comment.