search

WalletConnect / WalletConnectFlutterV2

WalletConnect v2 client made in Dart for Flutter.
https://pub.dev/packages/walletconnect_flutter_v2
Apache License 2.0
101 stars 52 forks source link

add chainId to registerRequestHandler handler #261

Closed Rimantovas closed 4 months ago

Rimantovas commented 4 months ago

Description

Added chainId as a parameter in the registerRequestHandler method. This helps identify which chainId the dApp is currently using.

Without this it is hard to tell which chain to use in the request since you can pass multiple chains to dApp (attaching a log of how session namespaces look when registering multiple chains):

{eip155: Namespace(accounts: [eip155:56:0xA0E86987Cc1e360B4BB191bcEf0548AD86Ed57A8, eip155:1:0xA0E86987Cc1e360B4BB191bcEf0548AD86Ed57A8], methods: [eth_sendTransaction, personal_sign], events: [chainChanged, accountsChanged])}

Resolves #258

How Has This Been Tested?

Tested the changes on the example wallet app as well as my own app. Everything works as expected.

quetool commented 4 months ago

Hello @Rimantovas! Thanks for opening this PR. It looks good at first glance! I'll take a closer look ASAP and include in the next beta if proceeds! Thanks!

quetool commented 4 months ago

Hello @Rimantovas! Thanks for waiting! After evaluating your PR we have found an issue in Flutter's SDK that, after fixing it, your concern is going to be solved. Furthermore this PR of yours represents a breaking change. Because of these 2 reasons we decided not to include it but we really appreciate it! Based on your legit concern we were able to find a non-previously reported issue! The proper fix is going to be pushed during this week along with a documentation change! Thanks!

quetool commented 4 months ago

Please check! https://github.com/WalletConnect/WalletConnectFlutterV2/issues/258#issuecomment-1959494085