Skip to content

Commit 4691f2f

Browse files
PatrykLuckaccharlymetamaskbot
authored
feat: add calls to discover accounts (#31485)
## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> This PR triggers Solana account discovery when importing an SRP and creates Solana accounts for all active addresses found. ## **Related issues** Fixes: ## **Manual testing steps** 1. Import SRP through onboarding or import SRP option in accounts menu 2. If Solana accounts have any activity, accounts should be added automatically ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. --------- Co-authored-by: Charly Chevalier <[email protected]> Co-authored-by: MetaMask Bot <[email protected]>
1 parent 0a94a44 commit 4691f2f

File tree

10 files changed

+175
-16
lines changed

10 files changed

+175
-16
lines changed

‎app/scripts/metamask-controller.js

+30-3
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@ import fetchWithCache from '../../shared/lib/fetch-with-cache';
264264
import { MultichainNetworks } from '../../shared/constants/multichain/networks';
265265
import { BRIDGE_API_BASE_URL } from '../../shared/constants/bridge';
266266
import { BridgeStatusAction } from '../../shared/types/bridge-status';
267+
///: BEGIN:ONLY_INCLUDE_IF(solana)
268+
import { addDiscoveredSolanaAccounts } from '../../shared/lib/accounts';
269+
///: END:ONLY_INCLUDE_IF
267270
import {
268271
///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
269272
handleMMITransactionUpdate,
@@ -4816,10 +4819,26 @@ export default class MetamaskController extends EventEmitter {
48164819
? { id: keyringId }
48174820
: { type: KeyringTypes.hd };
48184821

4819-
const accounts = await this.keyringController.withKeyring(
4822+
const {
4823+
accounts,
4824+
///: BEGIN:ONLY_INCLUDE_IF(solana)
4825+
entropySource,
4826+
///: END:ONLY_INCLUDE_IF
4827+
} = await this.keyringController.withKeyring(
48204828
keyringSelector,
4821-
async ({ keyring }) => {
4822-
return await keyring.getAccounts();
4829+
async ({
4830+
keyring,
4831+
///: BEGIN:ONLY_INCLUDE_IF(solana)
4832+
metadata,
4833+
///: END:ONLY_INCLUDE_IF
4834+
}) => {
4835+
const keyringAccounts = await keyring.getAccounts();
4836+
return {
4837+
accounts: keyringAccounts,
4838+
///: BEGIN:ONLY_INCLUDE_IF(solana)
4839+
entropySource: metadata.id,
4840+
///: END:ONLY_INCLUDE_IF
4841+
};
48234842
},
48244843
);
48254844
let address = accounts[accounts.length - 1];
@@ -4860,6 +4879,14 @@ export default class MetamaskController extends EventEmitter {
48604879
},
48614880
);
48624881
}
4882+
///: BEGIN:ONLY_INCLUDE_IF(solana)
4883+
const keyring = await this.getSnapKeyring();
4884+
await addDiscoveredSolanaAccounts(
4885+
this.controllerMessenger,
4886+
entropySource,
4887+
keyring,
4888+
);
4889+
///: END:ONLY_INCLUDE_IF
48634890
} catch (e) {
48644891
log.warn(`Failed to add accounts with balance. Error: ${e}`);
48654892
} finally {

‎app/scripts/metamask-controller.test.js

+78-2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
BtcAccountType,
1818
BtcMethod,
1919
EthAccountType,
20+
SolScope,
2021
} from '@metamask/keyring-api';
2122
import { Messenger } from '@metamask/base-controller';
2223
import { LoggingController, LogType } from '@metamask/logging-controller';
@@ -36,6 +37,7 @@ import {
3637
Caip25EndowmentPermissionName,
3738
} from '@metamask/chain-agnostic-permission';
3839
import { PermissionDoesNotExistError } from '@metamask/permission-controller';
40+
import { KeyringInternalSnapClient } from '@metamask/keyring-internal-snap-client';
3941
import { createTestProviderTools } from '../../test/stub/provider';
4042
import {
4143
HardwareDeviceNames,
@@ -3957,7 +3959,7 @@ describe('MetaMaskController', () => {
39573959
const newlyAddedKeyringId =
39583960
metamaskController.keyringController.state.keyringsMetadata[
39593961
metamaskController.keyringController.state.keyringsMetadata.length -
3960-
1
3962+
2 // -1 for the snap keyring, -1 for the newly added keyring
39613963
].id;
39623964

39633965
const newSRP = Buffer.from(
@@ -3967,7 +3969,10 @@ describe('MetaMaskController', () => {
39673969
expect(
39683970
currentKeyrings.filter((kr) => kr.type === 'HD Key Tree'),
39693971
).toHaveLength(2);
3970-
expect(currentKeyrings).toHaveLength(previousKeyrings.length + 1);
3972+
expect(
3973+
currentKeyrings.filter((kr) => kr.type === 'Snap Keyring'),
3974+
).toHaveLength(1);
3975+
expect(currentKeyrings).toHaveLength(previousKeyrings.length + 2);
39713976
expect(newSRP).toStrictEqual(TEST_SEED_ALT);
39723977
});
39733978

@@ -3982,6 +3987,77 @@ describe('MetaMaskController', () => {
39823987
'This Secret Recovery Phrase has already been imported.',
39833988
);
39843989
});
3990+
3991+
///: BEGIN:ONLY_INCLUDE_IF(multi-srp)
3992+
it('discovers and creates Solana accounts through KeyringInternalSnapClient when importing a mnemonic', async () => {
3993+
const password = 'what-what-what';
3994+
jest.spyOn(metamaskController, 'getBalance').mockResolvedValue('0x0');
3995+
3996+
const mockDiscoverAccounts = jest
3997+
.fn()
3998+
.mockResolvedValueOnce([{ derivationPath: "m/44'/501'/0'/0'" }])
3999+
.mockResolvedValueOnce([{ derivationPath: "m/44'/501'/1'/0'" }])
4000+
.mockResolvedValueOnce([]); // Return empty array on third call to stop the discovery loop
4001+
4002+
jest
4003+
.spyOn(KeyringInternalSnapClient.prototype, 'discoverAccounts')
4004+
.mockImplementation(mockDiscoverAccounts);
4005+
4006+
const mockCreateAccount = jest.fn().mockResolvedValue(undefined);
4007+
const mockSnapKeyring = { createAccount: mockCreateAccount };
4008+
jest
4009+
.spyOn(metamaskController, 'getSnapKeyring')
4010+
.mockResolvedValue(mockSnapKeyring);
4011+
4012+
await metamaskController.createNewVaultAndRestore(password, TEST_SEED);
4013+
await metamaskController.importMnemonicToVault(TEST_SEED_ALT);
4014+
4015+
// Assert that discoverAccounts was called correctly
4016+
// Should be called 3 times (twice with discovered accounts, once with empty array)
4017+
expect(mockDiscoverAccounts).toHaveBeenCalledTimes(3);
4018+
4019+
// All calls should include the solana scopes
4020+
expect(mockDiscoverAccounts.mock.calls[0][0]).toStrictEqual(
4021+
expect.arrayContaining([
4022+
SolScope.Mainnet,
4023+
SolScope.Testnet,
4024+
SolScope.Devnet,
4025+
]),
4026+
);
4027+
4028+
// First call should be for index 0
4029+
expect(mockDiscoverAccounts.mock.calls[0][2]).toBe(0);
4030+
// Second call should be for index 1
4031+
expect(mockDiscoverAccounts.mock.calls[1][2]).toBe(1);
4032+
// Third call should be for index 2
4033+
expect(mockDiscoverAccounts.mock.calls[2][2]).toBe(2);
4034+
4035+
// Assert that createAccount was called correctly for each discovered account
4036+
expect(mockCreateAccount).toHaveBeenCalledTimes(2);
4037+
4038+
// All calls should use the solana snap ID
4039+
expect(mockCreateAccount.mock.calls[0][0]).toStrictEqual(
4040+
expect.stringContaining('solana-wallet'),
4041+
);
4042+
// First call should use derivation path on index 0
4043+
expect(mockCreateAccount.mock.calls[0][1]).toStrictEqual({
4044+
derivationPath: "m/44'/501'/0'/0'",
4045+
entropySource: expect.any(String),
4046+
});
4047+
// All calls should use the same internal options
4048+
expect(mockCreateAccount.mock.calls[0][2]).toStrictEqual({
4049+
displayConfirmation: false,
4050+
displayAccountNameSuggestion: false,
4051+
setSelectedAccount: false,
4052+
});
4053+
4054+
// Second call should use derivation path on index 1
4055+
expect(mockCreateAccount.mock.calls[1][1]).toStrictEqual({
4056+
derivationPath: "m/44'/501'/1'/0'",
4057+
entropySource: expect.any(String),
4058+
});
4059+
});
4060+
///: END:ONLY_INCLUDE_IF
39854061
});
39864062
});
39874063

‎lavamoat/browserify/beta/policy.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@
14411441
"@ethereumjs/tx": true,
14421442
"@metamask/eth-snap-keyring>@metamask/eth-sig-util": true,
14431443
"@metamask/keyring-api": true,
1444-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": true,
1444+
"@metamask/keyring-internal-snap-client": true,
14451445
"@metamask/keyring-api>@metamask/keyring-utils": true,
14461446
"@metamask/utils>@metamask/superstruct": true,
14471447
"@metamask/utils": true,
@@ -1617,7 +1617,7 @@
16171617
"@metamask/keyring-controller>ulid": true
16181618
}
16191619
},
1620-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": {
1620+
"@metamask/keyring-internal-snap-client": {
16211621
"packages": {
16221622
"@metamask/keyring-snap-client": true
16231623
}

‎lavamoat/browserify/flask/policy.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@
14411441
"@ethereumjs/tx": true,
14421442
"@metamask/eth-snap-keyring>@metamask/eth-sig-util": true,
14431443
"@metamask/keyring-api": true,
1444-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": true,
1444+
"@metamask/keyring-internal-snap-client": true,
14451445
"@metamask/keyring-api>@metamask/keyring-utils": true,
14461446
"@metamask/utils>@metamask/superstruct": true,
14471447
"@metamask/utils": true,
@@ -1617,7 +1617,7 @@
16171617
"@metamask/keyring-controller>ulid": true
16181618
}
16191619
},
1620-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": {
1620+
"@metamask/keyring-internal-snap-client": {
16211621
"packages": {
16221622
"@metamask/keyring-snap-client": true
16231623
}

‎lavamoat/browserify/main/policy.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1441,7 +1441,7 @@
14411441
"@ethereumjs/tx": true,
14421442
"@metamask/eth-snap-keyring>@metamask/eth-sig-util": true,
14431443
"@metamask/keyring-api": true,
1444-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": true,
1444+
"@metamask/keyring-internal-snap-client": true,
14451445
"@metamask/keyring-api>@metamask/keyring-utils": true,
14461446
"@metamask/utils>@metamask/superstruct": true,
14471447
"@metamask/utils": true,
@@ -1617,7 +1617,7 @@
16171617
"@metamask/keyring-controller>ulid": true
16181618
}
16191619
},
1620-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": {
1620+
"@metamask/keyring-internal-snap-client": {
16211621
"packages": {
16221622
"@metamask/keyring-snap-client": true
16231623
}

‎lavamoat/browserify/mmi/policy.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -1572,7 +1572,7 @@
15721572
"@ethereumjs/tx": true,
15731573
"@metamask/eth-snap-keyring>@metamask/eth-sig-util": true,
15741574
"@metamask/keyring-api": true,
1575-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": true,
1575+
"@metamask/keyring-internal-snap-client": true,
15761576
"@metamask/keyring-api>@metamask/keyring-utils": true,
15771577
"@metamask/utils>@metamask/superstruct": true,
15781578
"@metamask/utils": true,
@@ -1748,7 +1748,7 @@
17481748
"@metamask/keyring-controller>ulid": true
17491749
}
17501750
},
1751-
"@metamask/eth-snap-keyring>@metamask/keyring-internal-snap-client": {
1751+
"@metamask/keyring-internal-snap-client": {
17521752
"packages": {
17531753
"@metamask/keyring-snap-client": true
17541754
}

‎package.json

+1
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@
286286
"@metamask/keyring-api": "^17.5.0",
287287
"@metamask/keyring-controller": "^21.0.1",
288288
"@metamask/keyring-internal-api": "^6.0.1",
289+
"@metamask/keyring-internal-snap-client": "^4.0.1",
289290
"@metamask/keyring-snap-client": "^4.1.0",
290291
"@metamask/logging-controller": "^6.0.4",
291292
"@metamask/logo": "^4.0.0",

‎privacy-snapshot.json

+3-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
"api.lens.dev",
77
"api.segment.io",
88
"api.simplehash.com",
9+
"api.testnet.solana.com",
910
"api.web3modal.com",
11+
"api.web3modal.org",
1012
"app.ens.domains",
1113
"arbitrum-mainnet.infura.io",
1214
"authentication.api.cx.metamask.io",
@@ -84,6 +86,5 @@
8486
"unresponsive-rpc.url",
8587
"user-storage.api.cx.metamask.io",
8688
"verify.walletconnect.com",
87-
"www.4byte.directory",
88-
"api.web3modal.org"
89+
"www.4byte.directory"
8990
]

‎shared/lib/accounts/accounts.ts

+53
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
import { InternalAccount } from '@metamask/keyring-internal-api';
2+
///: BEGIN:ONLY_INCLUDE_IF(solana)
3+
import { SolScope } from '@metamask/keyring-api';
4+
import {
5+
KeyringInternalSnapClient,
6+
KeyringInternalSnapClientMessenger,
7+
} from '@metamask/keyring-internal-snap-client';
8+
import { SnapKeyring } from '@metamask/eth-snap-keyring';
9+
import { SOLANA_WALLET_SNAP_ID } from './solana-wallet-snap';
10+
///: END:ONLY_INCLUDE_IF
211

312
/**
413
* Get the next available account name based on the suggestion and the list of
@@ -25,3 +34,47 @@ export function getUniqueAccountName(
2534

2635
return candidateName;
2736
}
37+
38+
///: BEGIN:ONLY_INCLUDE_IF(solana)
39+
export async function addDiscoveredSolanaAccounts(
40+
controllerMessenger: KeyringInternalSnapClientMessenger,
41+
entropySource: string,
42+
snapKeyring: SnapKeyring,
43+
) {
44+
const snapId = SOLANA_WALLET_SNAP_ID;
45+
const scopes = [SolScope.Mainnet, SolScope.Testnet, SolScope.Devnet];
46+
const client = new KeyringInternalSnapClient({
47+
messenger: controllerMessenger,
48+
snapId,
49+
});
50+
51+
for (let index = 0; ; index++) {
52+
const discovered = await client.discoverAccounts(
53+
scopes,
54+
entropySource,
55+
index,
56+
);
57+
58+
// We stop discovering accounts if none got discovered for that index.
59+
if (discovered.length === 0) {
60+
break;
61+
}
62+
63+
await Promise.allSettled(
64+
discovered.map(async (discoveredAccount) => {
65+
const options = {
66+
derivationPath: discoveredAccount.derivationPath,
67+
entropySource,
68+
};
69+
70+
// TODO: Use `withKeyring` instead of using the keyring directly.
71+
await snapKeyring.createAccount(snapId, options, {
72+
displayConfirmation: false,
73+
displayAccountNameSuggestion: false,
74+
setSelectedAccount: false,
75+
});
76+
}),
77+
);
78+
}
79+
}
80+
///: END:ONLY_INCLUDE_IF

‎yarn.lock

+2-1
Original file line numberDiff line numberDiff line change
@@ -5686,7 +5686,7 @@ __metadata:
56865686
languageName: node
56875687
linkType: hard
56885688

5689-
"@metamask/keyring-internal-snap-client@npm:^4.0.2":
5689+
"@metamask/keyring-internal-snap-client@npm:^4.0.1, @metamask/keyring-internal-snap-client@npm:^4.0.2":
56905690
version: 4.0.2
56915691
resolution: "@metamask/keyring-internal-snap-client@npm:4.0.2"
56925692
dependencies:
@@ -27380,6 +27380,7 @@ __metadata:
2738027380
"@metamask/keyring-api": "npm:^17.5.0"
2738127381
"@metamask/keyring-controller": "npm:^21.0.1"
2738227382
"@metamask/keyring-internal-api": "npm:^6.0.1"
27383+
"@metamask/keyring-internal-snap-client": "npm:^4.0.1"
2738327384
"@metamask/keyring-snap-client": "npm:^4.1.0"
2738427385
"@metamask/logging-controller": "npm:^6.0.4"
2738527386
"@metamask/logo": "npm:^4.0.0"

0 commit comments

Comments
 (0)