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 an integration with azero-wallet snap #95

Merged
merged 26 commits into from
Feb 22, 2024

Conversation

piotr-roslaniec
Copy link
Collaborator

@piotr-roslaniec piotr-roslaniec commented Dec 16, 2023

  • Based on r-8.7
  • Merged changes up to r-9.1
  • Adds integration with azero-wallet snap. Uses said snap as a new signer.

@jalooc jalooc requested a review from Marcin-Radecki January 11, 2024 09:43
Copy link

@jalooc jalooc left a comment

Choose a reason for hiding this comment

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

So far looks good. It's a draft PR, so I imagine there are some more change coming? (hence not approving yet)

packages/apps/src/Apps.tsx Outdated Show resolved Hide resolved
packages/react-signer/src/snap.ts Outdated Show resolved Hide resolved
packages/react-signer/src/TxSigned.tsx Show resolved Hide resolved
@jalooc
Copy link

jalooc commented Jan 11, 2024

Oh, I also asked @Marcin-Radecki @youPickItUp to co-review, because he's more up-to-date with this project.

@jalooc jalooc requested review from youPickItUp and removed request for Marcin-Radecki January 11, 2024 11:44
Copy link

@youPickItUp youPickItUp left a comment

Choose a reason for hiding this comment

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

Looks good! I had problem testing 😅 How can I test whole flow with snap? I get
image

yarn.lock Show resolved Hide resolved
packages/page-accounts/src/Accounts/index.tsx Outdated Show resolved Hide resolved
packages/react-signer/src/TxSigned.tsx Outdated Show resolved Hide resolved
packages/react-signer/src/TxSigned.tsx Outdated Show resolved Hide resolved
packages/react-signer/src/snap.ts Outdated Show resolved Hide resolved
@piotr-roslaniec
Copy link
Collaborator Author

Hi @youPickItUp. Until this snap registry PR is merged, you have to use the "Flask" development build of MetaMask

Copy link

@youPickItUp youPickItUp left a comment

Choose a reason for hiding this comment

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

Sorry, again couldn't get it to work 😢
Probably need some assistance 🙏
More in comment

packages/page-accounts/src/Accounts/index.tsx Outdated Show resolved Hide resolved
@piotr-roslaniec piotr-roslaniec marked this pull request as ready for review February 3, 2024 09:37
@youPickItUp
Copy link

Hello! I'm sorry for the delay. I wanted to deploy your change on our stage, but before, I've re-tested it and I'm afraid it's not working 😬 Perhaps I mess edsomething up, but it seems to hang on snap.connect() indefinitely. Could you assist me?

@piotr-roslaniec
Copy link
Collaborator Author

@youPickItUp Let's see if we can debug this remotely. Could you provide me with some extra information about your extension? You can find them in the Extension details view in Google Chrome (Mozilla Firefox has analogous section):
image

The background.html file contains a process used by the extension. You can find more information about the extension there. This is how the console looks like upon successful snap.connect():

image

@youPickItUp
Copy link

youPickItUp commented Feb 11, 2024

image

I don't see any change in the background.html console when I click the connect, some errors appear when I open the extension, but don't seem dangerous 🤷
image

The window.ethereum.isMetaMask is set to true.

I don't have the snap installed yet.

@youPickItUp
Copy link

I figured it out - I had Encrypt extension that somehow interfered with sending message to MetaMask 🤯
Sorry for inconvenience 😅

@youPickItUp
Copy link

Could you look into Enkrypt blocking Metamask integration issue? I'm a bit afraid this could be a somewhat common issue 😅

@piotr-roslaniec
Copy link
Collaborator Author

@youPickItUp I'm afraid it's another variation of a classic "browser wallets fighting over the monopoly of window.ethereum" problem. I will look into it - The least I can do is throw an error and ask the user to reconnect their wallet.

packages/react-signer/src/signers/MetaMaskSnapSigner.ts Outdated Show resolved Hide resolved
packages/react-signer/src/TxSigned.tsx Outdated Show resolved Hide resolved
@piotr-roslaniec piotr-roslaniec force-pushed the azero-wallet branch 4 times, most recently from e5aa1b9 to 7eaf103 Compare February 21, 2024 14:58
@piotr-roslaniec piotr-roslaniec force-pushed the azero-wallet branch 4 times, most recently from 2d40b02 to 8656fcd Compare February 21, 2024 18:38
@youPickItUp youPickItUp added this pull request to the merge queue Feb 22, 2024
Merged via the queue into Cardinal-Cryptography:alephzero with commit 57e90d3 Feb 22, 2024
5 checks passed
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.

3 participants