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: support multi wallet connector #201

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Conversation

RetricSu
Copy link
Collaborator

@RetricSu RetricSu commented Sep 29, 2022

this pr supports multiple wallet connectors via using web3-react lib. For injected webview environment (eg: Metamask/ImToken/SafePal), the connector use Metamask connector, we only add different icons to better identify the wallet, same as Yokai Swap.


some todos before merging this pr:

  • try wallet-connect with a mobile wallet to see if the process is normal
  • check if the previous provider used in light-godwoken library is compatible with new wallet connector providers.

@vercel
Copy link

vercel bot commented Sep 29, 2022

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

Name Status Preview Updated
godwoken-bridge-mainnet ✅ Ready (Inspect) Visit Preview Oct 9, 2022 at 8:00AM (UTC)
godwoken-bridge-testnet ✅ Ready (Inspect) Visit Preview Oct 9, 2022 at 8:00AM (UTC)

@RetricSu RetricSu changed the base branch from ref-custom-tokens to develop October 9, 2022 07:48
@RetricSu RetricSu changed the title wip: support multi wallet connector feat: support multi wallet connector Oct 9, 2022
@RetricSu RetricSu changed the title feat: support multi wallet connector Feat: support multi wallet connector Oct 9, 2022
@RetricSu RetricSu marked this pull request as ready for review October 9, 2022 08:15
@ShookLyngs
Copy link
Collaborator

ShookLyngs commented Oct 10, 2022

Testing

I've tested for a round or two, here's the connection test results for the newly added wallets.
Testing process:

  1. Connect to wallet
  2. Add and switch to testnet network
  3. Deposit capacity to L2

imToken

No issues found

WalletConnect (MetaMask&imToken):

  1. Cannot get L2 balance
instrument.ts:124 Error: Cannot request JSON-RPC method (eth_getBalance) without provided rpc url
    at t.default.request (index.ts:60:13)
    at Et.jsonRpcFetchFunc (web3-provider.ts:96:25)
    at Et.send (web3-provider.ts:167:21)
    at Et.<anonymous> (json-rpc-provider.ts:601:31)
    at Generator.next (<anonymous>)
    at 2.8653ed8f.chunk.js:2:1777567
    at new Promise (<anonymous>)
    at se (2.8653ed8f.chunk.js:2:1777312)
    at Et.perform (2.8653ed8f.chunk.js:2:1787048)
    at Et.<anonymous> (base-provider.ts:1441:35)
    at Generator.next (<anonymous>)
    at o (2.8653ed8f.chunk.js:2:1745275)

Safepal

  1. First time visiting the page, godwoken-bridge is already connected without asking
  2. Safepal doesn't support adding networks, and testnet is not in Safepal's built-in networks
  3. L2 balance is always 0
  4. Close and reopen Safepal, godwoken-bridge is now disconnected for no reason (tested for more round, it is not always disconnected when reopening the app), and when clicked "Connect" button and selected "Safepal", the modal closes and nothing else happens

@RetricSu
Copy link
Collaborator Author

thx, I will check the code and see how to resolve

@Flouse Flouse marked this pull request as draft December 1, 2022 03:41
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.

2 participants