-
Notifications
You must be signed in to change notification settings - Fork 149
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
Lift the Default
bound for the Circuit
trait
#716
base: master
Are you sure you want to change the base?
Conversation
The `Compiler::compile` function keeps this bound since it actually requires the circuit to have a `Default` implementation, but all other functionality now works without said bound. Resolves #715
aff90c1
to
8ed3308
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #716 +/- ##
=========================================
Coverage ? 85.14%
=========================================
Files ? 47
Lines ? 3830
Branches ? 0
=========================================
Hits ? 3261
Misses ? 569
Partials ? 0
Continue to review full report at Codecov.
|
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.
LGTM
The The general idea behind this concept is that a type should have a single circuit (hence, a keys set). This will provide type-safety and will make it easier to store the keys statically. |
If it is the case that we shouldn't rely on downstream to inform us on what the API should look like, and we really want to enforce type safety - which is already questionable in the current implementation (see #713) - the we should remove |
You mean that the user can serialize a PLONK key using a type If we feel this is indeed a violation to the principle, then my suggestion would be to fix that issue and improve the security instead of removing the concept. One alternative is to put additional bytes in the key to identify the type, and assert upon deserialization. But my feeling is this is overkill as it will consume more bytes. The fact that we can't have this implementation for the execute circuit just highlights it should be refactored to comply with the new rules. |
The
Compiler::compile
function keeps this bound since it actually requires the circuit to have aDefault
implementation, but all other functionality now works without said bound.Resolves #715