MetaMask / metamask-mobile

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

[Bug]: Transactions and Gas - `NAN ETH` when sending a deeplink transaction with another transaction pending in the wallet #9261

Open seaona opened 6 months ago

seaona commented 6 months ago

Describe the bug

Whenever we have a pending transaction in the wallet, and we trigger a deeplink from the browser we see:

  1. Another tx modal is open, overlapping the underlaying tx --> not sure this is okay
  2. If I accept the new tx, I see how the gas for the previous tx are set to NAN ETH --> this is another way to reproduce this issue, which can also be triggered doing this alternative steps https://github.com/MetaMask/metamask-mobile/issues/6481

Expected behavior

  1. For this one, I'm not sure what we should expect here, or how handling such cases, cc @bschorchit @cryptotavares
  2. For two, we should fix the gas issue and update the corresponding nonce

Screenshots/Recordings

Screenshot from 2024-04-16 15-17-47

https://github.com/MetaMask/metamask-mobile/assets/54408225/3e78b595-c385-4ec5-aee6-fdd83b3daf48

Steps to reproduce

  1. Go to the test dapp in your computer
  2. Select Sepolia
  3. Deploy an ERC20 contract with your extension wallet
  4. Replace your contract address here https://metamask.github.io/test-dapp/?contract=0x8aAF999b9bDf4BF6E5fAa267D635898C7208993B&decimals=4
  5. Copy the URL
  6. Go to Mobile
  7. Select Sepolia
  8. Use the same address used for deploying the contract
  9. Go to the mobile browser (not the MM browser)
  10. Paste the URL
  11. Click Transfer with Deeplink
  12. Open MM
  13. See tx
  14. Don't Accept/Reject
  15. Go to the browser widnow
  16. Now Click Approve with Deeplink
  17. See a new tx modal appears overlapping the old one
  18. Accept this one
  19. See gas NAN ETH`

Error messages or log output

No response

Version

7.20 prod but it's an old issue

Build type

None

Device

Pixel 6

Operating system

Android

Additional context

No response

Severity

No response

bschorchit commented 5 months ago

We should handle this by automatically rejecting the previous/first transaction before opening the new one.

vinistevam commented 2 months ago

I tried to reproduce the reported issue but unfortunately, it didn't work. Here are some observations from my testing:

Deep Link Functionality: I was able to use deep links successfully, but only on a physical device. The functionality did not work as expected on an emulator or virtual device.

Send Flow Behavior: I created a send flow, which functioned as expected. However, nothing happened if tried to create an approval while the send flow was in progress.

https://github.com/user-attachments/assets/8f474d4e-4aea-485e-9c29-c9d4d070a51c

seaona commented 2 months ago

Some simpler steps:

  1. Open MM
  2. Start a tx -> reach to the last confirmation screen
  3. Without closing it, open a new window
  4. Click a deeplink with a new transaction
  5. See how new transaction appears
  6. Reject this one
  7. See how the first transaction is now empty --> I found that now the gas is also empty instead of NaN. It could be that some code changes affected in the behaviour (or that now I was trying on iOS and in the original video it was Android)

https://github.com/user-attachments/assets/bbf9af8d-802d-4b91-bb60-e31644c12a99