Repository navigation
feat: changes to AccountTrackerController to enable migration in extension #6938
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.
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
Changes from all commits
6fbb0db
561d671
513b9f4
16269c5
2f1d63a
c5ed43d
5f5eac4
e77db65
ebf01f5
7219a99
f4fc838
46e7fff
e9a7668
8d1b9ed
82946f2
5bd049e
05db5d7
59c7b13
d25d92e
18b3603
05c9402
421e829
21c6dc6
5703ebf
0eed652
e66e1d6
d632f1d
d8e6584
6416b07
887c9c3
bdfc708
fb233ab
e929311
64d885f
55dcb92
878ed7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,6 @@ import type { | |
| AccountsControllerSelectedEvmAccountChangeEvent, | ||
| AccountsControllerGetSelectedAccountAction, | ||
| AccountsControllerListAccountsAction, | ||
| AccountsControllerSelectedAccountChangeEvent, | ||
| } from '@metamask/accounts-controller'; | ||
| import type { | ||
| ControllerStateChangeEvent, | ||
|
|
@@ -16,16 +15,17 @@ import { | |
| toChecksumHexAddress, | ||
| } from '@metamask/controller-utils'; | ||
| import EthQuery from '@metamask/eth-query'; | ||
| import type { KeyringControllerUnlockEvent } from '@metamask/keyring-controller'; | ||
| import type { InternalAccount } from '@metamask/keyring-internal-api'; | ||
| import type { Messenger } from '@metamask/messenger'; | ||
| import type { | ||
| NetworkClient, | ||
| NetworkClientId, | ||
| NetworkControllerGetNetworkClientByIdAction, | ||
| NetworkControllerGetStateAction, | ||
| NetworkControllerNetworkAddedEvent, | ||
| } from '@metamask/network-controller'; | ||
| import { StaticIntervalPollingController } from '@metamask/polling-controller'; | ||
| import type { PreferencesControllerGetStateAction } from '@metamask/preferences-controller'; | ||
| import type { | ||
| TransactionControllerTransactionConfirmedEvent, | ||
| TransactionControllerUnapprovedTransactionAddedEvent, | ||
|
|
@@ -174,7 +174,10 @@ export type AccountTrackerControllerActions = | |
| */ | ||
| export type AllowedActions = | ||
| | AccountsControllerListAccountsAction | ||
| | PreferencesControllerGetStateAction | ||
| | { | ||
| type: 'PreferencesController:getState'; | ||
| handler: () => { isMultiAccountBalancesEnabled: boolean }; | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fixes type issues in clients, as the extension PreferencesController state is not compatible with core. We only care about |
||
| | AccountsControllerGetSelectedAccountAction | ||
| | NetworkControllerGetStateAction | ||
| | NetworkControllerGetNetworkClientByIdAction; | ||
|
|
@@ -199,9 +202,10 @@ export type AccountTrackerControllerEvents = | |
| */ | ||
| export type AllowedEvents = | ||
| | AccountsControllerSelectedEvmAccountChangeEvent | ||
| | AccountsControllerSelectedAccountChangeEvent | ||
| | TransactionControllerUnapprovedTransactionAddedEvent | ||
| | TransactionControllerTransactionConfirmedEvent; | ||
| | TransactionControllerTransactionConfirmedEvent | ||
| | NetworkControllerNetworkAddedEvent | ||
| | KeyringControllerUnlockEvent; | ||
|
|
||
| /** | ||
| * The messenger of the {@link AccountTrackerController}. | ||
|
|
@@ -236,6 +240,8 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
|
|
||
| readonly #balanceFetchers: BalanceFetcher[]; | ||
|
|
||
| readonly #fetchingEnabled: () => boolean; | ||
|
|
||
| /** | ||
| * Creates an AccountTracker instance. | ||
| * | ||
|
|
@@ -247,6 +253,7 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
| * @param options.includeStakedAssets - Whether to include staked assets in the account balances. | ||
| * @param options.accountsApiChainIds - Function that returns array of chainIds that should use Accounts-API strategy (if supported by API). | ||
| * @param options.allowExternalServices - Disable external HTTP calls (privacy / offline mode). | ||
| * @param options.fetchingEnabled - Function that returns whether the controller is fetching enabled. | ||
| */ | ||
| constructor({ | ||
| interval = 10000, | ||
|
|
@@ -256,6 +263,7 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
| includeStakedAssets = false, | ||
| accountsApiChainIds = () => [], | ||
| allowExternalServices = () => true, | ||
| fetchingEnabled = () => true, | ||
| }: { | ||
| interval?: number; | ||
| state?: Partial<AccountTrackerControllerState>; | ||
|
|
@@ -264,6 +272,7 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
| includeStakedAssets?: boolean; | ||
| accountsApiChainIds?: () => ChainIdHex[]; | ||
| allowExternalServices?: () => boolean; | ||
| fetchingEnabled?: () => boolean; | ||
| }) { | ||
| const { selectedNetworkClientId } = messenger.call( | ||
| 'NetworkController:getState', | ||
|
|
@@ -302,6 +311,8 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
| ), | ||
| ]; | ||
|
|
||
| this.#fetchingEnabled = fetchingEnabled; | ||
|
|
||
| this.setIntervalLength(interval); | ||
|
|
||
| this.messenger.subscribe( | ||
|
|
@@ -316,23 +327,39 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
| (event): string => event.address, | ||
| ); | ||
|
|
||
| this.messenger.subscribe('NetworkController:networkAdded', async () => { | ||
| await this.refresh(this.#getNetworkClientIds()); | ||
| }); | ||
|
|
||
| this.messenger.subscribe('KeyringController:unlock', async () => { | ||
| await this.refresh(this.#getNetworkClientIds()); | ||
| }); | ||
|
bergarces marked this conversation as resolved.
|
||
|
|
||
| this.messenger.subscribe( | ||
| 'TransactionController:unapprovedTransactionAdded', | ||
| async (transactionMeta: TransactionMeta) => { | ||
| await this.#refreshAddress( | ||
| [transactionMeta.networkClientId], | ||
| transactionMeta.txParams.from, | ||
| ); | ||
| const addresses = [transactionMeta.txParams.from]; | ||
| if (transactionMeta.txParams.to) { | ||
| addresses.push(transactionMeta.txParams.to); | ||
| } | ||
| await this.refreshAddresses({ | ||
| networkClientIds: [transactionMeta.networkClientId], | ||
| addresses, | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
| this.messenger.subscribe( | ||
| 'TransactionController:transactionConfirmed', | ||
| async (transactionMeta: TransactionMeta) => { | ||
| await this.#refreshAddress( | ||
| [transactionMeta.networkClientId], | ||
| transactionMeta.txParams.from, | ||
| ); | ||
| const addresses = [transactionMeta.txParams.from]; | ||
| if (transactionMeta.txParams.to) { | ||
| addresses.push(transactionMeta.txParams.to); | ||
| } | ||
| await this.refreshAddresses({ | ||
| networkClientIds: [transactionMeta.networkClientId], | ||
| addresses, | ||
| }); | ||
| }, | ||
| ); | ||
|
|
||
|
|
@@ -540,13 +567,28 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
| }); | ||
| } | ||
|
|
||
| async #refreshAddress(networkClientIds: NetworkClientId[], address: string) { | ||
| const checksumAddress = toChecksumHexAddress(address) as ChecksumAddress; | ||
| async refreshAddresses({ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making this public the same as |
||
| networkClientIds, | ||
| addresses, | ||
| }: { | ||
| networkClientIds: NetworkClientId[]; | ||
| addresses: string[]; | ||
| }) { | ||
| const checksummedAddresses = addresses.map((address) => | ||
| toChecksumHexAddress(address), | ||
| ); | ||
|
|
||
| const accounts = this.messenger | ||
| .call('AccountsController:listAccounts') | ||
| .filter((account) => | ||
| checksummedAddresses.includes(toChecksumHexAddress(account.address)), | ||
| ); | ||
|
|
||
| await this.#refreshAccounts({ | ||
| networkClientIds, | ||
| queryAllAccounts: false, | ||
| selectedAccount: checksumAddress, | ||
| allAccounts: [], | ||
| queryAllAccounts: true, | ||
| selectedAccount: '0x0', | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With We really shouldn't be delegating this logic down to the BalanceFetcher class, but that'd require a chunkier refactor. |
||
| allAccounts: accounts, | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -570,6 +612,10 @@ export class AccountTrackerController extends StaticIntervalPollingController<Ac | |
|
|
||
| this.syncAccounts(chainIds); | ||
|
|
||
| if (!this.#fetchingEnabled()) { | ||
| return; | ||
| } | ||
|
bergarces marked this conversation as resolved.
bergarces marked this conversation as resolved.
bergarces marked this conversation as resolved.
|
||
|
|
||
| // Use balance fetchers with fallback strategy | ||
| const aggregated: ProcessedBalance[] = []; | ||
| let remainingChains = [...chainIds] as ChainIdHex[]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salimtb I'd like your input here if possible.
From what I can see, if we use multicall (
queryAllAccounts: true), the resulting addresses will be lowercase, but when we query a single address (queryAllAccounts: false) the result is a checksummed address.Therefore, since I have changed the these two events to query multiple accounts (both the
fromand theto), I had to change this return value from the test.Does that sound right?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this sounds right yes