Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Implemented Mova folding scheme #161
Implemented Mova folding scheme #161
Changes from 4 commits
637dc3c
3305e18
4940b48
a38eba0
9e60b9d
9f9361b
f61d660
54e6b60
b291a14
b2db660
8ba0165
eb45123
ff4c311
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Isn't this exactly as in
Nova
? Maybe we can reuse the struct from there and like this, gain some functions already implemented duplicating less codeThere 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.
Nova's witness also has a
rE
which is the blindness used for the commit of the Error term. Since we don't commit the error term in Mova, we don't need it. We can, however, if you prefer use the Nova's witness and make sure to always keep it at 0. To also address your comment below (about thefold_witness
) the difference is that in Nova we have the calculation ofrE
usingrT
. Again we can use it by keepingrT
always to zero. I am happy either way so just let me know which direction you prefer:Witness
andfold_witness
for MovarE
andrT
always zero.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.
Maybe we could add some comments for these structs. Specially since both have
w
inside. So would be nice to refer to the paper to understand the diffs etc..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.
There's a trait for this which would likely make more sense to implement.
See: https://github.com/privacy-scaling-explorations/sonobe/blob/main/folding-schemes/src/folding/nova/mod.rs#L82
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.
ditto.
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.
We should change the trait bounds here @winderica @arnaucube right? It should not affect the rest of impls. We will just need to enforce the bounds sometimes.
Maybe @NiDimi can you file an issue for this?
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.
Sorry for missing your comment! This is in fact introduced by me in https://github.com/privacy-scaling-explorations/sonobe/blame/cb1b8e37aa07dda255d3f3651385a45613beed4e/folding-schemes/src/folding/nova/mod.rs#L112-L121. Let me check if this can be resolved by changing the trait bound. Will create an issue or PR in these days!
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.
Issue opened: #170