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

erc20 transfer out of gas errors when sending max tokens #8136

Closed
wjmelements opened this issue Feb 28, 2020 · 17 comments
Closed

erc20 transfer out of gas errors when sending max tokens #8136

wjmelements opened this issue Feb 28, 2020 · 17 comments
Labels
area-gas area-tokens type-bug Something isn't working

Comments

@wjmelements
Copy link
Contributor

Version 7.7.2

I've twice seen erc20 out of gas errors when transferring tokens on mainnet. The two tokens were USDC and WETH. What the failures had in common was that the gas estimation seemed to not take into account that the recipient had no prior balance, so the SSTORE cost 20000 instead of 5000. This can also happen if the recipient empties their balance before the transfer confirms but that wasn't true this case.

In both cases I was sending the entire token balance of my account.

I did not see a similar issue when I was transferring ETH to the WETH contract, which created WETH for an account that had none.

The workaround is to manually set the gas limit in Advanced.

@wjmelements
Copy link
Contributor Author

I have now also reproduced this for GST2.

@wjmelements
Copy link
Contributor Author

This issue seems to only affect transfers.

@danfinlay
Copy link
Contributor

Sine we call eth_estimateGas from geth, and then add a 50% buffer, this seems like it is likely a bug in geth.

The most ideal reproduction would be to open the background network inspector, and copy as cURL the request to infura that is returning the insufficient gas estimate.

With that request, Infura and geth team members will be able to trace why the estimate is low.

@jacobc-eth
Copy link

@wjmelements Would you be willing to hop on a quick call to help us replicate this issue? We're having trouble recreating it. Do you have Telegram or some other medium I can use to contact you directly?

@wjmelements
Copy link
Contributor Author

I can still 100% repro on 7.7.7.

Sine we call eth_estimateGas from geth, and then add a 50% buffer, this seems like it is likely a bug in geth.

I don't think metamask does that. Probably your estimate gas params are wrong.

Do you have Telegram or some other medium I can use to contact you directly?

I have the same username on most platforms

@tmashuang
Copy link
Contributor

tmashuang commented Mar 10, 2020

In both cases I was sending the entire token balance of my account.

Is this by using the Max Button?

@wjmelements
Copy link
Contributor Author

I have found your bug, you are estimating gas with value=0 in the transfer

@wjmelements
Copy link
Contributor Author

Screen Shot 2020-03-10 at 3 48 15 PM

@tmashuang
Copy link
Contributor

Value is determined in the data hex for token transfers.

@wjmelements
Copy link
Contributor Author

Value is determined in the data hex for token transfers.

As seen in the data, it is 0

@wjmelements
Copy link
Contributor Author

There are no subsequent calls to eth_estimateGas after the estimate with transfer of 0, so that is the issue. You have to re-estimate with the correct transfer value. A transfer of 0 will not change the 0-ness in the SSTORE when the recipient has no tokens.

@tmashuang
Copy link
Contributor

Hmm I am fairly certain we update data on input change, otherwise we would see numerous other issues, but ill investigate that a little more. Can you clarify on what what method those params are for, I am guessing its for eth_estimateGas?

@wjmelements
Copy link
Contributor Author

Can you clarify on what what method those params are for, I am guessing its for eth_estimateGas?

right

@wjmelements
Copy link
Contributor Author

Here are repro steps, including a bug where you suggest a gas price that is 8x too high despite the "Slow" setting.

Screen Shot 2020-03-10 at 3 57 05 PM

Screen Shot 2020-03-10 at 3 57 16 PM

Screen Shot 2020-03-10 at 3 57 28 PM

Screen Shot 2020-03-10 at 3 57 37 PM

Screen Shot 2020-03-10 at 3 57 45 PM

Screen Shot 2020-03-10 at 3 58 08 PM

@wjmelements
Copy link
Contributor Author

Also, note how the "Send Amount" shows 100% of my ETH instead of 100% of my MKR in the penultimate screenshot. I don't think that manifests in the TX but it's still fairly scary to think you might be sending all my ETH to the MKR token contract.

@wjmelements
Copy link
Contributor Author

Hmm I am fairly certain we update data on input change

I have verified that you do when I type in the value, but you do not when the Max button is pressed instead.

@wjmelements wjmelements changed the title erc20 transfer out of gas errors erc20 transfer out of gas errors when sending max tokens Mar 10, 2020
@Gudahtt Gudahtt added type-bug Something isn't working and removed N08-needsInvestigation labels Mar 11, 2020
@tmashuang
Copy link
Contributor

Fixed by #8176. Will get it out in the next release thanks for the report and investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-gas area-tokens type-bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants