MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
12.08k stars 4.94k forks source link

[Bug]: Gas - Race condition where gas is not updated after switching assets and going to the last Confirmation screen #25243

Open seaona opened 5 months ago

seaona commented 5 months ago

Describe the bug

Whenever we are sending an asset (ERC20, ERC721..) and we are on the Send Asset page, if we change the asset for another one (ie ETH) and proceed to the last Confirmation screen, the gasLimit is not updated no matter how long we wait.

This can cause troubles to users which are under slow network conditions. Note: the hex data value is indeed updated immediately when we change assets, which is expected.

Expected behavior

If we don't want to block the Continue button before loading the new gas value, we should make sure that the gasLimit is set accordingly to the last transaction values (recipient, hex data and value) on the last confirmation screen. At the moment this doesn't happen, as it seems that the gasLimit is only updated in the Send Assets screen (where these values can be changed).

Screenshots/Recordings

https://github.com/MetaMask/metamask-extension/assets/54408225/cbef5440-078d-48e1-a3b6-57ab8503ed22

Steps to reproduce

You can run the assets test to reproduce it (once the test it's fixed, you can go to an older commit)

https://github.com/MetaMask/metamask-extension/issues/25181

Alternatively, you can simulate this by adding some delays in the code or slowing down your network

Error messages or log output

No response

Version

11.16.8 but seems earlier too

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

hjetpoluru commented 3 months ago

@MetaMask/confirmations, Could someone prioritize this bug? It is currently causing the automation test to fail. cc @bschorchit for more visibility.

jpuri commented 3 months ago

Confirmation pages use the Gas Limit value passed to it in the transaction object. In this case I found that the bug is with send pages actually which is not updating the gas limit as asset of the transaction is changed.

micaelae commented 3 months ago

The gas fees are fetched in the background each time the asset changes between native/nft/erc20 (also when changing recipient or amount). The pending gas fees are set to the most recently returned value. If the initial gas fetch takes longer than the next one, there's a chance the older gas fee replaces the current one on fetch

github-actions[bot] commented 1 day ago

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 45 days if there is no further activity. The MetaMask team intends on reviewing this issue before close, and removing the stale label if it is still a bug. We welcome new comments on this issue. We do not intend on closing issues if they report bugs that are still reproducible. Thank you for your contributions.