coinbase-wallet-sdk icon indicating copy to clipboard operation
coinbase-wallet-sdk copied to clipboard

Published 20 hours ago • verified publisher icon coinbase

fix: pass reloadOnDisconnect to WalletSDKRelay

Open RyanTimesTen opened this issue 2 years ago • 6 comments

Summary

Hey @jeongmincho and team, thanks for responding to feedback on this feature ❤️

This is related to: https://github.com/coinbase/coinbase-wallet-sdk/pull/542

How did you test your changes?

Local test app

RyanTimesTen avatar Jun 15 '22 16:06 RyanTimesTen

btw @jeongmincho, how does this work in the context of the coinbase wallet browser extension? does the extension use the SDK, and will it be updated to pass reloadOnDisconnect as false to the SDK?

RyanTimesTen avatar Jun 15 '22 16:06 RyanTimesTen

@RyanTimesTen @petejkim @ljharb @iamseth @smazhara Any updates?

https://github.com/coinbase/coinbase-wallet-sdk/issues/399#issuecomment-1199186223

Pasha8914 avatar Jul 29 '22 11:07 Pasha8914

Waiting for updates here 🙏

agmitron avatar Aug 08 '22 10:08 agmitron

hey @RyanTimesTen! so sorry for the late response here. yes, if you pass in reloadOnDisconnect = false into the SDK, the current setup prevents reload on a user disconnecting. then, you can detect the disconnect event via accountsChanged = [] and handle it accordingly on the dapp.

there's some code that isn't visible on your end due to the integration happening on our wallet codebase. was there anything in particular that wasn't working? cc: @Pasha8914 @agmitron

jeongmincho avatar Aug 31 '22 17:08 jeongmincho

hey @jeongmincho, this isn't working as expected. this is my user authentication flow:

  • (my app) display custom coinbase qr code
  • scan qr code with coinbase wallet
  • (my app) request signature from wallet
  • reject signature request
  • (my app) disconnect coinbase session since signature was rejected
  • reload occurs

i would expect the reload to not occur. am i missing something?

RyanTimesTen avatar Aug 31 '22 20:08 RyanTimesTen

mhm interesting, @RyanTimesTen do you mind providing us a couple details:

  1. what version of the SDK is implemented in your dapp
  2. what coinbase wallet app mobile version you are using
  3. what parameters you're passing into the SDK for reloadondisconnect
  4. how you are handling the accountsChanged event and if there's any reload happening on the dapp logic?

i just tested the following flow and it works:

  1. go to https://app.uniswap.org/#/swap?chain=mainnet (disable CB Wallet extension)
  2. scan the QR code provided on the modal flow with CB wallet mobile
  3. disconnect from the dapp on the CB wallet mobile
  4. no reload is triggered

let me know, thanks!

jeongmincho avatar Sep 01 '22 17:09 jeongmincho

@jeongmincho can you take a look to see whether we can close this?

bangtoven avatar Jan 10 '23 01:01 bangtoven

Hey guys, sorry for dropping comms here. The issue I'm talking about can be seen when triggering disconnect via the SDK, as opposed to the wallet triggering the disconnect. i can put together a repro if that would be helpful

RyanTimesTen avatar Jan 10 '23 02:01 RyanTimesTen

thanks, there seems to be a bug in the relay implementation as raised here and by others. @vishnumad will be helping address this issue and close it out

jeongmincho avatar Jan 10 '23 19:01 jeongmincho

Review Error for vishnumad @ 2023-01-19 01:54:57 UTC User failed mfa authentication, see go/mfa-help

cb-heimdall avatar Jan 19 '23 01:01 cb-heimdall