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 non jumpdests #36

Merged
merged 11 commits into from
Feb 26, 2024
Merged

Add non jumpdests #36

merged 11 commits into from
Feb 26, 2024

Conversation

4l0n50
Copy link
Contributor

@4l0n50 4l0n50 commented Feb 15, 2024

This PR adds an extra verification to invalid_jump_jumpi_destination_common that verifies that the jump destination is indeed not a jumpdest. This is done by non-deterministically guessing the closest previous opcode and verifying that after calling write_table_if_jumpdest, the corresponding bit in @SEGMENT_JUMPDEST_BITS is still 0.
Closes #27.

@4l0n50 4l0n50 added the soundness A soundness issue, i.e. malicious provers may create false proofs. label Feb 15, 2024
Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

I'm not sure that the test coverage is complete for this PR. Do you plan to add more tests in a different PR? Or…?

@4l0n50
Copy link
Contributor Author

4l0n50 commented Feb 15, 2024

I'm not sure that the test coverage is complete for this PR. Do you plan to add more tests in a different PR? Or…?

I actually forgot to add this test and I discovered some small bugs. Thanks!

@Nashtare Nashtare added the crate: evm_arithmetization Anything related to the evm_arithmetization crate. label Feb 15, 2024
Copy link
Collaborator

@Nashtare Nashtare left a comment

Choose a reason for hiding this comment

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

Not an actual review, but because this touches the jumpdest analysis logic, could you address the comment of the previous PR that got forgotten? 😅

log::debug!("jdt = {:?}", interpreter.jumpdest_table);

Move it to log::trace! and not abbreviate jdt for clarity. Thanks!

Copy link
Contributor

@hratoanina hratoanina left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

CHANGELOG.md Outdated Show resolved Hide resolved
@4l0n50 4l0n50 merged commit c9f9b98 into 0xPolygonZero:main Feb 26, 2024
5 checks passed
@Nashtare Nashtare deleted the non_jumpdests branch February 26, 2024 11:36
BGluth added a commit that referenced this pull request Jun 17, 2024
Fixed proof gen debug script not forcing `1` worker
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: evm_arithmetization Anything related to the evm_arithmetization crate. soundness A soundness issue, i.e. malicious provers may create false proofs.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Check non-validity of invalid jumpdests
4 participants