Replies: 9 comments 9 replies
-
|
One unsolved issue is extending already existing chains. For example, adding support for a hardware wallet account for Ethereum Mainnet chain. In current specification, the wallet can't join multiple Snaps to provide support for singular namespace. I currently have no idea on how to solve this problem. Any help appreciated. |
Beta Was this translation helpful? Give feedback.
-
|
Some open questions for any algorithm that could allow multiple Snaps supporting singular chain / namespace:
|
Beta Was this translation helpful? Give feedback.
-
|
glad to know [from twitter](https://twitter.com/cejay35752153/status/1553209565906235392)) this discussion, metamask snaps are a great idea, but only if snaps become first class citizens of metamask.
|
Beta Was this translation helpful? Give feedback.
-
|
Hi @ritave, so my reading of this is that snaps should implement their own version of the For my MPC/TSS use case I think this would work assuming the Is it worth me updating my work on I am a bit concerned about the signature of the importAccount?(chainId: ChainId, data: unknown): Promise<AccountId>;Could we pass the importAccount?(accountId: AccountId, data: unknown): Promise<void>;Then the implementation doesn't need to know how to derive an |
Beta Was this translation helpful? Give feedback.
-
|
@tmpfs It's still a long way before we have the implementation working, and so the design might change midway - I don't think it's worth it to update your default implementation on The wallet itself can't know how to derive Account Ids because it doesn't know how the underlying blockchain works (Is it EVM? Is it Bitcoin? Is it RSA based?), and so the user would have to know how to manually derive their fully CAIP-10 qualified AccountId to be able to provide it during importing process. I'd say that would be a bad user experience, especially for less technically inclined people. Do you have something in mind on how to provide AccountId to |
Beta Was this translation helpful? Give feedback.
-
|
Ah yeh, I was imagining overloading the behavior of When running DKG I end up with a JSON object that represents a share of the private key and I need to add it to the user's account, I thought I could use So I think we need to also expose an optional addAccount?(accountId: AccountId, data: Json): Promise<void>;What do you think? Also, I can imagine a future where the UI needs to differentiate between single party and multi-party keys, for example: The type to describe the properties we would want to associate with the account (when an address is generated from a multi-party key share) would look like: type MultiPartyShare = {
partyNumber: number;
threshold: number;
parties: number;
}Any ideas on the best way to incorporate this into the design? |
Beta Was this translation helpful? Give feedback.
-
|
@tmpfs // Data inputted by the user, probably in a simple textarea in UI in the prototype
const DATA = {
publicKey: '0xffe...ead',
parties: 5,
partyNumber: 2,
threshold: 3,
/*...*/
};
console.log(await keyring.importAccount('dkg:1', DATA));
// saves the state using snap_manageState
// prints 'dkg:1:ffe...ead'Metadata such as Adding markdown to Snaps in the first place, is a big and separate issue from Account Keyrings, and so should come in at a later date after we have a basic prototypes of both working. |
Beta Was this translation helpful? Give feedback.
-
|
I think the |
Beta Was this translation helpful? Give feedback.
-
|
@ritave, I didn't mean to imply we should remove Putting the If we put the multi party information inside the data then the wallet needs to know about the shape of the private key material which I don't think is a good idea as it is brittle. Also it's not sensitive information so if we could let the wallet know about it without looking at the private data then I think that is worthwhile; however, if there is no desire to display this in the UI to differentiate between single party and multi-party keys then it is not worth implementing. Personally, I think it is useful to show this as the signing flow will be significantly different. Of course, like you say we can store it in |
Beta Was this translation helpful? Give feedback.
-
|
@tmpfs I think I understand better now, what you would like to do is have the ability for the user to add some additional information during account creation? As in the below flow? |
Beta Was this translation helpful? Give feedback.
-
Sure thing @ritave. Yes, I imagine that the UI would show a graphic like "ledger" in the screenshot to indicate the key is a share and not the entire key; maybe just "MPC" would be sufficient to distinguish it from a standard single party key. But also showing the threshold and total number of parties would be useful if possible. |
Beta Was this translation helpful? Give feedback.
-
|
@tmpfs Hopefully we will deprecate |
Beta Was this translation helpful? Give feedback.
-
|
MetaMask Snaps had an internal meeting - here are the design notes that came out of that discussion:
|
Beta Was this translation helpful? Give feedback.
-
|
To support SIP-2, MetaMask will need quite a bit of refactoring to support multiple networks at the same time. We're currently exploring what kind of architecture will be needed - I'll update this discussion when we start implementation. |
Beta Was this translation helpful? Give feedback.
-
|
Finally read through this, sorry for the delay! This is looking very good! I’ve written some feedback here, and I’m open to composing it into a PR if you’d like. Copying that feedback here:
Anyways, very excited for this. I hope Elliott will be finding a short path to the multi-simultaneous networks, and hopefully we can get on with implementing this soon. Might be worth helping out with him on that, I swear there's a simple path to it (maybe something like this). |
Beta Was this translation helpful? Give feedback.
-
|
Hey @danfinlay, thank you for the extensive write-up. I've replied below to some feedback and questions, and some was put into #25
|
Beta Was this translation helpful? Give feedback.
-
|
Great, you've addressed most of my questions, thank you.
Sorry I keep thinking of the original beta as a sort of reference specification. In it, each snap had a normal provider, and so was able to request communication channels with other snaps the exact same way that dapps request communication to snaps. I think inter-process communication is a fundamental enough concept that it's worth defining an interface for early. Maybe we should just write that SIP soon. The confused deputy issue I noted will probably apply to any JSON-RPC based inter-process communication standard (unless it uses some kind of local-identifier table the way Fuchsia does), and probably belongs in documentation somewhere until we support a form of object passing as a primary way of exposing interfaces, but that doesn't need to block this, as we're already inheriting JSON-RPC, and we're using it for v1 of our API partly for the sake of ecosystem consistency. |
Beta Was this translation helpful? Give feedback.
-
|
We're currently in a feature freeze until Snaps can be released in the main extension, and so, Keyrings are put on hold. We want to continue building them as soon as we can, but that'll come earliest in third quarter of this year or later. There's a special build for DevCon Bogota that had a partial implementation - the PR in metamask-extension repo is #16107. No new API changes came out of that implementation, but they UI flow is still being researched and worked on. |
Beta Was this translation helpful? Give feedback.
-
This thread is meant to be general place for open discussions concerning the state of SIP-2 (Snap Keyrings).
The SIP is meant to explore minimum viable feature set needed for supporting accounts in Snaps and is expected to be extended in the future (such as declaring a list of assets for accounts and chains).
Beta Was this translation helpful? Give feedback.
All reactions