Skip to content

Commit 524d5ed

Browse files
ccharlyccharly
authored
feat(account-api): enforce Bip44Account for MultichainAccount* + add MultichainAccount*.sync (#321)
To avoid adding an event-based mechanism to the `AccountProvider` we (for now) rely on external wallet sync's (which will be handle by the new `MultichainAccountService`. This allow existing wallets to "find" new accounts and potentially create new missing multichan accounts. Also, we now enforce `Bip44Account<Account>` for all `Multichain*` classes. This allows us to use `options.entropy.groupIndex` and quickly index multichain accounts based on that, thus, forcing the `AccountProvider` to only provide valid BIP-44 accounts too.
1 parent f7f205f commit 524d5ed

5 files changed

Lines changed: 257 additions & 113 deletions

File tree

‎packages/account-api/CHANGELOG.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- Add `MultichainAccountWallet.sync` method ([#321](https://github.com/MetaMask/accounts/pull/321))
13+
- This can be used to force wallet synchronization if new accounts are available on the account providers.
14+
15+
### Changed
16+
17+
- **BREAKING:** Force `Bip44Account<Account>` for `Multichain*` types ([#321](https://github.com/MetaMask/accounts/pull/321))
18+
- This requires the `AccountProvider`s to also use new `Bip44Account` type constraint.
19+
1020
## [0.2.0]
1121

1222
### Added

‎packages/account-api/src/api/multichain/account.ts‎

Lines changed: 65 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
1-
import {
2-
KeyringAccountEntropyTypeOption,
3-
type KeyringAccount,
4-
} from '@metamask/keyring-api';
1+
import { type KeyringAccount } from '@metamask/keyring-api';
52
import { isScopeEqualToAny } from '@metamask/keyring-utils';
63

74
import type {
85
MultichainAccountWallet,
96
MultichainAccountWalletId,
107
} from './wallet';
8+
import type { Bip44Account } from '../bip44';
119
import type { AccountGroup } from '../group';
1210
import type { AccountProvider } from '../provider';
1311
import type { AccountSelector } from '../selector';
@@ -26,7 +24,7 @@ export type MultichainAccountId = `${MultichainAccountWalletId}/${number}`; // U
2624
/**
2725
* A multichain account that holds multiple accounts.
2826
*/
29-
export class MultichainAccount<Account extends KeyringAccount>
27+
export class MultichainAccount<Account extends Bip44Account<KeyringAccount>>
3028
implements AccountGroup<Account>
3129
{
3230
readonly #id: MultichainAccountId;
@@ -37,6 +35,10 @@ export class MultichainAccount<Account extends KeyringAccount>
3735

3836
readonly #providers: AccountProvider<Account>[];
3937

38+
readonly #providerToAccounts: Map<AccountProvider<Account>, Account['id'][]>;
39+
40+
readonly #accountToProvider: Map<Account['id'], AccountProvider<Account>>;
41+
4042
constructor({
4143
groupIndex,
4244
wallet,
@@ -50,6 +52,42 @@ export class MultichainAccount<Account extends KeyringAccount>
5052
this.#index = groupIndex;
5153
this.#wallet = wallet;
5254
this.#providers = providers;
55+
this.#providerToAccounts = new Map();
56+
this.#accountToProvider = new Map();
57+
58+
this.sync();
59+
}
60+
61+
/**
62+
* Force multichain account synchronization.
63+
*
64+
* This can be used if account providers got new accounts that the multichain
65+
* account doesn't know about.
66+
*/
67+
sync(): void {
68+
// Clear reverse mapping and re-construct it entirely based on the refreshed
69+
// list of accounts from each providers.
70+
this.#accountToProvider.clear();
71+
72+
for (const provider of this.#providers) {
73+
// Filter account only for that index.
74+
const accounts = [];
75+
for (const account of provider.getAccounts()) {
76+
if (
77+
account.options.entropy.id === this.wallet.entropySource &&
78+
account.options.entropy.groupIndex === this.index
79+
) {
80+
// We only use IDs to always fetch the latest version of accounts.
81+
accounts.push(account.id);
82+
}
83+
}
84+
this.#providerToAccounts.set(provider, accounts);
85+
86+
// Reverse-mapping for fast indexing.
87+
for (const id of accounts) {
88+
this.#accountToProvider.set(id, provider);
89+
}
90+
}
5391
}
5492

5593
/**
@@ -85,7 +123,8 @@ export class MultichainAccount<Account extends KeyringAccount>
85123
* @returns True if there's any underlying accounts, false otherwise.
86124
*/
87125
hasAccounts(): boolean {
88-
return this.getAccounts().length > 0;
126+
// If there's anything in the reverse-map, it means we have some accounts.
127+
return this.#accountToProvider.size > 0;
89128
}
90129

91130
/**
@@ -94,23 +133,19 @@ export class MultichainAccount<Account extends KeyringAccount>
94133
* @returns The accounts.
95134
*/
96135
getAccounts(): Account[] {
97-
let allAccounts: Account[] = [];
136+
const allAccounts: Account[] = [];
98137

99-
for (const provider of this.#providers) {
100-
allAccounts = allAccounts.concat(
101-
provider.getAccounts().filter(
102-
// NOTE: For now we always query the providers to get the latest
103-
// account list. If this becomes too "heavy" in terms of computation
104-
// we might wanna consider adding a state to that object and store
105-
// the list of account IDs here.
106-
(account) =>
107-
account.options.entropy &&
108-
account.options.entropy.type ===
109-
KeyringAccountEntropyTypeOption.Mnemonic &&
110-
account.options.entropy.id === this.wallet.entropySource &&
111-
account.options.entropy.groupIndex === this.index,
112-
),
113-
);
138+
for (const [provider, accounts] of this.#providerToAccounts.entries()) {
139+
for (const id of accounts) {
140+
const account = provider.getAccount(id);
141+
142+
if (account) {
143+
// If for some reason we cannot get this account from the provider, it
144+
// might means it has been deleted or something, so we just filter it
145+
// out.
146+
allAccounts.push(account);
147+
}
148+
}
114149
}
115150

116151
return allAccounts;
@@ -123,9 +158,14 @@ export class MultichainAccount<Account extends KeyringAccount>
123158
* @returns The account or undefined if not found.
124159
*/
125160
getAccount(id: Account['id']): Account | undefined {
126-
// NOTE: Same remark here. We could keep a state to make this operation
127-
// faster.
128-
return this.getAccounts().find((account) => account.id === id);
161+
const provider = this.#accountToProvider.get(id);
162+
163+
// If there's nothing in the map, it means we tried to get an account
164+
// that does not belong to this multichain account.
165+
if (!provider) {
166+
return undefined;
167+
}
168+
return provider.getAccount(id);
129169
}
130170

131171
/**

‎packages/account-api/src/api/multichain/wallet.ts‎

Lines changed: 37 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {
2-
KeyringAccountEntropyTypeOption,
32
type EntropySourceId,
43
type KeyringAccount,
54
} from '@metamask/keyring-api';
@@ -9,6 +8,7 @@ import {
98
isMultichainAccountId,
109
MultichainAccount,
1110
} from './account';
11+
import type { Bip44Account } from '../bip44';
1212
import type { AccountGroupId } from '../group';
1313
import { toDefaultAccountGroupId } from '../group';
1414
import type { AccountProvider } from '../provider';
@@ -25,8 +25,9 @@ export type MultichainAccountWalletId =
2525
* A multichain account wallet that holds multiple multichain accounts (one multichain account per
2626
* group index).
2727
*/
28-
export class MultichainAccountWallet<Account extends KeyringAccount>
29-
implements AccountWallet<Account>
28+
export class MultichainAccountWallet<
29+
Account extends Bip44Account<KeyringAccount>,
30+
> implements AccountWallet<Account>
3031
{
3132
readonly #id: MultichainAccountWalletId;
3233

@@ -48,60 +49,49 @@ export class MultichainAccountWallet<Account extends KeyringAccount>
4849
this.#entropySource = entropySource;
4950
this.#accounts = new Map();
5051

51-
// NOTE: This will traverse all accounts to compute the max index. We could try
52-
// to minimize the number of filtering we're doing on each providers if this
53-
// becomes too costly.
54-
const maxGroupIndex = MultichainAccountWallet.getHighestGroupIndexFrom(
55-
providers,
56-
entropySource,
57-
);
58-
59-
// NOTE: We could have some gap for now, until we fully implement the
60-
// gap/alignment mechanisms to backfill all "missing accounts".
61-
for (let groupIndex = 0; groupIndex <= maxGroupIndex; groupIndex++) {
62-
// Use "lower or equal", since we need to "include" the max index (which
63-
// can also be 0)
64-
const multichainAccount = new MultichainAccount<Account>({
65-
groupIndex,
66-
wallet: this,
67-
providers: this.#providers,
68-
});
69-
70-
// We only add multichain account that has underlying accounts.
71-
if (multichainAccount.hasAccounts()) {
72-
this.#accounts.set(groupIndex, multichainAccount);
73-
}
74-
}
52+
// Initial synchronization.
53+
this.sync();
7554
}
7655

7756
/**
78-
* Gets the highest group index from multiple account providers for a given
79-
* entropy source.
57+
* Force wallet synchronization.
8058
*
81-
* @param providers - Account providers.
82-
* @param entropySource - Entropy source to filter on.
83-
* @returns The highest group index for a given entropy source.
59+
* This can be used if account providers got new accounts that the wallet
60+
* doesn't know about.
8461
*/
85-
static getHighestGroupIndexFrom<Account extends KeyringAccount>(
86-
providers: AccountProvider<Account>[],
87-
entropySource: EntropySourceId,
88-
): number {
89-
let max = -1;
90-
91-
for (const provider of providers) {
62+
sync(): void {
63+
for (const provider of this.#providers) {
9264
for (const account of provider.getAccounts()) {
93-
if (
94-
account.options.entropy &&
95-
account.options.entropy.type ===
96-
KeyringAccountEntropyTypeOption.Mnemonic &&
97-
account.options.entropy.id === entropySource
98-
) {
99-
max = Math.max(max, account.options.entropy.groupIndex);
65+
const { entropy } = account.options;
66+
67+
// Filter for this wallet only.
68+
if (entropy.id !== this.entropySource) {
69+
continue;
70+
}
71+
72+
// This multichain account might exists already.
73+
let multichainAccount = this.#accounts.get(entropy.groupIndex);
74+
if (!multichainAccount) {
75+
multichainAccount = new MultichainAccount<Account>({
76+
groupIndex: entropy.groupIndex,
77+
wallet: this,
78+
providers: this.#providers,
79+
});
80+
81+
this.#accounts.set(entropy.groupIndex, multichainAccount);
10082
}
10183
}
10284
}
10385

104-
return max;
86+
// Now force-sync all remaining multichain accounts.
87+
for (const [groupIndex, multichainAccount] of this.#accounts.entries()) {
88+
multichainAccount.sync();
89+
90+
// Clean up old multichain accounts.
91+
if (!multichainAccount.hasAccounts()) {
92+
this.#accounts.delete(groupIndex);
93+
}
94+
}
10595
}
10696

10797
/**

‎packages/account-api/src/api/provider.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,13 @@ import type { KeyringAccount } from '@metamask/keyring-api';
44
* An account provider is reponsible of providing accounts to an account group.
55
*/
66
export type AccountProvider<Account extends KeyringAccount> = {
7+
/**
8+
* Gets an account for a given ID.
9+
*
10+
* @returns An account, or undefined if not found.
11+
*/
12+
getAccount: (id: Account['id']) => Account | undefined;
13+
714
/**
815
* Gets all accounts for a given entropy source and group index.
916
*

0 commit comments

Comments
 (0)