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 algorand snap as wallet option #112

Closed
wants to merge 2 commits into from

Conversation

paulfears
Copy link

Description

Included the snapalgo-sdk

@pbennett
Copy link
Collaborator

How are you using the algorand snap at this point and testing it? The only interface to manipulate accounts, etc. appears to be the old https://snapalgo.io (with the bizarre cut-off algorand logo as the 'interface'). Yet connecting an account and doing something simple like showing the receive address yields an error like this in the released version of metamask w/ official algorand snap:
image

@SilentRhetoric
Copy link
Contributor

SilentRhetoric commented Sep 24, 2023

How are you using the algorand snap at this point and testing it? The only interface to manipulate accounts, etc. appears to be the old https://snapalgo.io (with the bizarre cut-off algorand logo as the 'interface'). Yet connecting an account and doing something simple like showing the receive address yields an error like this in the released version of metamask w/ official algorand snap:

The npm package was renamed, so you can connect to the snap with:

params: { 'npm:@algorandfoundation/algorand-metamask-snap': {}, }

@paulfears
Copy link
Author

paulfears commented Sep 24, 2023 via email

@pbennett
Copy link
Collaborator

My bad it I was pretty tired and I think I submitted a pull request on the wrong package

I'm just curious how we test it and add accounts, etc so that use-wallet can then "use" it ?
Apparently the snap framework is extremely limiting and that any interface to manage accounts actually has to be external to metamask.

@drichar
Copy link
Collaborator

drichar commented Sep 25, 2023

Thank you @paulfears for taking the time to make a PR adding the Metamask Algorand Snap provider to use-wallet. As @pbennett mentioned, when we last investigated adding this feature the Snap didn't seem to be fully functional. Were you able to get it working in the library before submitting your pull request?

My bad it I was pretty tired and I think I submitted a pull request on the wrong package

I'm confused by this comment. Are you referring to this pull request? Should it remain open?

If so, a few things jump out at me right away. There is an unresolved yarn.lock merge conflict causing the build to fail. Also, it seems like you copied the AlgoSigner provider's types and renamed them to Metamask, but I can't imagine their type signatures are identical.

The pull request template lists requirements that must be satisfied before it can be reviewed:

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary

Your PR currently doesn't meet any of these requirements.

Let me know if you have any questions; I'll be happy to answer them for you. I hope we can figure out how to add your Metamask Snap to the library!

@paulfears
Copy link
Author

I will close this pull request and resubmit. The floating interface is required to provide UI to a Metamask snap, but the SDK was developed using an older version of the snap, that does not work on the consumer version of Metamask, so that will need to be swapped out. As far as the other issues outlined I will look into them before resubmitting, thank you guys for the feedback.

@paulfears paulfears closed this Sep 25, 2023
@drichar
Copy link
Collaborator

drichar commented Sep 26, 2023

Sounds good, thank you!

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

Successfully merging this pull request may close these issues.

4 participants