shapeshift / web

ShapeShift Web
https://app.shapeshift.com
MIT License
159 stars 180 forks source link

feat: walletconnect wallet #1733

Closed GMSteuart closed 1 year ago

GMSteuart commented 2 years ago

Description

Integrates WalletConnect wallet

UPDATE: Moved the provider object from hdwallet over to web since there is a scenario where the connect modal will hang indefinitely and there is no graceful way to automatically exit on behalf of the user. This indefinite situation occurs after the QR modal pops up and the user rejects the session. The reason this occurs is because the connector at the time is only listening for connect events and the only event emitted is the disconnect event; alas, the connector enters a state of waiting with no apparent end. The code of concern can be found in WalletConnect's web3-provider.

Notice

Pull Request Type

Issue (if applicable)

Closes #1686

Risk

Testing

Open a terminal for web and another for hdwallet.

# /hdwallet
yarn
yarn build
cd packages/hdwallet-walletconnect
yarn link
# hdwallet-walletconnect tests
 yarn test packages/hdwallet-walletconnect/src/walletconnect.test.ts
#  hdwallet-walletconnect integration tests
cd integration
yarn test src/walletconnect.test.ts
# /web
# clone this branch 
gh pr checkout 1733
yarn clean
yarn link "@shapeshiftoss/hdwallet-walletconnect"
# update package.json "@shapeshiftoss/hdwallet-walletconnect": "^1.21.2"
# to use filepath "@shapeshiftoss/hdwallet-walletconnect": "../hdwallet/packages/hdwallet-walletconnect"
# the above assumes your directory structure is setup like:
# .
# ├── hdwallet
# ├── lib
# └── web
yarn
open https://test.walletconnect.org/
yarn dev
  1. Sign in with some other wallet
  2. Go to flag settings
  3. Turn on WalletConnectWallet flag and save
  4. Disconnect from current wallet
  5. Go through wallet sign in process but select WalletConnect this time around

Screenshots (if applicable)

Screen Shot 2022-05-18 at 3 33 42 PM

Screen Shot 2022-05-18 at 3 34 36 PM

Screen Shot 2022-05-18 at 3 35 26 PM

@0xdef1cafe copy on pairing modal will need to be updated since a QR code pops up:

Screen Shot 2022-05-18 at 3 35 59 PM

Screen Shot 2022-05-12 at 3 41 20 PM

Screen Shot 2022-05-18 at 3 43 58 PM

On test.walletconnect.org Screen Shot 2022-05-18 at 3 48 27 PM

Screen Shot 2022-05-18 at 3 51 06 PM

And lastly on test.walletconnect.org Screen Shot 2022-05-18 at 3 51 53 PM

GMSteuart commented 2 years ago

@0xdef1cafe will need product review for user flow when clicking WalletConnect option

gomesalexandre commented 2 years ago

@GMSteuart I suppose this needs linking with https://github.com/shapeshift/hdwallet/pull/529 for now, could you add some instructions for local testing until hdwallet-walletconnect is published?

gomesalexandre commented 2 years ago

Also, seems like the diff touches many files/lines that shouldn't be modified by this PR, rebase/merge might help 👀

GMSteuart commented 2 years ago

@GMSteuart I suppose this needs linking with shapeshift/hdwallet#529 for now, could you add some instructions for local testing until hdwallet-walletconnect is published?

Ya, I can add local testing steps, one of those will have to be running the associated hdwallet-walletconnect package that isn't published yet.

GMSteuart commented 2 years ago

Also, seems like the diff touches many files/lines that shouldn't be modified by this PR, rebase/merge might help 👀

Oh fun. lol. Glad I'm finding out earlier in the day rather than later. Thanks for pointing that out. 🙂

gomesalexandre commented 2 years ago

@GMSteuart I suppose this needs linking with shapeshift/hdwallet#529 for now, could you add some instructions for local testing until hdwallet-walletconnect is published?

Ya, I can add local testing steps, one of those will have to be running the associated hdwallet-walletconnect package that isn't published yet.

Built, linked, and ran web off the hdwallet PR branch but I'm seeing this, so I'm wondering whether or not there are more steps or something I missed:

image
sababa-2019 commented 2 years ago

bumping this, any updates here @GMSteuart ser?

0xdef1cafe commented 1 year ago
image

i connected a wallet with eth, btc, and atom assets

i can see tx history for eth and atom but not bitcoin

i can only see account data for eth

i'm only seeing eth account specifiers in the client

image

plz fix

GMSteuart commented 1 year ago

Screen Shot 2022-06-09 at 6 12 49 PM

@0xdef1cafe no longer a fuzzy fox 🙂

pastaghost commented 1 year ago

i connected a wallet with eth, btc, and atom assets i can see tx history for eth and atom but not bitcoin i can only see account data for eth i'm only seeing eth account specifiers in the client

I think this is an hdwallet issue. I'll make the necessary changes and publish a new alpha release, then include the updated dependency here.

edit: @0xdef1cafe Correction on this - WalletConnect 1.0 only supports ETH.

gomesalexandre commented 1 year ago

This is currently blocked by the hdwallet WalletConnect integration: https://github.com/shapeshift/hdwallet/pull/544

pastaghost commented 1 year ago

Keplr also supports WalletConnect, and can be used to connect to e.g osmosis.zone. It isn't supported at all as the app is effectively in full skeleton mode for dashboard and has no loaded accounts in state. Do we want to handle the case of Keplr Mobile (and Cosmos/Osmosis accounts derivation in WalletConnect) not being supported, by having some UX for it?

This is because WalletConnect 1.0 (the version we've implemented in HDWallet) is ETH-only. WalletConnect 2.0 has multi-chain support but is still in beta. I'm currently looking to see if it is possible to either detect the user wallet connected on the other side of the WalletConnect bridge or to limit the set of wallets supported by the application via WalletConnect.

pastaghost commented 1 year ago
Screen Shot 2022-07-19 at 3 59 33 PM Screen Shot 2022-07-19 at 3 59 16 PM Screen Shot 2022-07-19 at 3 58 52 PM
0xdef1cafe commented 1 year ago

merging as previously approved by @gomesalexandre and the most recent commit was stylistic/trivial