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

Uniswap 900K gas estimation fixing solution #243

Closed
k06a opened this issue Sep 19, 2019 · 6 comments
Closed

Uniswap 900K gas estimation fixing solution #243

k06a opened this issue Sep 19, 2019 · 6 comments
Assignees
Labels
Milestone

Comments

@k06a
Copy link
Contributor

k06a commented Sep 19, 2019

Related issues: vyperlang/vyper#1091, Uniswap/v1-contracts#39

We suggest to move _emit call before tokenFallback to resolve Uniswap 900K gas estimation. Solidity passes 63/64 of gas in subcalls and now estimator increases gas until 1/64 of gas left is enough for _emit call.

Currently gas estimator is feeding Uniswap proxy until 1/64 of gasleft would be enough for _emit call.

Moving _emit before tokenFallback would decrease gas estimation to 100K instead of 900K

image

@k06a k06a changed the title Uniswap 900K gas estimation fixing solution [BUG] Uniswap 900K gas estimation fixing solution Sep 19, 2019
@k06a k06a changed the title [BUG] Uniswap 900K gas estimation fixing solution Uniswap 900K gas estimation fixing solution Sep 19, 2019
@k06a
Copy link
Contributor Author

k06a commented Sep 19, 2019

Also limiting tokenFallback gas is a good idea. For example provide Math.min(gasleft(), 200000) of gas.

@hav-noms hav-noms self-assigned this Oct 16, 2019
@hav-noms hav-noms added the bug label Oct 16, 2019
@hav-noms hav-noms added this to the Rigil milestone Oct 17, 2019
@k06a
Copy link
Contributor Author

k06a commented Oct 21, 2019

Same valid for all synth tokens. See for example sETH+ETH Uniswap pool transactions: https://etherscan.io/address/0xe9cf7887b93150d4f2da7dfc6d502b216438f244

@k06a
Copy link
Contributor Author

k06a commented Oct 26, 2019

It seems having some synths minted from user wallet, increases gas limit to 10M: https://www.reddit.com/r/synthetix_io/comments/djd6ca/uniswap_synthetix_gas_required_exceeds_allowance/

@k06a
Copy link
Contributor Author

k06a commented Nov 6, 2019

It is not possible to swap SNX to DAI or any other token because of this bug.

@marc0olo
Copy link

@hav-noms why is this closed? I am not able to swap sETH to ETH on uniswap due to this bug

@ihlec
Copy link

ihlec commented Sep 7, 2020

@hav-noms the problem seams to still exists.

For certain transactions it would be useful to send the transaction with a default user defined gas limit after the gas estimation failed.

Currently the users are facing unresponsive dApps.

e.g. Uniswap:
inpage.js:1 MetaMask - RPC Error: gas required exceeds allowance (12426210) or always failing transaction Object (anonymous) @ inpage.js:1 index.tsx:283 estimateGas failed removeLiquidityETHWithPermit Array(10) Object (anonymous) @ index.tsx:283 inpage.js:1 MetaMask - RPC Error: gas required exceeds allowance (12426210) or always failing transaction Object (anonymous) @ inpage.js:1 index.tsx:283 estimateGas failed removeLiquidityETHWithPermitSupportingFeeOnTransferTokens Array(10) Object (anonymous) @ index.tsx:283 index.tsx:295 This transaction would fail. Please contact support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants