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

Update VM initialisation gas cost benchmark #1807

Closed
xgreenx opened this issue Apr 5, 2024 · 0 comments · Fixed by #1870
Closed

Update VM initialisation gas cost benchmark #1807

xgreenx opened this issue Apr 5, 2024 · 0 comments · Fixed by #1870
Assignees

Comments

@xgreenx
Copy link
Collaborator

xgreenx commented Apr 5, 2024

After FuelLabs/fuel-vm#697, the allocation of the stack or heap may cause the copying of many bytes. The VM initialization should charge the user for the worst possible scenario where we will resize the heap/stack 26 times to be sure that new behaviour is not exploited by the attacker.

This strategy overcharges the user but improves the network's safety. Later, we can refund gas if the user doesn't initiate re-allocations(it will be a separate feature).

@xgreenx xgreenx assigned xgreenx and unassigned Dentosal Apr 29, 2024
xgreenx added a commit that referenced this issue Apr 30, 2024
Closes #1807

The PR updates benchmarks to work properly with new VM rules and fixes
the issue with non-optimal `InMemoryTransaction` behavior for the
`size_of_value` method.

Because of the new VM rules: the memory that we are accessing should be
initialized. The `cfe` opcode in the benchmarks expands the stack to
allocate the memory that will later be used by the instruction that we
are benchmarking.

### Before requesting review
- [x] I have reviewed the code myself

---------

Co-authored-by: Hannes Karppila <[email protected]>
Co-authored-by: Brandon Vrooman <[email protected]>
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.

2 participants