5afe / safe-react

Deprecated! New repo – https://github.com/safe-global/web-core
MIT License
332 stars 363 forks source link

Issues confirming Tx (WalletConnect w/ Trust Wallet) #350

Closed lukasschor closed 4 years ago

lukasschor commented 4 years ago

Bug report from user:

I am currently trying to use the team safe with wallet connect + trust wallet. I could connect the wallet and I could trigger a tx request. However when I submitted the tx on trustwallet nothing happened. In the team safe I found those error messages on the console

Environment: Production Mainnet, using Trust Wallet through Walletconnect

Screen Shot 2019-12-14 at 14 49 37

fernandomg commented 4 years ago

@lukasschor, these are my findings so far:

In regard to Trust/WalletConnect, I was able to connect but when tried to make a transfer from the safe to another account I get the following in the trust wallet: image

On the other hand, using Nifty I was able to submit the tx: image

My hunch is that there's something wrong in the payload it's being sent to the Trust Wallet. But as I don't know much about WalletConnect maybe one of the devs that worked on its implementation can give me a better insight.

In the meantime, I'll have a look at the docs/implementation.

fernandomg commented 4 years ago

I'm removing myself from the 'assignee'

lukasschor commented 4 years ago

We should take another look at this issue as we have quite some users complaining about not being able to sign with Trust. If we conclude that this is an issue on the side of the Trust wallet, please notify the Trust team so they can take a look.

nicosampler commented 4 years ago

@tschubotz I think this issue can be fixed when this PR https://github.com/gnosis/safe-react-apps/issues/49 is merged. If wallet-connect receives a method different than eth_sendTransaction we reject the request, as far as I understand if the DApp handles the rejection correctly the users won't have problems.

tschubotz commented 4 years ago

@nicosampler are you sure? In WalletConnect, there are 2 sides: Wallet and dapp.

The linked PR is about the Safe being the Wallet. However in the issue with Trust, the Safe is the dapp. So I think that's a different case, no?

(The underlying issue of not handling eth_ call correctly might be the same though.

nicosampler commented 4 years ago

@tschubotz you right, sorry.

dasanra commented 4 years ago

Sending funds transaction worked perfectly today. Experienced random issues until today and multiple Trust Wallet updates later I can confirm that for basic operation it works.

Once I even got the message of 'invalid network', other days I got simply console errors.

Nothing changed from our side to get this result except updating Trust Wallet app to an stable version.

Currently for Android v1.19.15

tschubotz commented 4 years ago

@tschubotz test this on iOS

tschubotz commented 4 years ago

I just tested with iOS 5.3 (5313) and I was able to sign off chain as well as execute txs without any problems. -> Closing