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

Move hardfork-changed gas costs from VM to Common #803

Closed
jochem-brouwer opened this issue Jun 30, 2020 · 1 comment · Fixed by #806
Closed

Move hardfork-changed gas costs from VM to Common #803

jochem-brouwer opened this issue Jun 30, 2020 · 1 comment · Fixed by #806

Comments

@jochem-brouwer
Copy link
Member

In some hardforks, the only thing which is changed is the gas cost of some opcodes. See for instance this part of opcodes.ts where default opcodes are overwritten, but the only change is the gas fee.

These should be read from common. I'd say we keep the fee attribute in opcodes.ts, but we set those values in getOpcodesForHF. I'm not entirely sure if we should move all "default" opcodes to Common (i.e. we put them in chainstart), or that we should keep the fee of opcodes which have not changed gas at any fork in opcodes.ts. If it has changed in the future, we remove this from opcodes.ts and for getOpcodesForHF to read the value from Common. If we do this in getOpcodesForHF this means we only call Common once, instead of doing it multiple times every time we invoke a gas cost which is constant if the fork of the VM does not change.

I also think we should do similiar things for things like the expByte gas cost, we should set these values in getOpcodesForHF to get a slight performance boost, since we don't have to keep retrieving these constants from Common.

@jochem-brouwer
Copy link
Member Author

jochem-brouwer commented Jul 3, 2020

I propose we actually move the default gas costs from chainstart to Common and set the "base fee" (and related attributes such as constants like expByte) in getOpcodesForHF. I think it is natural to move these constants to Common; if a gas cost changes in the future we can simply set this constant in Common without having to tinker with the VM.

Thoughts? @holgerd77 @s1na @alcuadrado

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant