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

Gas limit reached on wETH pool Add Liquidity transaction #104

Open
dxiy opened this issue Dec 19, 2021 · 10 comments
Open

Gas limit reached on wETH pool Add Liquidity transaction #104

dxiy opened this issue Dec 19, 2021 · 10 comments

Comments

@dxiy
Copy link

dxiy commented Dec 19, 2021

Hi,

An out of gas error on an Add Liquidity transaction to the wETH pool has occurred. The user insists that they did not modify the gas limit when generating this transaction. The transaction ID is: 0xb04d4bcc535736b10c5ccec10ccdf5ccac1b4e5cdd6a7e4d1607ec09ce22b626

Oddly, the gas limit for this transaction was about 5% lower than most other recent Add Liquidity transactions (94,296 in this instance vs ~99,884 for others). If it is correct that the user did not modify the gas limit (tbc) then it might indicate that there was an issue with the gas estimation for this particular transaction.

Comparably recent transaction IDs:
0x3f4c217e41c99b36accfea23f6e8b9c38e0b96cd2ea51a1a71388e34451c9118
0xcb39165ac5395129ee442baa034fb7688e6f106a8383bd0178bf28248bec1add
0x1ed860e6833025f4fc011d195f5c175e0c6f7ae43f56f29446052f30e71ba7f8
0x724370e25d1501ec10baf5e0ef28f217d97864cb89f973fc7347d3ce2219b805

@dxiy
Copy link
Author

dxiy commented Dec 19, 2021

Have just seen another instance of this issue, with the exact same incorrect gas calculation (94,296 => 100% utilisation, out of gas)

Transaction ID: 0xd76c3719b1dd3f2fac380be2f41daf759714ee9cb462c19f9015588b22fc1200

@mrice32
Copy link
Contributor

mrice32 commented Dec 28, 2021

@dxiy Sorry for the delayed response here. The TLDR is that the wallet (metamask) controls gas estimation. The across UI does not. The transaction is passed to metamask and it determines where to set the gas limit to set.

I looked at the transactions individually and I noticed a common thread between them: in each case, another transaction hit the contract moments before the failed one. This could offer some explanation of why things are going wrong in metamask.

The amount of gas a transaction uses is not always the same. It depends on a lot of things. Put simply, if the state of the contract isn't exactly the same when the gas is estimated vs when the transaction gets mined, the gas estimation will be wrong. Sometimes the difference is harmless. For instance, the new cost may be cheaper than anticipated. However, in some cases, the gas cost could be considerably more expensive, leading to an OOG error.

In my opinion, metamask should more generously overestimate. I often manually modify the gas limit in metamask to double or triple its estimated value just to be sure that the transaction goes through. You aren't charged for unused gas, so there's no real downside as long as you have enough ETH so that you could pay for that extra gas.

Does that make sense?

@dxiy
Copy link
Author

dxiy commented Dec 28, 2021

Hi Matt, thanks for following up on this topic.

The TLDR is that the wallet (metamask) controls gas estimation. The across UI does not. The transaction is passed to metamask and it determines where to set the gas limit to set.

Thanks for the clarification. I'd had the misunderstanding that the web UI could supply the gas estimation to MetaMask, or that it could somehow influence that calculation at least. I appreciate you clearing that up for me; I'll relay the details to the affected users.

@dxiy dxiy closed this as completed Dec 28, 2021
@mrice32
Copy link
Contributor

mrice32 commented Dec 29, 2021

We could try to influence the gas estimation. We may be able to seed metamask with our own gas limit to override its estimate, but I'm not 100% sure. @Gamaranto @daywiss @Tulun is this possible?

@mrice32 mrice32 reopened this Dec 29, 2021
@daywiss
Copy link
Contributor

daywiss commented Dec 29, 2021

We could try to influence the gas estimation. We may be able to seed metamask with our own gas limit to override its estimate, but I'm not 100% sure. @Gamaranto @daywiss @Tulun is this possible?

In the first iteration of the app, we tried to calculate the gas price and do an estimate when adding eth to pool. This ended up being less effective than just relying on metamask, so thats what we do now. There are certain cases though where the app does need to do an estimation, and thats when you are supplying the "max" amount of eth to the pool. In this case we cant send 100% of the users eth to the pool since some will need to be reserved for gas. This might be where some transactions are failing. If a user attempts to send max eth to pool, and the app underestimates the gas amount, it could cause an out of gas error. Metamask will warn you though that you dont have enough eth to complete the transactions, but maybe even metamask fails to do that some times, im not sure.

Long story short, we could add more room to our internal gas estimation, for cases when sending max eth, but its not totally clear that this will resolve these particular issues.

@daywiss
Copy link
Contributor

daywiss commented Dec 29, 2021

I went ahead and added a 50 gwei gas price buffer which will take effect next time we deploy the app

@dxiy
Copy link
Author

dxiy commented Dec 29, 2021

I went ahead and added a 50 gwei gas price buffer which will take effect next time we deploy the app

Hi David, thanks for the feedback.

Correct me if I am wrong, but as I understand it, increasing the estimation for maximum gas price won't necessarily save the transaction in the event that the gas limit is reached, since the gas price and gas limit are independent variables.

I'm not familiar enough with the web3 API, but my hope is that if the web app can influence the gas price suggested to MetaMask then it might also be possible to influence the gas limit. Intuitively this would make sense, since the webapp knows the intricacies and quirks of the contract much better than MetaMask ever will.

I've tried searching a bit for this but haven't had any success - MetaMask's own guidance on this is that the user themselves should be setting the gas limit themselves (but obviously that's not a reasonable solution; I don't understand why they propose it).

@daywiss
Copy link
Contributor

daywiss commented Jan 3, 2022

Correct me if I am wrong, but as I understand it, increasing the estimation for maximum gas price won't necessarily save the transaction in the event that the gas limit is reached, since the gas price and gas limit are independent variables.

Thats right, my understanding of the problem was wrong, matt has explained it to me. I'm sure there is a way to update the gas limit of a transaction sent to metamask, we use the ethers client to do this. Its tricky though to set this, as setting this really high will show the user a really high gas cost for the transaction, even though the unused gas will be returned. We will have to think more on how to calculate this as to not alarm the user with some crazy gas fee.

@dxiy
Copy link
Author

dxiy commented Jan 3, 2022

Its tricky though to set this, as setting this really high will show the user a really high gas cost for the transaction, even though the unused gas will be returned. We will have to think more on how to calculate this as to not alarm the user with some crazy gas fee.

Yeah I certainly appreciate the challenge in getting it right. Both users impacted have been notified of the situation and don't have any further feedback, and I haven't seen any additional tickets complaining of this. Perhaps it's not a bad idea to let it stew away in your subconscious for a while, until a solution just pops out :)

@daywiss
Copy link
Contributor

daywiss commented Jan 6, 2022

we currently have a rough idea on how to solve this and its on our agenda. will leave this issue open until we have that work in

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

No branches or pull requests

3 participants