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

EIP-2315 change gascosts and update opcode #995

Merged
merged 3 commits into from
Jun 2, 2020

Conversation

matkt
Copy link
Contributor

@matkt matkt commented May 28, 2020

Signed-off-by: Karim TAAM [email protected]

PR description

Signed-off-by: Karim TAAM <[email protected]>
@matkt matkt changed the title EIP-2315 change gascosts EIP-2315 change gascosts and update opcode May 29, 2020
@matkt matkt marked this pull request as ready for review June 2, 2020 14:37
@@ -36,7 +36,7 @@ public AbstractPrecompiledContractTest(
registryFactory
.apply(
new PrecompiledContractConfiguration(
new BerlinGasCalculator(), PrivacyParameters.DEFAULT))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, would love other opinions, but do we want to still have a BerlinGasCalculator that just delegates to IstanbulGasCalculator? I think it makes it more obvious which parts of the code have been updated if we use this convention.

Copy link
Contributor

Choose a reason for hiding this comment

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

That thought crossed my mind. However I think making a no-op gas calculator might imply to the casual reader that there were gas cost changes in Berlin when in fact there are not currently.

If the two gas cost EIPs do make an appearance we can return it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also had a doubt regarding this part. but I think that if we don't change anything it's not useful to have a GasCalculator per fork but only when we change something IMO

@matkt matkt merged commit 513d2d9 into hyperledger:master Jun 2, 2020
macfarla pushed a commit to macfarla/besu that referenced this pull request Jun 3, 2020
Change gascosts

  - JUMPSUB -> 10 (high tier gas cost)
  - RETURNSUB -> 5 (low tier gas cost)
  - BEGINSUB -> 2 (base tier gas cost)

Update opcode 

  - 0x5c BEGINSUB
  - 0x5d RETURNSUB
  - 0x5e JUMPSUB

Signed-off-by: Karim TAAM <[email protected]>
Signed-off-by: Sally MacFarlane <[email protected]>
@matkt matkt deleted the feature/eip2315-change-gas-cost branch June 11, 2020 10:01
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