[Bug]: Approve button disabled for Approvals of ERC20 tokens

marctatham commented 2 months ago

Describe the bug

This issue was already opened here: https://github.com/MetaMask/metamask-mobile/issues/6654 It has since been closed, however it is clearly still an issue

Behaviour:

Approvals are impossible to complete with the Metamask mobile application due to the Approval button being disabled.
The same wallet being used via the metamask browser extension is able to complete the Approval just fine.

Expected behavior

I should be able to approve the transaction, however the approve button is disabled 🙅

Screenshots/Recordings

Recording: https://github.com/user-attachments/assets/7d2cb0f7-b346-4111-a59c-8572d6047b04

Steps to reproduce

connect to UniSwap (i did so using WalletConnect, may or may not be relevant)
initiate a transaction that requires Approval (first time you're interacting with a token, you can revoke previous approvals via something like revoke.cash)
receive the eth_sendTransaction request to perform the approval

Error messages or log output

Approve button is disabled

Detection stage

In production (default)

Version

7.28.1

Build type

None

Device

Pixel 6, OS 15

Operating system

Android

Additional context

As this is expected behaviour the first time you're interacting with a token via the uniswap protocol, it means this will affect ALL users and completely block them from being able to use this sort of service.

In my opinion, this is High in terms of severity.

Severity

No response

jpuri commented 1 month ago

I tried this out in android emulator and could not replicate:

https://github.com/user-attachments/assets/9a9b91c8-55a7-48d6-878e-3f0612d8ddbc

marctatham commented 1 month ago

So @jpuri , i think i've discovered the cause. I've reproduced it with your test dApp: https://metamask.github.io/test-dapp/

when my Metamask wallet has the ethereum network selected, it works ✅ (though i don't have sufficient ETH in there to cover the gas fees)
when my Metamask wallet has the base network selected, it is always disabled

See new recording: https://github.com/user-attachments/assets/75f4972d-c774-41e1-97e5-fc11d8ea6344

sleepytanya commented 1 month ago

@marctatham Thank you for reporting this! I'm trying to reproduce on physical devices. What I see that in some cases I see disabled 'Approve' button for a short time and the 'Next' button appears and I can proceed with approval. I wasn't able to reproduce the bug on Android.

Samsung S24+, Android 14, MetMask 7.29.2

Swap on Base:

https://github.com/user-attachments/assets/f4488c46-c0ce-4eac-ad78-241a9027475f

Malicious ERC20 approval from test dapp:

Uploading maliciousApprovalAndroid.mp4…

ERC20 token approval from test dapp

https://github.com/user-attachments/assets/5af9057b-4ee0-4d62-938a-a30314fcdf1f

sleepytanya commented 1 month ago

iPhone 15, iOS 17.5.1, MetaMask 7.29.0

Approvals from test dapp:

https://github.com/user-attachments/assets/52880f3b-b314-4b3e-8da0-98a589625b6f

Approval from Pancake swaps:

https://github.com/user-attachments/assets/3da1d609-92ae-42ed-94a6-b646202d4cb4

Here you can see disabled 'Approve' button for a short time:

https://github.com/user-attachments/assets/029e82ef-143b-4111-a086-1c4b0eb97aec

marctatham commented 1 month ago

Fascinating... thank you for your effort in attempting reproducing @sleepytanya What's interesting is using my reproduction steps I am able to reproduce this 10/10 (including new recording of transaction that requires approval via uniswap here: https://github.com/user-attachments/assets/6255c13c-9932-4736-8427-38eeb0a50a0c) Perhaps something to do with the state of my wallet 🤷 (worth noting that this is consistent even when I uninstall and re-import this same wallet)

I realise this is unconventional, but as this just is a one of our wallets we use for testing purposes and is very replaceable. If you are open to it, you can share your details with me and I would be open to sharing this wallet's privateKey with you to facilitate your debugging process.

I have a vested interest in seeing this one solved.

sleepytanya commented 1 month ago

@marctatham @jpuri @bschorchit

Unfortunately, we're unable to share test wallet details, but we're equally keen on delving into this bug. I think I can repro the bug: swap WETH to another token. When WETH is a destination token bug doesn't happen.

Uniswap, Base, in-app MetaMask browser, iPhone 15, iOS 17.5.1

USDC -> WETH - success

https://github.com/user-attachments/assets/4bd6ab49-4e62-419d-b16e-e1728c19edbb

WETH -> 1Inch - Aprrove is disabled

https://github.com/user-attachments/assets/5d1e7961-9a59-4b83-bb31-42ae29098695

WETH <-> AXLUSDC - Approve is available only when WETH is destination token

https://github.com/user-attachments/assets/2e1191b9-2f83-46ad-8476-356239f20652

marctatham commented 1 month ago

Amazing that you are able to reproduce! If you need anything else from my side please don't hesitate to get in touch.

If you could be so kind as to let me know when I can hope to see this fix in a new version, that would be appreciated

sleepytanya commented 1 month ago

@marctatham Sure! Will keep you updated.

jpuri commented 1 month ago

Hey @sleepytanya : I tried the above steps, but its is working for me:

https://github.com/user-attachments/assets/860698e6-2bbc-4251-9a58-d9afd78eb8bf

sleepytanya commented 1 month ago

@jpuri

WETH <-> ERC20 works fine on Ethereum, seems like the bug is specific to Base.

'Approve' works on mainnet:

https://github.com/user-attachments/assets/04984795-f229-4168-b857-7fdf89ab3cd8

'Approve' disabled on Base:

https://github.com/user-attachments/assets/d8b5f9f6-7025-4c2a-8864-5752c10d86cb

jpuri commented 1 month ago

I could replicate this issue on Base Sepolia, I found that this is happening we try to interact a token not present on base for instance I was trying to approve 0x4Fabb145d64652a948d72533023f6E7A623C7C53 on base and I found this error and later found that token does not exist on base network.

In these cases error is originating in AssetsContractController and token details, balance are not obtained and this page is blank.

sleepytanya commented 1 month ago

cbETH -> WETH on Base, 'Approval' works:

https://github.com/user-attachments/assets/3fe5423c-edf4-47ba-a517-dfd274068de9

bschorchit commented 1 month ago

I could replicate this issue on Base Sepolia, I found that this is happening we try to interact a token not present on base for instance I was trying to approve 0x4Fabb145d64652a948d72533023f6E7A623C7C53 on base and I found this error and later found that token does not exist on base network.

In these cases error is originating in AssetsContractController and token details, balance are not obtained and this page is blank.

in Tanya's case above, WETH should be present on Base, right?

It seems approving WETH on Base has this issue on mobile, but not on extension. Is there something we might be doing on extension that we're not on mobile?

jpuri commented 1 month ago

Hey @bschorchit from @sleepytanya 's comment WETH transaction is workiing on mobile.

jpuri commented 1 month ago

The issue is caused by rate limiting in Base provider which causes query for user balance and asset details to fail.

marctatham commented 1 month ago

@jpuri what strikes me as key information is that

this transaction (swapping WETH for 1INCH on Base as per my initial video, and my subsequent try that exhibits the same behaviour, which is consistent)
using this wallet
using the Metamask mobile application Fails.

But performing the exact same transaction, using the exact same wallet but using the metamask chrome extension instead of the mobile app works without issue 🤔

jpuri commented 1 month ago

@marctatham : yep that is valid point why its breaking in mobile only. We will check this further.

marctatham commented 3 weeks ago

Thank you to all of you involved in looking into this issue, it is appreciated 🙌

MetaMask / metamask-mobile