OriginProtocol / ousd-governance

OUSD Governance Portal and Contracts
11 stars 4 forks source link

Switch the governance DApp to use gas estimate + small buffer #372

Closed franckc closed 1 year ago

franckc commented 1 year ago

As opposed to currently hardcoded gas limits.

@sparrowDom Do you have any pointers or recommendation for @HrikB on this issue?

@micahalcorn Do we have examples of any failed transactions due to hitting a gas limit?

micahalcorn commented 1 year ago

Here are some recent failures and successes:

https://docs.google.com/spreadsheets/d/1VzDYs5mC8911AG5MyGdyNlu0BqZmvgK0zdesRyF3nxY/edit

sparrowDom commented 1 year ago

I can't think of a better solution then a better hardcoded gas limits. There are many types of wallets with sometimes their own proprietary eth node providers and some of them can be super unreliable when it comes to transaction gas estimation. I have re-simulated the transactions on Tenderly with higher gas limit see spreadsheet

(thanks @micahalcorn for putting that spreadsheet together)

According to re-simulated transaction gas usage I would increase gas limits to:

shahthepro commented 1 year ago

Just wanted to flag that when PR #367 is merged and contracts deployed, we will have to revisit the gas limits (if we end up continuing with hardcoding)

sparrowDom commented 1 year ago

Yup very good point, thanks for flagging this @shahthepro

sparrowDom commented 1 year ago

So we have decided to leave stake and collect gas limits alone since the problem seems to have originated from users setting custom gas limits.

And have increased the extend gas limit to 260k. Hopefully that will be enough to include the changes to the staking contract that does auto delegation (not yet deployed but will soon be)