[VM] group opcodes based upon hardfork

[jochem-brouwer]

Closes #543

Some notes: I also have grouped pre-byzantium opcodes (homestead, that's DELEGATECALL), but homestead is not supported by the VM yet.

I have included a very simple test which obviously does not test all cases.

Also per the issue: I think that any gas cost changes should either:

1. should be read every time from the opcode call from common (think about SLOAD, BALANCE, etc.)

2. if the call price is fixed (SLOAD for instance always charges the same amount of gas) then we should use the fee attribute of the opcode and change this fee if the gas price changes (ideally reading this once from common)

I tend to encourage us to use (2) since this does a single call to common for these constant-priced opcodes. (1) keeps calling common every time we use the opcode, though we know the hard fork of the VM is invariant and thus the gas cost is constant in these situations.

I think this gas stuff should be in a different issue?

[codecov]

Codecov Report

Merging #798 into master will decrease coverage by 0.16%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #798      +/-   ##
==========================================
- Coverage   84.49%   84.32%   -0.17%     
==========================================
  Files          19       20       +1     
  Lines        1245     1238       -7     
  Branches      246      247       +1     
==========================================
- Hits         1052     1044       -8     
  Misses        125      125              
- Partials       68       69       +1     

Flag	Coverage Δ
#account	92.85% <ø> (ø)
#block	80.07% <ø> (-0.08%)	⬇️
#blockchain	84.50% <ø> (-0.21%)	⬇️
#common	93.78% <ø> (ø)
#ethash	85.40% <ø> (-0.90%)	⬇️
#tx	94.16% <ø> (ø)

Impacted Files	Coverage Δ
packages/blockchain/src/cache.ts	66.66% <0.00%> (-14.59%)	⬇️
packages/ethash/src/util.ts	96.29% <0.00%> (-0.93%)	⬇️
packages/block/src/index.ts	100.00% <0.00%> (ø)
packages/block/src/header-from-rpc.ts	77.77% <0.00%> (+11.11%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 703fbd9...b1e19a3. Read the comment docs.

[holgerd77]

Thanks @jochem-brouwer, this looks good!

For later reference: have used this (actually very nice) Ethereum HF history article to check on the correct opcode association (so: Byzantium & Homestead opcode attribution correct, no additional opcodes introduced post-Frontier).

@jochem-brouwer yes, think it makes a lot of sense to move all gas cost params over to Common, also like the suggestion to keep the fee attribution in the opcodes.ts file to prevent repeated calls to the Common library.

Can you open an issue on that? Or - if no one intervenes here and you want to take on this - you might also just directly open a PR, think this is likely (?) relatively undisputed as a change suggestion.

[jochem-brouwer]

Cool, this article was actually something I was looking for. Thanks @holgerd77

I will open an issue otherwise we/I will forget doing this RE: gas usages to Common.

[s1na]

Late to the party but this is a good change! We can go one step further and remove the HF check from the opcode handlers in opFns.

[jochem-brouwer]

I like this, could you open a PR/issue @s1na?