SIP-27: Accounts Metadata Request #148
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||
| --- | ||||||
| sip: 27 | ||||||
| title: Accounts Metadata Request | ||||||
| status: Draft | ||||||
| author: Olaf Tomalka (@ritave) | ||||||
| created: 2024-09-17 | ||||||
| --- | ||||||
|
|
||||||
| ## Abstract | ||||||
|
|
||||||
| This SIP allows snaps to retrieve metadata related to accounts that exist in the extension. | ||||||
|
|
||||||
| ## Motivation | ||||||
|
|
||||||
| The intention of this SIP is to allow snaps providing new accounts for new chains to be able to list all accounts when selecting one during transfers. | ||||||
ritave marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| ## Specification | ||||||
|
|
||||||
| > Indented sections like this are considered non-normative. | ||||||
|
|
||||||
| > Usage of `CAIP-N` specifications, where `N` is a number, are references to [Chain Agnostic Improvement Proposals](https://github.com/ChainAgnostic/CAIPs). | ||||||
|
|
||||||
| ### Language | ||||||
|
|
||||||
| The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", | ||||||
| "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and | ||||||
| "OPTIONAL" written in uppercase in this document are to be interpreted as described in [RFC 2119](https://www.ietf.org/rfc/rfc2119.txt) | ||||||
|
|
||||||
| ### Snap Manifest | ||||||
|
|
||||||
| This SIP specifies a permission named `keyring_listAccountsAll`. This permission grants a snap the ability to retrieve account metadata through an RPC call. | ||||||
|
||||||
| This SIP specifies a permission named `keyring_listAccountsAll`. This permission grants a snap the ability to retrieve account metadata through an RPC call. | |
| This SIP specifies a permission named `keyring_listAllAccounts`. This permission grants a snap the ability to retrieve account metadata through an RPC call. |
There was a problem hiding this comment.
I specifically don't want it to be mistaken with existing keyring_listAccounts, which only lists accounts managed by the Snap. IMHO, by appending it at the end it's more visible,
There was a problem hiding this comment.
I wonder if it makes sense to prefix this with keyring_ in the first place. So far these APIs are only used by account Snaps.
There was a problem hiding this comment.
If we want to display all accounts, we have to support how keyring manages and stores them in the extension, so this API mimics keyring internals almost 1:1.
ritave marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
Does it make sense to expose this? What do you envision we use it for?
There was a problem hiding this comment.
When you have two accounts with the same address (eg 2 different hardware wallets with the same seed), you need to differentiate them somehow.
There was a problem hiding this comment.
I feel like this just complicates things. The Snap has no information about how the address was created, right? Is it even relevant for the Snap?
There was a problem hiding this comment.
This provides a stable identity for an account.
For example, you could want to map accounts to a user editable notes about those accounts.
You can't use the name since it's user editable and can change, and you can't use address because there are multiple accounts with the same address.
By providing a stable identity (similar to React's key property) you can easily manipulate those accounts.
Outdated
There was a problem hiding this comment.
Should this be a CAIP-10 address?
There was a problem hiding this comment.
This address can exist on multiple chains at the same time. Keyrings initially had a chains property, but it's not implemented
There was a problem hiding this comment.
Then how would you identify which network the address is tied to? 🤔
There was a problem hiding this comment.
The only data available in MetaMask currently is the type property.
There was a problem hiding this comment.
I guess my question is, should we combine type and address into one field with a CAIP-10 address.
There was a problem hiding this comment.
type exposes more information than just the network the address is on.
Accounts team envisioned the following types:
type AccountType =
| 'eip155:eoa'
| 'eip155:sca:erc4337'
| 'bip122:p2pk'
| 'bip122:p2pkh'
| 'bip122:p2sh'
| 'bip122:p2wpkh'
| 'bip122:p2tr';Notice they are all on mainnet while still differing
There was a problem hiding this comment.
The main goal of type is to differentiate between accounts that may behave differently, such as EOAs and Smart Contract Accounts.
For multichain, we are still working on a solution that addresses two key issues:
- Storing past Bitcoin addresses
- Supporting multiple addresses for multiple chains (e.g., Cosmos)
We are considering approaches like addresses: Map<Caip2ChainId, Address[]> or addresses: Caip10Address[] (PR draft). Which is aligned with what you said above.
There was a problem hiding this comment.
@ritave, I need to double-check, but the reason we dropped the chains field was that CAIP-2 doesn't support wildcards. We considered using eip155:*, even though it wouldn’t be compliant, but ultimately decided to replace it with a filterAccountChains method. This method can be called with a list of CAIP-2 chain IDs, and the Snap will return the list of supported chains from the input where the account can be used.
I'm not super happy with the decision and think we can reconsided the chains field.
There was a problem hiding this comment.
IIRC, we were also considering using just the CAIP namespace as an alternative to wildcard. This way we can regroup "things" on the same namespace, so instead of eip155:*, we would use eip155.
We started using this pattern in the extension/core IIRC. But this is still not 100% compliant with a true CAIP-2 identifier...
There was a problem hiding this comment.
If we wish to have a similar UI than the native one to list the accounts, we might needs some more information coming from the metadata field from InternalAccount (maybe not everything, but I was thinking about hardware-wallet accounts).
In case of HW, the account.type will be eip155:eoa, but we don't encode any other information regarding which HW if comes from. We do have this information in the account.metadata.keyring.type field like:
{
"id": "651e5769-86b1-4717-ae7b-63a00d7b3872",
"address": "0x9f753769a4b45e8e2ed61108f1174b5a0939e08e",
"options": {},
"methods": [
...
],
"type": "eip155:eoa",
"metadata": {
"name": "Ledger 1",
"importTime": 1727081928281,
"keyring": {
"type": "Ledger Hardware"
},
"lastSelected": 1727081928285,
"nameLastUpdatedAt": 1727081928286
}
}And I believe, if we implement a similar AccountList component to display the list of accounts, we will need this keyring.type information to be able to display the small pill Ledger alongside the account entry.
But I think @danroc had a different ideas regarding rendering a AccountList component, so maybe we won't even need to expose any of those metadata :).
I just wanted to be explicit here that type lacks some context/information.
Uh oh!
There was an error while loading. Please reload this page.