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

Add docstrings to all opcodes in the Opcodes Enum #383

Closed
marioevz opened this issue Jan 15, 2024 · 8 comments
Closed

Add docstrings to all opcodes in the Opcodes Enum #383

marioevz opened this issue Jan 15, 2024 · 8 comments
Labels
good first issue Good for newcomers scope:fw Scope: Framework (evm|tools|forks|pytest) type:chore Type: Chore

Comments

@marioevz
Copy link
Member

It would be nice to have some docstrings with descriptions of each Opcode in the Opcodes Enum.

The docstring could have some nicely formatted description that includes at least the following information:

  • Small description of the opcode behavior
  • Parameters description
  • Output description
  • Fork when it was introduced
  • Gas usage description

The docstrings are normally picked up by the IDE, such as vscode, and it's a nice and handy way to have the parameters show up when going through the code as a reminder of the parameters and whatnot.

@marioevz marioevz added good first issue Good for newcomers type:chore Type: Chore scope:fw Scope: Framework (evm|tools|forks|pytest) labels Jan 15, 2024
@ThreeHrSleep
Copy link
Contributor

hey ! can I work on this issue?

@marioevz
Copy link
Member Author

hey ! can I work on this issue?

Of course!

@ThreeHrSleep
Copy link
Contributor

ThreeHrSleep commented Jan 30, 2024

please lmk if this format looks okay-ish or something needs to be changed(so I can replicate this with every opcode)

    STOP = Opcode(0x00)
    """
    Behavior: Halts execution
    Input: 
    Output: 
    Was introduced in: Frontier
    Gas: 0
    """

    ADD = Opcode(0x01, popped_stack_items=2, pushed_stack_items=1)
    """
    Behavior: Addition operation
    Input: a: first integer value to add
           b: second integer value to add
    Output: a + b: integer result of the addition modulo 2**256
    Was introduced in: Frontier
    Gas: 3
    """

@marioevz
Copy link
Member Author

marioevz commented Jan 30, 2024

I took a look at how vscode renders the docstrings, and made some changes:

    ADD = Opcode(0x01, popped_stack_items=2, pushed_stack_items=1)
    """
    ADD(a, b) = c
    ----

    Description
    ----
    Addition operation

    Inputs
    ----
    - a: first integer value to add
    - b: second integer value to add

    Outputs
    ----
    - c: integer result of the addition modulo 2**256

    Fork
    ----
    Frontier

    Gas
    ----
    3
    """

I think for other opcodes that are not arithmetic, e.g. CREATE, we can name the inputs and outputs gas, to, etc, instead of a, b, c ...

edit: Added hyphens to the inputs and outputs for it to better render in the documentation

@ThreeHrSleep
Copy link
Contributor

Thanks a lot for the feedback! making a PR as soon as I'm done :)

@danceratopz
Copy link
Member

danceratopz commented Jan 31, 2024

Hey @ThreeHrSleep! Nice to have you here.

If you're keen, I wondered if a script could do a one-time pull of the doc from https://www.evm.codes?

Here's the doc for ADD in their repo.

After a quick check, it seems that if an opcode's functionality was modified in a subsequent fork, the additional information is documented in a a sub-folder, e.g., checkout https://github.com/smlxl/evm.codes/tree/main/docs/opcodes/55 (a complicated example). The additional info relevant for the latest fork could be added manually though.

We could also add a link at the bottom of the docstring directly to the opcode on evm.codes, e.g. (as above in #383 (comment)), but with:

Source: [evm.codes/#01](https://www.evm.codes/#01).

@ThreeHrSleep
Copy link
Contributor

Thank you so much for the suggestion @danceratopz !
On it
(very sorry for taking so long 😅 )

@ThreeHrSleep
Copy link
Contributor

update: almost done,will make a PR by tonight

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers scope:fw Scope: Framework (evm|tools|forks|pytest) type:chore Type: Chore
Projects
None yet
Development

No branches or pull requests

3 participants