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

Switching hardfork in VM overwrites the opcodes codes variable #591

Closed
jochem-brouwer opened this issue Sep 8, 2019 · 4 comments · Fixed by #592
Closed

Switching hardfork in VM overwrites the opcodes codes variable #591

jochem-brouwer opened this issue Sep 8, 2019 · 4 comments · Fixed by #592

Comments

@jochem-brouwer
Copy link
Member

If you create a VM using istanbul and then create a VM using petersburg you will get in the petersburg VM a VM which has (this is what I tested) set the SLOAD gas cost 800 while it should be 200 (the original value). I suspect this also means that you can access SELFBALANCE and CHAINID in the petersburg VM and that the EXTCODEHASH and BALANCE opcodes are gas priced the wrong way.

The (clean) solution would be to give every VM their own codes list instead of pointing it to the "global" variable codes in lib/evm/opcodes.ts.

I quickly fixed this myself by adding this to lib/evm/opcodes.ts: (which is obviously a dirty solution)

const normalOpcodes: any = {
  0x31: ['BALANCE', 400, true],
  0x3f: ['EXTCODEHASH', 400, true],
  0x54: ['SLOAD', 200, true],
}

export function setOpcodes(hf: string) {
  if (hf === 'istanbul') {
    codes = { ...codes, ...istanbulOpcodes }
  } else {
    codes = { ...codes, ...normalOpcodes }
    codes[0x47] = undefined;
    codes[0x46] = undefined;
  }
}
@s1na
Copy link
Contributor

s1na commented Sep 9, 2019

Hey, thanks for opening the issue.

This was introduced in #582 as a temporary solution for #581. I had in mind to move the list of opcodes and their base gas prices to the VM instance before we release the new version.

@jochem-brouwer
Copy link
Member Author

Ah right. I should have checked the other issues first. For every VM it is indeed much cleaner if the opcode list is part of the VM =)

I will close this issue.

@jochem-brouwer
Copy link
Member Author

Actually it looks like the referenced PR does not create a new opcode list for every VM, so I will keep the issue open.

@s1na
Copy link
Contributor

s1na commented Sep 9, 2019

Sorry for the confusion. I didn't mean that PR creates a new opcode list for every VM. I was just pointing to the PR that introduced this setOpcodes function for reference.

Now created #592 to address this issue.

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.

3 participants