Skip to content
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

Add Metamask Snap integration #8070

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

BeroBurny
Copy link

Integrate Metamask Snaps Accounts into Polkadot JS Apps

image

@danforbes
Copy link
Contributor

@jacogr is there anything else you need from us before running these workflows? This work was done as part of a W3F grant and it would be great to get it out there for the community!

Copy link

@takahser takahser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BeroBurny @danforbes thanks for your PR which is part of this delivery.
I added some comments, feel free to check them out.

packages/apps-config/src/extensions/index.ts Outdated Show resolved Hide resolved
Comment on lines 46 to 60
export function injectExtensions (): Promise<boolean[]> {
return Promise.all([
initPolkadotSnap
].map((method) => {
// Timeout injecting extension
return Promise.race([
method(),
new Promise<false>((resolve) => {
setTimeout((): void => {
resolve(false);
}, 1000 /* 1 sec */);
})
]);
}));
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Why does the return time have to be Promise<boolean[]>? After all, initPolkadotSnap is just returning a single Promise<boolean>.

  2. In general, this code could be simplified when using rxjs which already is a dependency:

Suggested change
export function injectExtensions (): Promise<boolean[]> {
return Promise.all([
initPolkadotSnap
].map((method) => {
// Timeout injecting extension
return Promise.race([
method(),
new Promise<false>((resolve) => {
setTimeout((): void => {
resolve(false);
}, 1000 /* 1 sec */);
})
]);
}));
}
export const injectExtensions = () =>
firstValueFrom(from(initPolkadotSnap())
.pipe(
timeout(1000), // timeout if initPolkadotSnap() doesn't resolve after 1s
catchError(() => of(false)),
));

(Please note that I haven't actually tested this code)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Idea is to make it future proof it there will be another wallet like MM snaps so users can easily add it to apps
    for example @polkadot/extension-compat-metamask you can just import initMetaMask and add it to the list to support it

  2. did the same solution with RxJs (personally for me is more complex as I'm uset to async/await js)

@semuelle
Copy link

@BeroBurny / @danforbes, could you resolve the conflicts or resubmit, so this has a chance of being merged?

@danforbes
Copy link
Contributor

Thank you for the feedback, @semuelle! I have informed @BeroBurny and @irubido that this Issue should take priority over the other one you recently commented on - please let me know if you disagree 🙏🏻

@danforbes
Copy link
Contributor

@semuelle - this PR should be up-to-date 🚀 Please let us know if there's anything else we can do to help the team evaluate things. We're getting started on support for parachains, etc now 💪🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants