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

Implement dispatching to forked ops #123

Closed
zah opened this issue Aug 28, 2018 · 3 comments
Closed

Implement dispatching to forked ops #123

zah opened this issue Aug 28, 2018 · 3 comments

Comments

@zah
Copy link
Contributor

zah commented Aug 28, 2018

1) Modify the current opcode table to include the following information

a) opcode number
b) opcode name
c) forks where the opcode was changed

For example, it can look like this:

opcodeTable:
  0x11 sdiv
  0x12 create [Tangerine, Spurious]

2) Introduce a forkedOp macro

This will be similar to the current op macro, but creating a proc with an additional static enum parameter called fork.

forkedOp create:
  when fork >= Tangerine:
    ...

3) Compile the VM instruction dispatching loop for each fork

This will consult the opcodeTable and it will place calls to the forked procs using the correct static parameters. The information in the table is enough to ensure that only the minimal number of instantiations are created (i.e. only 3 versions of create are needed - "Initial", "After Tangerine" and "After Spurious")

4) Make sure the block validation logic uses the correct dispatching loop depending on the block number

@mratsim
Copy link
Contributor

mratsim commented Aug 29, 2018

I had the following idea in mind for new opcodes when I made the current architecture, similar to gas costs you have a BaseOpcodeTable.

See https://github.com/status-im/nimbus/blob/master/nimbus/vm/interpreter_dispatch.nim#L18-L27

# Note pseudocode

let FrontierOpDispatch {.compileTime.}: array[Op, NimNode] = block:
  fill_enum_table_holes(Op, newIdentNode"invalidInstruction"):
    [
      Stop: newIdentNode "toBeReplacedByBreak",
      Add: newIdentNode "add",
      Mul: newIdentNode "mul",
      Sub: newIdentNode "sub",
      Div: newIdentNode "divide",
      Sdiv: newIdentNode "sdiv",
      Mod: newIdentNode "modulo", 
      ...
      ExtCodeSize: newIdentNode "extCodeSize",
      ExtCodeCopy: newIdentNode "extCodeCopy",
      # ReturnDataSize: introduced in Byzantium
      # ReturnDataCopy: introduced in Byzantium
      ...
      # f0s: System operations
      Create: newIdentNode "create",
      Call: newIdentNode "call",
      CallCode: newIdentNode "callCode",
      Return: newIdentNode "returnOp",
      DelegateCall: newIdentNode "delegateCall",
      # StaticCall: introduced in Byzantium
      # Revert: introduced in Byzantium
      # Invalid: newIdentNode "invalid",
      SelfDestruct: newIdentNode "selfDestruct"
    ]

func byzantiumOpDispatch(previous_opcodes: array[Op, NimNode]): array[Op, NimNode] =
  result = previous_opcodes

  # Now assuming new opcodes are introduced:
  result[ReturnDataSize] = newidentNode "returnDataSize"
  result[ReturnDataCopy] = newidentNode "returnDataCopy"
  result[StaticCall] =  newidentNode "staticCall"
  result[Revert] =  newidentNode "revert"
  result[Invalid] =  newidentNode "invalid"

let ByzantiumOpDispatch {.compileTime.} = FrontierOpDispatch.byzantiumOpDispatch

This was to make sure that invalid opcodes at Genesis time (but valid after Tangerine/Byzantium) are correctly reported invalid.

For opcodes that where patched over hardforks the when fork >= Tangerine is clearer.

@zah
Copy link
Contributor Author

zah commented Aug 29, 2018

I think we only need a way to specify that a certain opcode was introduced in certain fork. Then the single opcode table will still be enough to figure out that this particular opcode is not valid in the Genesis block.

The only strong assumption of the single opcode table is that opcodes won't change their numeric IDs across forks, which is a reasonable assumption.

@jangko
Copy link
Contributor

jangko commented Dec 22, 2022

I believe we can close this issue, our current evm have solve this, although not using macro.

@jangko jangko closed this as completed Dec 22, 2022
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

No branches or pull requests

3 participants