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

feat(cli): evm_bytes rename + print asm #844

Merged
merged 10 commits into from
Oct 4, 2024
Merged

Conversation

marioevz
Copy link
Member

@marioevz marioevz commented Sep 26, 2024

🗒️ Description

Changes included:

  • Module Renaming and Refactoring:

    • Renamed the evm_bytes_to_python module to evm_bytes and refactored the code to improve readability and functionality. (pyproject.toml, src/cli/evm_bytes.py, src/cli/evm_bytes_to_python.py) [1] [2] [3]
  • Test Updates:

    • Renamed and updated tests to reflect the module name change from evm_bytes_to_python to evm_bytes. (src/cli/tests/test_evm_bytes.py) [1] [2]
  • --assembly flag:

    • Added --assembly flag which allows printing the bytecode in assembly format (e.g. see format used in EIP-7002 or EIP-7251).

🔗 Related Issues

None

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.
  • Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
  • Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
  • Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
  • Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

@marioevz marioevz added type:chore Type: Chore type:refactor Type: Refactor scope:tooling Scope: Python tools (uv, ruff, tox,...) labels Sep 26, 2024
@marioevz marioevz marked this pull request as ready for review September 26, 2024 19:02
@marioevz marioevz force-pushed the update-evm-bytes-to-python branch from 73cf968 to 3a5d898 Compare October 1, 2024 17:22
@marioevz
Copy link
Member Author

marioevz commented Oct 1, 2024

@danceratopz this is the output of the assembly version:

uv run evm_bytes binary-file ./src/ethereum_test_forks/forks/contracts/withdrawal_request.bin --assembly
caller
push20 0xfffffffffffffffffffffffffffffffffffffffe
eq
push1 0x90
jumpi
...

Don't really know where we could fit this example though.

@danceratopz
Copy link
Member

Don't really know where we could fit this example though.

I asked our old collaborator gpt to put the example in the docstrings with google docstring style - then we get some nice online docs. The parameter table isn't quite right (from a cli user perspective, it is from an api perspective), but perhaps we can incrementally improve this over time. I do like the examples though.

image

@danceratopz
Copy link
Member

@marioevz I broke tox -e docs in multiple ways, will get back to this!

@marioevz
Copy link
Member Author

marioevz commented Oct 2, 2024

@marioevz I broke tox -e docs in multiple ways, will get back to this!

No worries, super simple fixes, loved the way the docstrings were formatted, thanks!

Copy link
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

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

Nice addition! LGTM!

@marioevz marioevz force-pushed the update-evm-bytes-to-python branch from cda244b to e48ce61 Compare October 4, 2024 17:12
@marioevz marioevz merged commit 888fac5 into main Oct 4, 2024
5 checks passed
@marioevz marioevz deleted the update-evm-bytes-to-python branch October 4, 2024 19:05
@raxhvl
Copy link
Contributor

raxhvl commented Oct 21, 2024

I'm trying to use evm_bytes to generate python source code from bytecode for gentest (#862). The square brackets in the CLI output is not valid python code in this context.

If the square brackets in the output is used only for visual representation, can we please replace them with parenthesis? I'd be happy to PR this change.

return f"Op.{self.opcode._name_}[{operands}]"

cc: @danceratopz @marioevz

@danceratopz
Copy link
Member

I'm trying to use evm_bytes to generate python source code from bytecode for gentest (#862). The square brackets in the CLI output is not valid python code in this context.

If the square brackets in the output is used only for visual representation, can we please replace them with parenthesis? I'd be happy to PR this change.

Maybe I don't understand the question? It should be valid 🙂 It was added to help testing EOF. For example:

Container(
name="DATALOADN_middle",
sections=[
Section.Code(
code=Op.DATALOADN[16] + Op.POP + Op.STOP,
),
Section.Data(data="1122334455667788" * 16),
],
),

This is achieved by implementing a __getitem__() method in Opcode, which is used to express the "data portion" of opcodes:

def __getitem__(self, *args: "int | bytes | str | Iterable[int]") -> "Opcode":
"""
Initialize a new instance of the opcode with the data portion set, and also clear
the data portion variables to avoid reusing them.

Ah, does evm_bytes generate opcodes with data portions for legacy (non-EOF) code? Can you provide an example?

@raxhvl
Copy link
Contributor

raxhvl commented Oct 29, 2024

For legacy opcodes, both approach seems to be equivalent.

In [1]: (Op.PUSH1[0x81] + Op.PUSH1[0x81]).hex()
Out[1]: '60816081'

In [2]: (Op.PUSH1(0x81) + Op.PUSH1(0x81)).hex()
Out[2]: '60816081'

When writing this manually, is there a convention to be followed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:tooling Scope: Python tools (uv, ruff, tox,...) type:chore Type: Chore type:refactor Type: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants