Add support for Metamask Snap #1327
Conversation
Update index.spec.ts
Test Extension-dapp/snap
Update defaults.ts Update defaults.ts Delete note
|
Thanks for the PR but why didn't you discuss this in an issue first! Hardcoding such snap and version is probably something we don't want to do. The polkadot package hasn't been released in many years, and still worked until now for the main reason that it has remained very generic. I am very much on the fence regarding this. From my experience, Metamask snaps is a very sub-optimal way to manage accounts for Polkadot. We have many other great wallets that are multi-chain, and do a much better job. The UX of snaps is clunky, you see your balance in some weird UI. Regarding the supported networks, do you have to add them manually? This is for sure opening a can of worms, with default snap etc. You don't want to favor and hardcode a snap rather than another. |
|
Hi @Tbaut, Thank you for your prompt feedback. Firstly, I'd like to clarify that this project was initiated several months ago with the support of W3F. Our project operates independently and currently offers a package available on npm named @polkagate/extension-dapp. By integrating it into the official @ polkadot/extension-dapp package, the entire ecosystem can benefit seamlessly without necessitating a package switch. Regarding versioning, the specified version serves as a foundation, allowing support for all subsequent versions without altering the extension-dapp package itself. While Polkagate Snap is designated as the default option for now, the provided code serves as placeholders for potential future snaps. They are not hardcoded but rather intended to accommodate future snap additions. We acknowledge the existence of other extensions and have indeed developed Polkagate extensions with extensive features. However, the intention behind Metamask snaps is to facilitate the onboarding of Metamask users, particularly those without any native Polkadot extensions installed, into the Polkadot ecosystem. Contrary to the perception of remaining generic and not being updated for years, the @polkadot/extension-dapp is regularly updated, with the latest version being updated on npm just a few hours ago. Furthermore, it's essential to note that connection to extensions can be managed via dapps, granting dapp developers the flexibility to choose which extensions or snaps to utilize. In essence, this PR merely upgrades the @polkadot/extension-dapp package, enabling dapps utilizing this package to integrate Metamask snaps, including Polkagate Snap, as a new option. As for account management, users can handle accounts via the Polkadot apps, and switching chains can be seamlessly achieved by switching chains within the apps, which the snap will recognize. By accepting this PR and publishing the new @polkadot/extension-dapp, and then updating the new version of this package on Polkadot JS apps, Metamask snaps will be another option for communicating with the apps, without the need for additional changes in the Polkadot JS apps code. The snap code is publicly available here, and we're more than willing to address any additional concerns or queries that may arise. Thank you for your attention and feedback. |
|
Generally speaking, there is a lot to unpack in this PR. There is no guarantee it will go in soon, but I will promise to do the research and look into this more deeply, as it requires a deep review. |
|
I'd love to get @rossbulat feedback on this. I wrote you on several other platforms BTW without answers. My main questions would be:
Also we absolutely need to make it fair for other snaps providers, having a hard-coded default doesn't work, there are already several snaps out there. |
No strong opinions. They are good to have as options in the spirit of collaboration & to provide feedback loops to allow them time to evolve & improve. No feedback has been received regarding usage as of yet. Usage statistics are not being tracked. |
Update bundle.ts
|
The only feedback I've ever received from a user using a snap (that was on Solana) was a positive one in conjunction with a wallet app (this is Solarflare) that allows to do things such as:
Now the points 1 and 2 are crucial, if users with no polkadot compatible extension land on an app such as the staking dashboard for the first time, and install a snap from there, they are guaranteed to be totally lost. What's my account, how can I fund it, what's my balance... IMHO the snap would only be good to use with an all in one dashboard such as the Talisman and Subwallet's ones, or Nova Spektr. I purposefully don't mention pjs/apps because it shouldn't be a Dapp for first time Polkadot users. For this reason, I don't think it MM snaps should be pushed by default to all Dapps, or even worse, be forced to users as it's done right now in the staking dashboard, with the MM popup at launch without any user action. It should be available for devs who want to enable it though, without hardcoding anything in this lib. |
|
Polkagate Snap's initial release, already whitelisted and available on the MetaMask directory, focuses on displaying users' balances across different chains via the Snap homepage. I'm excited to share that our upcoming release, already implemented and undergoing testing, will encompass the additional features you mentioned, all within the standalone Snap. Regarding Snap usage, it's designed to benefit users unfamiliar with full-featured extensions like Polkagate extension or Talisman, who may only have MetaMask and are transitioning from the Ethereum ecosystem. By leveraging their familiar extension, they can seamlessly onboard to the Polkadot ecosystem. As for the dashboard issue, we've submitted a fix, which was merged yesterday, and we're actively working on further improvements to be released soon. At Polkagate, we strongly believe that our approach is a win-win solution for bringing users from other ecosystems into the Polkadot ecosystem. We hope that you will see the value in this approach. Thank you for considering our contributions. |
There was a problem hiding this comment.
I'm totally convinced (and I changed my mind) that Snaps have place, and that they may be useful to some users, in some specific conditions. My comments regarding the current implementation remain though. As this is the lib that is used by all dapps, hardcoding a default is not acceptable.
| } | ||
|
|
||
| /** inject snap into window */ | ||
| hasMetamask && (win.injectedWeb3[DEFAULT_SNAP_NAME] = injectedMetamaskSnap); |
There was a problem hiding this comment.
You can't just pick a default here, you should inject all snaps with some convention such as metamask_snap_${snapName}.
| let extensions: Record<string, InjectedWindowProvider>; | ||
|
|
||
| /** If originName indicates enabling only snap */ | ||
| if (['onlysnap', 'only_snap', 'snaponly', 'snap_only'].includes(originName.toLowerCase())) { |
There was a problem hiding this comment.
this looks hacky, why would one only want the snap. Devs can pick the snaps only should they wish to. I'd remove this part.
There was a problem hiding this comment.
I have add inline comment as follows:
/** Since web3Enable enables all available extensions, which is the default behavior
- for Polkadot JS apps, some dapps, like the Staking dashboard, provide an extension
- list where users can choose specific extensions to enable. Therefore, we utilize
- "Snap only" to inject snaps as an additional feature, suitable for such dapps.
- */
There was a problem hiding this comment.
There shouldn't be a default snap. This lib should remain neutral and propose all snaps listed in a file.
… inject snaps like what w3ux in Staking Dashboard needs
We're thrilled to announce the addition of a new 'snap' folder within the extension-dapp directory. This integration empowers websites like polkadotjs, leveraging @polkadot/extension-dapp, to seamlessly connect with PolkaGate's Metamask snap functionality.
Noteworthy Features:
Experience the Power of Polkagate Snap:
Smooth Integration:
Following this merge, all websites utilizing @polkadot/extension-dapp will automatically support Polkagate Metamask snap without requiring any additional updates. Just upgrading to the latest version of @polkadot/extension-dapp will do the trick!