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 word alignment checks for LOAD32 and STORE32 #154

Merged
merged 1 commit into from
Apr 20, 2024

Conversation

thealmarty
Copy link
Collaborator

@thealmarty thealmarty commented Apr 19, 2024

If any of the read/write addresses are not multiple of 4, the VM panics with an error message describing which opcode and read/write address is the problem.

I've confirmed that these checks work on runtime. I.e., I made a false assertion to an address value and confirmed that the VM does panic.

I've confirmed that cargo test and C unit tests all pass locally.

@thealmarty thealmarty force-pushed the thealmarty-ldst-align branch from 1d6423c to 6fb6498 Compare April 19, 2024 17:13
@morganthomas morganthomas merged commit 0a1b859 into main Apr 20, 2024
2 checks passed
@thealmarty
Copy link
Collaborator Author

Sorry I'm having second thoughts on this. Perhaps this is overkill and I should use debug_assert instead? I'm worried about affecting the performance with assert. If the words are not aligned, the prover should fail to prove, and then users can check with our debug version of the VM?

@morganthomas
Copy link
Collaborator

I don't think execution costs are much of a concern at this point because proving costs are so much higher. I'm fine with it as is.

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.

2 participants