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

Reading from the Public Parameter Cache Always Fails #401

Closed
winston-h-zhang opened this issue May 19, 2023 · 6 comments · Fixed by #499
Closed

Reading from the Public Parameter Cache Always Fails #401

winston-h-zhang opened this issue May 19, 2023 · 6 comments · Fixed by #499
Assignees

Comments

@winston-h-zhang
Copy link
Member

In #391, we optimized the time and space to cache with bincode. However, it seems like reading from the cache always results in failure. When we add some print statements for the errors:

➜  lurk-rs git:(hz/fib-benchmark) ✗ cargo run --release --example sha256 1 f5a5fd42d16a20302798ef6ed309979b43003d2320d9f0e8ea9831a92759fb4b false
    Finished release [optimized] target(s) in 0.58s
     Running `target/release/examples/sha256 1 f5a5fd42d16a20302798ef6ed309979b43003d2320d9f0e8ea9831a92759fb4b false`
Setting up public parameters...
Cache deserialization error: invalid length 1, expected struct PoseidonConstants
Writing public params to disk-cache: -SHA256-HASH-64-ZERO-BYTES
Public parameters took 47.033343459s
Beginning proof step...
T
Proofs took 464.069375ms
Verifying proof...
Verify took 327.120542ms
Congratulations! You proved and verified a SHA256 hash calculation in 47.824533376s time!

So, we always end up regenerating and writing new public params.

@winston-h-zhang winston-h-zhang self-assigned this May 19, 2023
@winston-h-zhang
Copy link
Member Author

Looking at the error and at Neptune, there's a handwritten serializer for PoseidonConstants, which seems to be causing issues for bincode. @huitseeker Would you have any knowledge of this weird behavior?

Also, if anyone can run on their machine to validate, that would be great confirmation too.

@porcuquine
Copy link
Collaborator

If I understand the issue, we probably should have checked to see whether loading the parameters worked (ideally with test coverage) before merging #391 then.

@huitseeker
Copy link
Member

One thing to sanity-check: any change in serialization format between a read and a write is likely to generate this error. So any format change should lead to deletion of the comm data path, so as to not create false positives.

@winston-h-zhang
Copy link
Member Author

winston-h-zhang commented May 19, 2023

If I understand correctly, you mean that any failure to read should delete the previous artifact and write a new one in? Or do you mean to delete all of $FCOMM_DATA_PATH/public_params/**? If the former, that is already the current behavior; we should be more clear with it.

@huitseeker
Copy link
Member

I meant the former, all good then!

@huitseeker
Copy link
Member

This will be resolved by the next Nova release (microsoft/Nova#172)

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 a pull request may close this issue.

3 participants