-
-
Notifications
You must be signed in to change notification settings - Fork 6
feat(account-api): add multichain accounts support #302
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
base: main
Are you sure you want to change the base?
Conversation
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
327416e
to
daddb4b
Compare
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
do { | ||
// New index means new accounts. | ||
accounts = []; | ||
|
||
const missingProviders = []; | ||
for (const provider of this.#providers) { | ||
const discoveredAccounts = await provider.discoverAndCreateAccounts({ | ||
entropySource: this.#entropySource, | ||
groupIndex, | ||
}); | ||
|
||
if (discoveredAccounts.length) { | ||
// This provider has discovered and created accounts, meaning there might | ||
// be something to discover on the next index. | ||
accounts = discoveredAccounts; | ||
} else { | ||
// This provider did not discover or create any accounts. We mark it as | ||
// "missing", so we can create accounts on this index if other providers | ||
// did discover something. | ||
missingProviders.push(provider); | ||
} | ||
} | ||
|
||
if (accounts.length) { | ||
// We only create missing accounts if one of the provider has discovered | ||
// and created accounts. | ||
for (const provider of missingProviders) { | ||
await provider.createAccounts({ | ||
entropySource: this.#entropySource, | ||
groupIndex, | ||
}); | ||
} | ||
|
||
// We've got all the accounts now, we can create our multichain account. | ||
multichainAccounts.push(this.#createMultichainAccount(groupIndex)); | ||
|
||
// We have accounts, we need to check the next index. | ||
groupIndex += 1; | ||
} | ||
} while (accounts.length); |
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.
Question: Maybe we should just run discovery with the providers that HAS discovered something? (e.g Provider1 found something on index 0, but Provider2 did not, then we continue with index 1 for Provider1 but without Provider2?
If we do that, then we can use the "alignment mechanism" at the end of the discovery to re-align all accounts accross the multichain account.
for (const provider of this.#providers) { | ||
await provider.createAccounts({ | ||
entropySource: this.#entropySource, | ||
groupIndex, | ||
}); | ||
} |
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.
Move to async/parallel.
getAccounts: (opts: { | ||
entropySource: EntropySourceId; | ||
groupIndex: number; | ||
}) => AccountId[]; |
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.
Should we make this async
too? The main problem here would be that a MultichainAccount
would need a async init()
method too, because constructor cannot be async
.
That was one of my initial pattern, and to make it easier to use, I introduced a static from(...)
class method to be able to construct + init
in one go.
If getAccounts
is meant to be "wired up" to the keyring API {get,list}Accounts
method, then yes, we might need to make it async
. However, we could also keep a copy KeyringAccount
s into each keyring instances on the client side, this way we can just use the copies and not "fetch" the accounts from the Snap itself, making it a "sync" call (similar to how AccountsController:listMultichainAccounts
works today).
for (const provider of missingProviders) { | ||
await provider.createAccounts({ | ||
entropySource: this.#entropySource, | ||
groupIndex, | ||
}); | ||
} |
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.
Move to async/parallel.
@metamaskbot publish-preview |
Preview builds have been published. See these instructions (from the Expand for full list of packages and versions.
|
8c73ff3
to
76246b7
Compare
🚨 BugBot couldn't runSomething went wrong. Try again by commenting "bugbot run", or contact support (requestId: serverGenReqId_96e2ebca-429a-4355-ac04-242fb081de35). |
🚨 BugBot couldn't runSomething went wrong. Try again by commenting "bugbot run", or contact support (requestId: serverGenReqId_78fda8b5-9885-4b8f-8cd7-782e4784412b). |
f623e93
to
0457ad7
Compare
d5d9c23
to
1ff4115
Compare
1ff4115
to
3d4240e
Compare
Draft implementation of the new multichain accounts API and some adapters around it to help with the integration.