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

Remove usage of gasleft in execute flows #459

Closed
1 task done
rmeissner opened this issue Dec 16, 2022 · 4 comments
Closed
1 task done

Remove usage of gasleft in execute flows #459

rmeissner opened this issue Dec 16, 2022 · 4 comments
Assignees
Milestone

Comments

@rmeissner
Copy link
Member

rmeissner commented Dec 16, 2022

Some networks discourage the use of gasleft therefore it should be replace with type(uint256).max

ToDo:

  • check if this has any unwanted side effects
@rmeissner rmeissner added this to the Version 1.4.0 milestone Dec 16, 2022
@mmv08
Copy link
Member

mmv08 commented Dec 16, 2022

Some networks discourage the use of gasleft

Could you please give some more context on this? I thought this was driven by the EIP4337 spec

@rmeissner
Copy link
Member Author

It is, but some other networks also considered this a bad usage. E.g. arbitrum mentioned in their docs that their gas metering behaves different and therefore you should not depend on this.

@mmv08
Copy link
Member

mmv08 commented Dec 24, 2022

Could you please elaborate on what execute flows are in the scope of this ticket?

Is execTransaction a part of it? I'm unsure if it's doable because the gas refund logic is heavily dependent on gasleft.

Also, regarding Arbitrum, if I understand it correctly, the problem is that it will not represent the total fee amount for the transaction because of its two-dimensional gas structure (you also need to pay for L1 call data), which may be critical for refund logic, but it doesn't seem to be related with gasleft usage alone. Got it from here

@rmeissner
Copy link
Member Author

Is execTransaction a part of it? I'm unsure if it's doable because the gas refund logic is heavily dependent on gasleft.

Hmmm yeah good question. As this seems to be unclear what networks/tool really have dependencies to it (as I also don't have references anymore and might remember incorrectly) I would only focus on the flows where we know it would be helpful to change it (namely executeTxFromModule* for EIP-4337). The proposed solution would be the same.

This is for v1.4.0. For v2 I would remove all usage of gasleft to avoid potential issues and differences in this with networks (but that can be done as part of #197)

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

2 participants