-
Notifications
You must be signed in to change notification settings - Fork 315
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 version of SnapDeals #1547
Conversation
5527615
to
4c23b61
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing this! Looks good to me.
A couple of comments/questions:
- It looks like this is a single partition circuit, am I correct in thinking that?
- I think that restructuring this module makes sense when the vanilla side is built (mostly Merkle tree/proof shape changes) after the trusted-setup is run, e.g. maybe splitting the
empty_sector_update
crate into two modulessha
andposeidon
(which signifyTreeD
's hasher and shape).
Yes, it is a single partition.
Agreed, I wanted to limit changes to SHA version that is why I moved new vanilla definitions to the circuit module itself and avoided touching the SHA sides in general where possible. |
There was a problem hiding this 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. Thanks!
@Kubuxu Can we get this rebased against latest so that it can have final review before merge? |
bb4bf33
to
988be1c
Compare
@Kubuxu @DrPeterVanNostrand @cryptonemo - Just wanted to say I'm glad we got this one in the scope. Great idea and all the work behind it. 🎉 👍 |
Signed-off-by: Jakub Sztandera kubuxu@protocol.ai