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

Poseidon integration tests fix #623

Merged
merged 9 commits into from
Aug 7, 2024
Merged

Conversation

MaksymMalicki
Copy link
Contributor

No description provided.

@MaksymMalicki
Copy link
Contributor Author

MaksymMalicki commented Aug 6, 2024

After comparing our memory with rust I've noticed, that in lambdaclass they are only infering the memory values for the segment addresses, which were found during getting op0 and op1. The memory values under those addresses are initially of type unknown. They decided to store all the produced hash inputs and outputs in a separate cache (address - mv map) and then populate only the memory values under the relevant addresses. What we were doing (and still are in different builtins), is that we are immiadetely writing the produced hash to the memory instead of cache, populating also the addresses which are apparently not relevant to the execution. That was the reason of the difference in the traces.

For instance in the screenshot below, I printed only the addresses found during getting op0 and op1 , which should infer the values for.
image
By writing the entire hash to the memory, we populated the holes between addresses (like 9:3 - 9:9) and thus the differences in the trace.

If you guys agree with the usage of cache, we should implement it for the ec_op and keccak, bc they write to more than one cell.

@MaksymMalicki MaksymMalicki marked this pull request as draft August 6, 2024 07:13
@MaksymMalicki MaksymMalicki marked this pull request as ready for review August 7, 2024 03:11
Copy link
Contributor

@TAdev0 TAdev0 left a comment

Choose a reason for hiding this comment

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

lgtm!

@TAdev0 TAdev0 merged commit 2ea2e10 into main Aug 7, 2024
4 checks passed
@TAdev0 TAdev0 deleted the poseidon_integration_tests_fix branch August 7, 2024 15:30
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