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

refactor: update addresses used in tests #553

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

enitrat
Copy link

@enitrat enitrat commented May 14, 2024

🗒️ Description

RIP precompiles will use addresses starting from 0x100. As such, and to make the ethereum execution spec tests re-usable by EVM rollups teams, I propose to bump the addresses 0x100 used in tests to something else - in this case, 0x1000. This allows rollup teams to benefit from the test coverage of ethereum, while innovating on new features.

🔗 Related Issues

Fixes #552

✅ 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: 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.

@enitrat enitrat force-pushed the refactor/update-addresses-used branch from c7914cf to 4533a26 Compare May 14, 2024 12:36
@spencer-tb
Copy link
Collaborator

The address range used by rollups is between 0x000 and 0xFFF? I would love for rollups to get use from the tests here.

I think we could optionally add something into to the framework, where addresses that dont fit within that range are bumped. Although that might be tricky.

We could add an error check if a test uses an address that fits within that range nonetheless.

@enitrat
Copy link
Author

enitrat commented May 16, 2024

Hi @spencer-tb, as per https://eips.ethereum.org/EIPS/eip-7587

The address range between 0x0000000000000000000000000000000000000100 and 0x00000000000000000000000000000000000001ff is reserved for use by the RIP process.

At Kakarot we are extensively using ethereum/tests for our zkevm, and we recently implement RIP7212, a secp256r1 signature verification available at address 0x100. This caused conflicts with some existing tests, so I bumped all the addresses in the range 0x100-0x1ff and re-filled the fixtures, so that we can keep our test coverage - and it worked very well!

I also refilled the legacy (non-python) tests, you can check here for more info:
https://github.com/kkrt-labs/tests

@marioevz
Copy link
Member

Hi, I think #584 is the beginning to make a solid foundation to prevent this from ever being an issue again.

All tests written with the new convention should start from 0x1000 but, adding to that, this start address is now even configurable so rollups could do:

fill -n auto --test-contract-start-address 0x1000000000000000000 --test-contract-address-increments 0x1000000000000000000

to specify from which address the test contracts should be defined.

The PR does not update all contract addresses, but we will start the refactor effort for the rest of the tests in the following weeks.

@enitrat
Copy link
Author

enitrat commented Jun 1, 2024

Hi !
Indeed this was just a quick fix, a more robust and customizable solution is definitely the right approach - especially now that rollups will start innovating and use different specific ranges of reserved addresses.

This is just a "quick fix", let me know if you want this to get merged or simply wait for the full refactor.

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.

test addresses collide with RIP Precompiles address ranges
3 participants