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

fix: ganache simulation fails with out of gas #7

Closed
mds1 opened this issue Jan 20, 2022 · 4 comments · Fixed by #14
Closed

fix: ganache simulation fails with out of gas #7

mds1 opened this issue Jan 20, 2022 · 4 comments · Fixed by #14

Comments

@mds1
Copy link
Owner

mds1 commented Jan 20, 2022

Discovered by @feuGeneA in #4, moving tracking of that issue here.

Ganache simulation fails with out of gas error, which @gnidan and @davidmurdoch are planning to look into. For reference, here's how much gas used is reported when simulating this transaction with various tools:

Gas usage at block 13,724,056:

@mds1
Copy link
Owner Author

mds1 commented Jan 20, 2022

description updated with gas usage

@brockelmore
Copy link

brockelmore commented Jan 20, 2022

1 point here that likely make up the difference between foundry and tenderly: we remove the base tx cost + calldata cost, i cant imagine tenderly does. This likely accounts for roughly 21064 of our difference wrt tenderly

@nebojsa94
Copy link

Hey @brockelmore, curious why you remove base + calldata cost, what do you achieve with that?

@brockelmore
Copy link

brockelmore commented Jan 21, 2022

we remove the base cost because it's just noise especially when trying to gas optimize, for example, what may normally be an internal function, but during gas testing they are testing it explicitly. We remove calldata for a similar reason, but slightly different. since we test with smart contracts, we always have an extra 64 gas that doesn't change regardless of what you do internally. so the long and short of it is, we want to report execution cost of tests as pure as possible and let people not have to do redundant math in their head

feuGeneA added a commit to NomicFoundation/convex-shutdown-simulation that referenced this issue Feb 1, 2022
Fixes mds1#7, not by fixing the bug in Ganache, but rather by working around
it, per the suggestion at https://twitter.com/atdavidmurdoch/status/1487125565425520645?t=3VKAXE2pKS2tBI3ZYJpw0Q&s=03
@mds1 mds1 closed this as completed in #14 Feb 3, 2022
mds1 pushed a commit that referenced this issue Feb 3, 2022
Fixes #7, not by fixing the bug in Ganache, but rather by working around
it, per the suggestion at https://twitter.com/atdavidmurdoch/status/1487125565425520645?t=3VKAXE2pKS2tBI3ZYJpw0Q&s=03
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

Successfully merging a pull request may close this issue.

3 participants