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

feat: safepalWallet support #1506

Merged
merged 11 commits into from
Oct 15, 2023
Merged

feat: safepalWallet support #1506

merged 11 commits into from
Oct 15, 2023

Conversation

QiAbc
Copy link
Contributor

@QiAbc QiAbc commented Sep 6, 2023

Please let me know if you have any questions, thank you

@QiAbc QiAbc requested a review from a team as a code owner September 6, 2023 10:53
@vercel
Copy link

vercel bot commented Sep 6, 2023

@QiAbc is attempting to deploy a commit to the rainbowdotme Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Sep 6, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rainbowkit-example ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2023 9:23pm
rainbowkit-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2023 9:23pm

@DanielSinclair
Copy link
Collaborator

@QiAbc Going forward, RainbowKit will adopt EIP-6963 for injected wallets. It is recommended that you begin supporting this standard in your wallet via mipd. This will automatically be supported for dApps in an upcoming release of RainbowKit alongside Wagmi v2. You can test your wallet against a test dApp at eip6963.org in the meantime. We will still work towards merging this PR for WalletConnect support, but we will soon strip out the injection logic implemented here.

@DanielSinclair
Copy link
Collaborator

Seeing a minor deployment error. Running pnpm run lint:fix should resolve the issue

41:3 Error: Member 'safepalWallet' of the import declaration should be sorted alphabetically. sort-imports

@QiAbc
Copy link
Contributor Author

QiAbc commented Sep 7, 2023

Seeing a minor deployment error. Running pnpm run lint:fix should resolve the issue

41:3 Error: Member 'safepalWallet' of the import declaration should be sorted alphabetically. sort-imports

It has been resubmitted, please take a look at it

@QiAbc QiAbc closed this Sep 7, 2023
@QiAbc QiAbc reopened this Sep 7, 2023
@QiAbc
Copy link
Contributor Author

QiAbc commented Sep 7, 2023

Seeing a minor deployment error. Running pnpm run lint:fix should resolve the issue

41:3 Error: Member 'safepalWallet' of the import declaration should be sorted alphabetically. sort-imports

It has been resubmitted, please take a look at it

@QiAbc
Copy link
Contributor Author

QiAbc commented Oct 7, 2023

Seeing a minor deployment error. Running pnpm run lint:fix should resolve the issue

41:3 Error: Member 'safepalWallet' of the import declaration should be sorted alphabetically. sort-imports

When will the code be merged?

@DanielSinclair
Copy link
Collaborator

@QiAbc When QAing, I am seeing quite a few issues:

  1. For unsupported networks when attempting to switch via WalletConnect, it appears an error isn't properly returned from the mobile wallet
Screenshot 2023-10-15 at 5 11 08 PM 2) I was unable to successfully test the injection flow; it appears the inpage script is throwing an error. Otherwise, this is possibly one of the following: `window.ethereum` is not detected at load-time, possibly because window.ethereum.isZeal is injected too slowly 2) the provider is failing Wagmi's ready logic here: https://github.com/wagmi-dev/wagmi/blob/38306606d2fd72a4c6918323bf86a1afda348638/packages/connectors/src/injected.ts#L88 Screenshot 2023-10-15 at 5 11 12 PM

@DanielSinclair
Copy link
Collaborator

I'll proceed to merge, but note that the extension path is not functional for end users on the wallet side

@DanielSinclair DanielSinclair merged commit 02e796c into rainbow-me:main Oct 15, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
connector New wallet connector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants