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

Do not panic on wire set twice or generator not run issues #1611

Merged
merged 2 commits into from
Aug 7, 2024
Merged

Conversation

Nashtare
Copy link
Collaborator

@Nashtare Nashtare commented Aug 6, 2024

closes #1490

Also allows to address 0xPolygonZero/zk_evm#292.

Given that we already have some breaking changes pending, and this PR would just trigger warnings of unused returned Result on projects depending on plonky2, this should be a fairly sane change.

Note: I'd recommend reviewing with hidden whitespaces.

@Nashtare Nashtare added this to the System strengthening milestone Aug 6, 2024
@Nashtare Nashtare requested a review from 4l0n50 August 6, 2024 16:49
@Nashtare Nashtare self-assigned this Aug 6, 2024
@Nashtare Nashtare requested a review from muursh as a code owner August 6, 2024 16:49
@Nashtare Nashtare changed the title Do not panic on wire set twice or generator not run issues Do not panic on wire set twice or generator not run issues Aug 6, 2024
Copy link
Contributor

@4l0n50 4l0n50 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 to me!

);
F::ZERO,
)
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

since IoResult is also a Result, can you map the error instead of panicking?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's a unit test, and it doesn't return anything hence why I kept unwrap() (there are some pre-existing ones below)

@Nashtare Nashtare merged commit 349beae into main Aug 7, 2024
5 checks passed
@Nashtare Nashtare deleted the wire_error branch August 7, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Panic when trying to generate proof with invalid witness
3 participants