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
+-x into 2x #6
+-x into 2x #6
Changes from 9 commits
e5bb92c
f0ff599
c61f79e
8b91f01
ff8950f
b072b87
264f55e
38691f7
567eb59
9e72fbc
7bbf4f2
55f0fdb
3106df2
c79e62f
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.
Openssl (and maybe some other) backends of unknown_order ignore provided source of randomness. I just can imagine someone choosing openssl backend and, as a result, getting unintuitive errors and unexpected behavior.
Can we detect somehow which backend is chosen and produce
compiler_error!
(if it's possible to detect on compile-time) orpanic!
(if we can determine backend only in runtime)?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.
I think it's easier to forbid openssl as a feature, since a backend is explicitly selected with this crate. I'll disable it in the next commit
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.
Done
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.
You can do that, but one can bypass that by setting
pailler-zk.deafult-features = false
, and turning openssl feature on directly onunknown-order
. If we can't check which backend is used, we should at least add a note, that this backend is not supportedThere 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 don't have any root-level docs or readme where we could mention that, I'm fine if you open an issue to remember mention it in the docs in the future
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.
Ok, makes sense. Let's make this and the discussion below into a separate documentation MR