MobileWalletProtocol / wallet-mobile-sdk

An open protocol for mobile web3 apps to interact with wallets
https://mobilewalletprotocol.github.io/wallet-mobile-sdk/
Other
293 stars 110 forks source link

Bug: MMKV Module could not be found #199

Closed felipecamposfabel closed 1 year ago

felipecamposfabel commented 1 year ago

Describe the bug

The react-native-mmkv native module is not being installed with auto-linking on Bare RN Project

Steps

  1. Follow the steps described here
  2. Add this to your project const cbProvider = new WalletMobileSDKEVMProvider();
  3. Run the project on any Device (iOS or Android)
  4. Get the following error: Screenshot_20230307_161906_Bitrefill

I'm running on RN 0.71.3. And I do not use MMKV on my project.

Two side notes.

  1. The github version of the code is not published on npm so the WalletMobileSDKEVMProvider is not exported on @coinbase/wallet-mobile-sdk(need to import it from its file.
  2. This part of the build.gradle file can be problematic if the project runs versions below 1.6.20
    ext.getKotlinVersion = {
    if (ext.has("kotlinVersion")) {
      ext.kotlinVersion()
    } else {
      ext.safeExtGet("kotlinVersion", "1.6.10")
    }
    }

Thanks!

Expected behavior

The project should run as per docs

Version

1.0.4

Additional info

No response

Desktop

No response

Smartphone

No response

cb-jake commented 1 year ago

Hello @felipecamposfabel thanks for reporting. @vishnumad will you look into this issue? Thanks

vishnumad commented 1 year ago

You can provide a storage object as a constructor param in WalletMobileSDKEVMProvider, it just needs to conform to this interface and implement the set, getString, and delete functions.

const customStorage = {
  set: (key, value) => { ... },
  getString: (key) => { ... },
  delete: (key) => { ... }
};

const provider = new WalletMobileSDKEVMProvider({ storage: customStorage });

@bangtoven, @cb-jake - Any thoughts on just removing the react-native-mmkv dependency entirely and always having the consumer inject the storage object into the provider?

iketw commented 1 year ago

It's nice to have a default storage option with the SDK so that we don't have to provide one every time. It would be nice if we didn't need to import mmkv as a dependency in our project and that it would just work by importing the Coinbase SDK.

felipecamposfabel commented 1 year ago

@vishnumad Thanks for the fast reply. Providing a custom storage works!

felipecamposfabel commented 1 year ago

But calling await cbProvider.request({ method: 'eth_requestAccounts', params: [] }); on Android does not open up the Coinbase wallet (it works on iOS). I'm using:

configure({
  hostURL: new URL('https://wallet.coinbase.com/wsegue'),
  callbackURL: new URL('myscheme://'),
  hostPackageName: 'org.toshi',
});

EDIT: using Linking.openURL('https://wallet.coinbase.com/wsegue'); opens the Coinbase Wallet. So I'm not sure what's going on...

vishnumad commented 1 year ago

@felipecamposfabel Are you seeing any errors in console or in Android logcat? Could you also try calling the initiateHandshake function directly and see if you're still seeing the same issue?

await initiateHandshake([
  {
    method: 'eth_requestAccounts',
    params: {}
  }
]);
felipecamposfabel commented 1 year ago

@vishnumad There's nothing on logcat nor on the console. Tried initiateHandshake with no success either. The promise never resolves, apparently.

vishnumad commented 1 year ago

@felipecamposfabel I wasn't able to reproduce the issue using our example app. Would you be able to share an example where I can reproduce the issue?

https://user-images.githubusercontent.com/3137711/223587604-1363caf6-83a4-47cb-b508-94ba98e93733.mp4

felipecamposfabel commented 1 year ago

@vishnumad I'll try to provide you with an App. In the meantime, after reinstalling my App on Android the app now crashes every time I call request. Here's the logcat:

--------- beginning of crash
03-08 09:55:52.308 10911 11058 F libc    : Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x718396cfe0 in tid 11058 (mqt_js), pid 10911 (m.xxx.app)

Also on iOS, on every reload of the app, the app freezes with a CoinbaseWalletSDK.configure should be called only once. Apparently the isConfigured/hasConfigured variable is not exposed for RN, so I need to close and reopen the app every time I need to reload...

felipecamposfabel commented 1 year ago

@vishnumad Here it is: https://github.com/felipecamposfabel/coinbase

vishnumad commented 1 year ago

@felipecamposfabel Thanks for providing that example. I just released version 1.0.5 that should fix the issue you were running into with handshakes.

felipecamposfabel commented 1 year ago

@vishnumad Thanks for the update. With version 1.0.5 the Android app connects to Coinbase Wallet. But every time the user cancels any request/transaction, when coinbase redirects to the app, it just crashes on Android. You can try it on the app I sent you.

EDIT: The app also crashes on iOS

Also, why do I need to persist the connected address myself (since it's undefined every time the app is relaunched)? Shouldn't the selectedAddress be synced with the storage?