search

MetaMask / metamask-mobile

Mobile web browser providing access to websites that use the Ethereum blockchain
https://metamask.io
Other
2.15k stars 1.11k forks source link

EIP-681 deeplink for ERC-20 function "approve" does not work in MetaMask mobile 1.05 #1980

Closed ravisriv closed 3 years ago

ravisriv commented 3 years ago

Describe the bug This issue has reference to: Previously reported bug regarding EIP-681: https://github.com/MetaMask/metamask-mobile/issues/1828 Feature EIP-681: https://github.com/MetaMask/metamask-mobile/pull/1847

Please refer to EIP-681 and EIP-831 standards. These standards have been implemented to create a deeplink and QR code here, which we will call MetaMask version. A more advanced version of EIP-681 has been implemented by Bruno Barbieri here, which we will call Bruno's version.

MetaMask's and Bruno's versions (of EIP-681 deeplink generators) are functionally equivalent and provide an excellent way of cross-checking creation of EIP-681 compliant deeplinks for MetaMask mobile. However, MetaMask version is limited to ERC-20 "transfer" function only, while Bruno's version can support all of ERC-20 functions.

For example, MetaMask version generates a deeplink of the following type: https://metamask.app.link/send/pay-Contract-Address@chain-id/transfer?address=Receiver-Address&uint256=1e21

Notice that it includes "Contract Address", "Chain id", ERC-20 "transfer" function, "Receiver Address" and the amount of tokens being transferred.

Bruno's advanced version can generate a deeplink of the same type as follows: ethereum:pay-Contract-Address@chain-id/transfer?address=Receiver-Address&uint256=1e21

When the QR code generated by these deeplinks (MetaMask and Bruno's version both) for "transfer" function is scanned by MetaMask mobile, it works perfectly. The MetaMask mobile transfers correct amount of ERC-20 tokens to "Receiver-Address."

So far so good.

Now instead of inserting ERC-20 function "transfer" in Bruno's version (since MetaMask version is hardcoded for "transfer" function only), we will insert "approve". "approve" is a valid ERC-20 function. Under EIP-681 it should be supported. In fact, "approve" is one of the important methods of engaging with a smart contract.

So we can generate a QR code for ERC-20 "approve" function. We generated one below:

ethereum:pay-Contract-Address@chain-id/approve?address=Receiver-Address&uint256=1e21

(Please note that during test actual Ethereum addresses were used. I'm using "Contract-Address", "chain-id", "Receiver-Address" for ease of understanding.)

MetaMask mobile fails to process the QR code generated with "approve" function in MetaMask-mobile version 1.05 on iPhone X iOS 14.1.

Next issue is that deeplinks should work like deeplinks. Therefore, if I email someone the following deeplink with a valid "Contract-Address", "Receiver-Address" and other parameters, the receiver of the email should be able to click on the link to invoke the MetaMask to send payment. They should also be able to "approve" payment, in case I want to withdraw payment from their wallet at a later date. While deeplink for "transfer" function works fine, the following deeplink for "approve" function fails: https://metamask.app.link/send/pay-Contract-Address@chain-id/approve?address=Receiver-Address&uint256=1e21

Screenshots image image

After I replace "transfer" function with "approve" function, the following deeplinks do not work in MetaMask-mobile: image image

To Reproduce

  1. Prepare a basic ERC-20 contract to create some ERC-20 tokens. This is the target contract. Get the address of the target contract, which is "Contract-Address".
  2. Go to Bruno's version of deeplink generator at https://brunobar79.github.io/eip681-link-generator/#. The first question, it will ask is "What kind of URL do you want go generate?" Select the "Other (advanced)" button - yellow one on the right.
  3. Enter ERC-20 contract address (Target Address or Contract-Address) in the "Target Address" box.
  4. Chain id will be 4 if you are using Rinkeby.
  5. Enter function name as "approve".
  6. Click on "Add Parameter" to add 2 key-value parameters. Enter "address" as the first key. The value of this key will be the Receiver-Address, the Ethereum address of the contract which you want to be able to withdraw tokens from your wallet.
  7. The second key-value pair will have the key as "uint256" and its value will be the number of tokens you want to approve the other Receiver-Address to be able to spend from your wallet.
  8. Click on the green button below to generate the URL.
  9. It will generate a link as well as the QR code.
  10. Scan the QR code using MetaMask-mobile version 1.05.
  11. MetaMask-mobile will display an incorrect message. It will show 0 Ether being sent. When you click on the "Send" button, the transaction will be executed on the Rinkeby blockchain and it will fail. All "approve" transactions fail.

An alternate approach to test it is to generate a link using the MetaMask version of deeplink generator. It will generate a deeplink with "transfer" function. Copy and edit the deeplink to replace "transfer" function with "approve" function. After that email the link to yourself and click on the link from your iPhone. The "approve" transaction will fail with incorrect messages.

Expected behavior Clicking on the deeplink with "approve" function or scanning the QR code with "approve" function should display this message: Allow "Receiver-Address" to spend "uint256" amount of "ERC20 token name" from your wallet? If the user allows that, ERC-20 "approve" function of the "target contract" will be executed to allow the receiver to spend money from the approvers wallet.

Smartphone (please complete the following information):

to be added after bug submission by internal support / PM Severity

ravisriv commented 3 years ago

Hi @estebanmino, You had very kindly added a new feature for EIP-681 under Feature #1847 to address the issue that I had reported under issue #1828 . I extensively tested the feature EIP-681. While it works perfectly for "transfer" function, it does not work for "approve" function. Instead of reopening #1828, I decided to re-document my testing to provide further clarification. Shortly, I'll add additional screenshots to support further analysis of the issue. Deeply appreciate your support. Thank you!

ibrahimtaveras00 commented 3 years ago

@ravisriv https://github.com/MetaMask/metamask-mobile/pull/1847 is not live yet

It should be live in a few days on: android = v1.0.6 (40) iOS = v1.0.6 (548)

Once live, please test again and let us know if you're still experiencing any issues

ravisriv commented 3 years ago

Thank you @ibrahimtaveras00. I'll keep this issue open for a few days and I'll test version 1.06 as soon as it is released. After that I'll close this issue. Deeply appreciate your response.

ravisriv commented 3 years ago

I tested newly released version 1.06 this morning. Specifically, I performed the following EIP681 tests:

https://metamask.app.link/send/0x3dD3DfaAdA4d6765Ae19b8964E2BAC0139eeCb40@4/approve?address=0x178e3e6c9f547A00E33150F7104427ea02cfc747&uint256=5e8

In addition, I scanned the "approve" QR code generated by EIP681 deeplink generator (by Bruno Barbieri).

Great news is that EIP681 "approve" functionality that I tested is working! Messages and UI are excellent. The UI even allows me to change the amount that I want to "approve". Super job by @estebanmino, @ibrahimtaveras00 and the team! Heartiest congratulations!

Just a minor issue is that it appears that MetaMask 1.06 version is not getting some kind of return code from the blockchain. That is a different issue, which I'll record under a different number. I'm closing this one now!