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

Allow for configuring costs per WebAssembly opcode #1

Closed
wants to merge 2 commits into from

Conversation

ureeves
Copy link
Member

@ureeves ureeves commented May 13, 2024

This is not meant to merged, as we maintain this crate slightly differently, but it is important enough functionality to require review.

If the commits in this PR are included, the user of dusk-wasmtime will be able to configure the fuel costs of each WebAssembly opcode, in much the same way as we could before. This paves the way for making storage cost more than other opcodes, as required by dusk-network/piecrust#359

@ureeves ureeves changed the title Alllow for configuring costs per WebAssembly opcode Allow for configuring costs per WebAssembly opcode May 13, 2024
@moCello
Copy link
Member

moCello commented May 14, 2024

If I understand correctly, with this change we could set the amount of "fuel units" for the call of a single op-code individually to any value we like?

@HDauven
Copy link
Member

HDauven commented May 14, 2024

If I understand correctly, with this change we could set the amount of "fuel units" for the call of a single op-code individually to any value we like?

That's right. If you look at the last line in tunables, it calls the for_each_operator! macro from wasmparser, and applies the define_operator_cost macro to it, which uses the default_cost macro.

It iterates over all the operators that wasmparser recognizes and assigns the default fuel cost of 0 for the defined ops and 1 for others. In the default_cost macro we can define custom gas cost for specific ops.

Copy link
Member

@moCello moCello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ureeves
Copy link
Member Author

ureeves commented May 15, 2024

As said, this won't be merged, but I'm happy you like the implementation

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 this pull request may close these issues.

3 participants