-
Notifications
You must be signed in to change notification settings - Fork 158
feat(benchmark): add jumpi-intense test case #1693
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
Conversation
CC either @jsign @jochem-brouwer @chfast |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with the comments from @chfast, also have a small style suggestion, plus a suggestion for a test where the JUMPI
performs the jump (instead of "falling through" and thus not performing the jump).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some extra information and some comments, hope it is helpful π π
@jochem-brouwer Thank you so much for the explanation, it really helped me understand the inner workings of the test better. Iβve updated accordingly, and this PR is now ready for review. It would be great if you could take another look. Thanks again! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the explanation, it really helped me understand the inner workings of the test better.
Love that it was so helpful π π Always happy to help π
I left two small comments for some tiny improvements π
@jochem-brouwer Thanks for the review, update accordingly! |
#1804 got merged! Please rebase the branch and move the tests to the appropriate folder. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just a couple of comments, a rebase to main due to conflicts, and then we can merge.
dbd8af9
to
23be0d8
Compare
ποΈ Description
Add a JUMPI-intensive operation for ZKVm as test case.
Currently
π Related Issues
Update issue #1690
β Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.