fix: use pre-computed gas usage for deposit max amounts #873
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const gasPrice = await provider.getGasPrice(); | ||
| return gasPrice.mul(paddedGasEstimation); | ||
| return gasPrice.mul(gasExpenditureDeposit); |
There was a problem hiding this comment.
getGasPrice() can be unreliable because it's mostly ignorant of the gas pricing strategies of each chain. It's probably fine as an approximation, but should we apply a multiplier? I think something really coarse would be fine - i.e. 3x or so. Given that the deposit cost is so low it will then reserve only a small amount of the native token, and that should be fine by users.
There was a problem hiding this comment.
Yeah, it makes sense to me. Changed here f20b1af
| console.error(err); | ||
| return max(balance.sub(fallbackEstimatedGasCosts), 0); | ||
| }); | ||
| await estimateGasCostsForDeposit(selectedRoute).then( |
There was a problem hiding this comment.
ooc, how regularly does this run? I wonder if we want some smoothing logic so that the estimated gas cost doesn't bounce around and cause the deposit amount to flip-flop between being valid and invalid.
There was a problem hiding this comment.
The default is 5 minutes... But we can easily customize the refresh interval for this value
There was a problem hiding this comment.
Wow - I'd expected something like 2 - 10 seconds. 5 minutes is actually really long; we might want to revisit that separately, because it might lead to issues if gas becomes volatile.
There was a problem hiding this comment.
Ah wait, this actually updates more frequently I noticed. Because it is dependent on the useBalance query which queries the balance on every new block.
- comment on empty message - use multiplier
Yeah, it would lead to a bit less accuracy vs. a successful gas estimation. The thinking behind this PR is that there have been several reports of falling back to the default gas buffer of 0.01 ETH, mostly on deposits originating on zkSync. Secondarily, incurring the cost of an estimation on every single deposit is not really ideal - it's both faster and cheaper to use a fixed cost. As far as I understand, the user's wallet should anyway simulate this against their own RPCs before they sign it. |
This PR makes setting the max. bridgable amount for native token deposits more
accurate andreliable by: