Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Gas is not re-calculated when updating a transaction #5865

Closed
seaona opened this issue Mar 1, 2023 · 1 comment
Closed

[Bug] Gas is not re-calculated when updating a transaction #5865

seaona opened this issue Mar 1, 2023 · 1 comment
Assignees
Labels
area-gas Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead type-bug Something isn't working

Comments

@seaona
Copy link
Contributor

seaona commented Mar 1, 2023

Describe the bug
There is a problem with gas calculation when we switch from one type of Send to another (Send ETH/ Send ERC20 / Send NFT). It looks like gas is not re-calculated resulting in non-desired behaviors for both cases.

See below the effects:

  • If I switch from an ETH Send back to a Token Send (ERC20 or ERC721) I can see that the proposed gas is the same and if I try to perform the tx I get the error intrinsic gas too low and even if I set High gas fees, I still see the error and cannot proceed. The problem is that we are using the gas proposed for the first Send, and it's not re-calculated. We expected higher costs when we interact with contracts and send hex data along our call.
too-low-intrinsic-gas.mp4
  • If I switch from a Token Send (ERC20 or ERC721) back to an ETH Send I see that the suggested gas is unnecessary high. It seems that we are carrying over the gas calculation for the Token Send - which is usually higher due to hex data. We should re-calculate the proposed gas for a Send, so user does not spend unnecessary fees.
proposed-gas-too-high.mp4

Affected flows:

  • Change Send ETH to Send ERC20
  • Change Send ERC20 to Send ETH
  • Change Send ETH to Send ERC721
  • Change Send ERC721 to Send ETH

To Reproduce
Steps to reproduce the behavior

  1. Click Send ETH
  2. Add recipient and amount and proceed to the next screen
  3. See proposed gas value
  4. Click Edit
  5. Change token for an ERC20 Token and proceed
  6. See proposed gas value is the same
  7. Try to Send the tx -- get error intrinsic gas too low

Expected behavior
We should re-calculate the proposed gas everytime we update a transaction

Smartphone (please complete the following information):

  • Device: Pixel 6
  • OS: Android 12
  • App Version: v6.0.1 (1072)

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

  • How critical is the impact of this bug on a user?
  • Add stats if available on % of customers impacted
  • Is this visible to all users?
  • Is this tech debt?
@seaona seaona added type-bug Something isn't working team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead labels Mar 1, 2023
@seaona seaona changed the title [Bug] Gas is not re-calculated when updating a transacion. I.e. Change a Send ETH to Send ERC20/ERC721 and the other way around [Bug] Gas is not re-calculated when updating a transacion Mar 1, 2023
@seaona seaona changed the title [Bug] Gas is not re-calculated when updating a transacion [Bug] Gas is not re-calculated when updating a transaction Mar 1, 2023
@seaona
Copy link
Contributor Author

seaona commented Mar 3, 2023

This relates to #4853

@bschorchit bschorchit added Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking area-gas labels Mar 7, 2023
@blackdevelopa blackdevelopa self-assigned this Mar 9, 2023
@seaona seaona closed this as completed Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-gas Sev2-normal An issue that may lead to users misunderstanding some limited risks they are taking team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead type-bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants