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

new(tests): EOF - EIP-5450: RJUMP* vs CALLF tests #833

Merged
merged 8 commits into from
Oct 22, 2024

Conversation

pdobacz
Copy link
Collaborator

@pdobacz pdobacz commented Sep 24, 2024

🗒️ Description

Adds a first batch of tests checking stack validation of these two kinds of operations RJUMP-ing and CALLF-ing (and RETF-ing ofc). I'm still missing JUMPF, but I want to go step by step as this has taken a while to prepare anyway.

First remark: I've decided to start off with an "automated" approach here, but as you'll see this has several complexities/inelegancies involved. There always is an alternative to just manually type out some interesting scenarios by hand, which we must bear in mind.

The problematic parts are:

  • it would be quite hard for all the automatically generated codes to come with a 100% accurate max_stack_height, so I introduce a flag no_expecations_on_validity to eof_test. This is probably the biggest controversy, and I'm also still unsold on this, let's discuss more in the diff. The nice part is that somewhat automatically we get to test invalid and valid codes en masse, without spending too much time on figuring out the exact cases. EDIT: Thanks to Mario's fix and suggestions this is no longer the case
  • it initially generated many many more tests, I've managed to trim this down aggressively to 400 768, however I took a - potentially unacceptable - shortcut and trimmed the "uninteresting" cases by pytest.skip. Without the pytest.skip it goes up to 800, all of the extra cases are ones which never successfully validate. All that being said, 400 is still a lot of cases, and I'd like to add JUMPF and recursive calls to this in the future iteration
  • it is generally complex and has an air of "throw mud at wall, see if it sticks" and maybe gets into the court of what fuzzing should accomplish. Again, the "good part" of this is that we're not effectively re-writting the stack validation routines in EEST. EDIT: Again, Mario's fix and suggestion make it much less of a problem

🔗 Related Issues

NA

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

@pdobacz pdobacz requested a review from marioevz September 24, 2024 13:02
@pdobacz pdobacz self-assigned this Sep 25, 2024
@pdobacz
Copy link
Collaborator Author

pdobacz commented Sep 27, 2024

I pushed some updates after incorporating the addition from ipsilon#3. Unfortunately, I've noticed that we should expand the tests by a little, I went to far while trimming and cut out some cases without which some RJUMP* snippets never validated. Now we're at 768 cases mark 😬.

@pdobacz pdobacz force-pushed the rjumps-callf branch 2 times, most recently from 9a94896 to c0a97ce Compare October 7, 2024 15:29
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the changes, it's cleaner IMO.

This might need a rebase because now EOF tests are in the osaka folder.

@pdobacz pdobacz merged commit 7758f37 into ethereum:main Oct 22, 2024
3 checks passed
@pdobacz pdobacz deleted the rjumps-callf branch October 22, 2024 10:00
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.

3 participants