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

feat(storage-proofs): zizag, drgporep and por abstract over the hasher in circuits #543

Merged
merged 7 commits into from
Mar 19, 2019

Conversation

dignifiedquire
Copy link
Contributor

@dignifiedquire dignifiedquire commented Mar 12, 2019

Basic tests are passing. Needs some more checks

  • make por and drgporep circuits abstract over the hasher
  • implement blake2s hasher using blake2s_simd

Ref #531

@dignifiedquire dignifiedquire force-pushed the blake2s branch 2 times, most recently from 0971a3f to 45c9f3d Compare March 13, 2019 15:33
@dignifiedquire
Copy link
Contributor Author

@porcuquine this adds the hasher to the parameter identifiers, so I am guessing we have to change parameters versions again?

@porcuquine
Copy link
Collaborator

Hmmm, if it only changes the set of parameters generated but doesn't change the circuit/parameters for a given identifier, we don't need to bump the version. (The version is needed so we don't find old parameters by the name with which we request new ones.)

But we will need to generate and publish the parameter again. When doing that, remember to clear your cache first, so we don't keep stale parameters around in parameters.json.

(It's possible I'm seeing something wrong, so if this sounds mistaken, please flag.)

@dignifiedquire dignifiedquire force-pushed the blake2s branch 2 times, most recently from a597101 to e7bfd1d Compare March 13, 2019 20:18
@dignifiedquire dignifiedquire changed the title feat: make por abstract over the hasher feat(storage-proofs): zizag, drgporep and por abstract over the hasher in circuits Mar 13, 2019
@dignifiedquire
Copy link
Contributor Author

This is ready to go from my side. Note this will need new params published again, as now the hasher is part of the cache identifier

@porcuquine
Copy link
Collaborator

Can you include the param publishing in this PR? That would be the best way to do it and should be our policy. Include a comment here stating that you pinned and verified.

@porcuquine
Copy link
Collaborator

We should see a diff to parameters.json in this branch as well (over-explaining because this is new process).

@porcuquine
Copy link
Collaborator

Looking good, @dignifiedquire. Any chance of extending this to the merklepor example, so we can run that with --hasher blake2s?

porcuquine
porcuquine previously approved these changes Mar 15, 2019
Copy link
Collaborator

@porcuquine porcuquine left a comment

Choose a reason for hiding this comment

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

Nice.

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