search

portis-project / web-sdk

Portis Web SDK
https://portis.io
MIT License
77 stars 31 forks source link

portis provider needs better parity with standard web3 (metamask) providers #8

Closed cloudonshore closed 5 years ago

cloudonshore commented 5 years ago

Hey! Really excited about your product, tried integrating it into the airswap frontend but ran into some issues with message signing.

Mainly I'm just looking for parity with metamask. When authenticating with airswap, this is the hashed message that users are required to sign.

Screen Shot 2019-03-23 at 2 22 12 PM

However, when I switch out the window.ethereum from metamask with portis.provider, it doesn't appear that the human-readable interpretation of the message hash is supported

Screen Shot 2019-03-23 at 2 20 20 PM

Also, most importantly, when I try to approve the signing of the message hash, an exception is thrown that the message exceeds the allowed length.

Screen Shot 2019-03-23 at 2 17 37 PM

So even though the first issue just creates some usability issues, the last issue prevents me from integrating with Portis altogether.

Also, as a lower priority issues portis.provider.networkVersion should resolve to the current network chainId. When supporting many different wallet types, it's helpful to create abstractions around providers, and missing features like this will cause a little extra friction with adoption.

Anyway, thanks for your great work! Really excited with what you're doing. Let me know if you need any more info for reproducing the above issues.

radotzki commented 5 years ago

Hi! thanks for the feedback :) Can you please share the code you use to generate and sign the message?

radotzki commented 5 years ago

And I've just released a new SDK version with networkVersion support 😃

cloudonshore commented 5 years ago

I think that I've discovered the issue. I'm using ethers.js and it seems like there is a check in their web3Provider constructor that checks explicitly for metamask, and if it is metamask, it uses personal_sign instead of eth_sign. https://github.com/ethers-io/ethers.js/blob/061b0eae1d4c570aedd9bee1971afa43fcdae1a6/src.ts/providers/web3-provider.ts#L61

This error occurs because ethers.js is trying to use eth_sign instead of personal_sign.

I've fixed this locally by basically faking an isMetaMask: true flag on the portis provider when I initialize the web3 provider in ethers.js. I think it's all working alright now, wanted to let you know the resolution in case anyone else ran into this issue. Thanks for the quick turn around on adding the networkVersion!

Thanks, Sam Walker

radotzki commented 5 years ago

Thanks for letting us know, we will fix it also on our side.