-
Notifications
You must be signed in to change notification settings - Fork 17
zk framework ppt #28
base: main
Are you sure you want to change the base?
zk framework ppt #28
Conversation
|
Looks like you need to sign off on your commit: https://github.com/hyperledger/ursa-rfcs/pull/28/checks?check_run_id=4711411658 |
Signed-off-by: Avradip Mandal <[email protected]>
|
Hi Brent,
Thanks for pointing that out. Now it should be fixed.
Avradip
…On Wed, Jan 5, 2022 at 11:23 AM Brent Zundel ***@***.***> wrote:
Looks like you need to sign off on your commit:
https://github.com/hyperledger/ursa-rfcs/pull/28/checks?check_run_id=4711411658
—
Reply to this email directly, view it on GitHub
<#28 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAUA326VM6VYM6XZWRBLBLUUSLDDANCNFSM5LJFALBA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
|
||
| We have chosen r1cs-select as frontend, which might not be preferred frontend for all users. | ||
|
|
||
| # Rationale and alternatives |
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 would like to see some content in this section
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.
Addressed in the new commit
|
|
||
| zk-interface is the defacto prior art. It has extensive features. However, it is not being actively maintained and set up is relatively complex. | ||
|
|
||
| # Unresolved questions |
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.
This section as well should be filled out
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.
Addressed in the new commit
| [reference-level-explanation]: #reference-level-explanation | ||
|
|
||
| main.rs: this file contains sample code on how to use the framework, get_circuit function defines the circuit for which zero knowledge proofs will be generated | ||
| r1csbellman.rs, r1csbulletproof.rs: modules that convert r1cs spec to underlying backends |
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.
can you include a normative r1cs definition and/or spec?
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.
Addressed in the new commit
0000-zk-framework.md
Outdated
| # Drawbacks | ||
| [drawbacks]: #drawbacks | ||
|
|
||
| We have chosen r1cs-select as frontend, which might not be preferred frontend for all users. |
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.
please provide an r1cs-select reference
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.
Addressed in the new commit
0000-zk-framework.md
Outdated
| # Prior art | ||
| [prior-art]: #prior-art | ||
|
|
||
| zk-interface is the defacto prior art. It has extensive features. However, it is not being actively maintained and set up is relatively complex. |
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 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.
Yes, included the reference in the new commit
|
General items:
|
Signed-off-by: Aveadip Mandal <[email protected]>
|
Thanks for the additional clarifications. |
|
I'm glad to see the additions that have been made. |
|
Things that need to be done:
|
brentzundel
left a comment
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.
@hartm I have more thoroughly reviewed
|
|
||
| # Summary | ||
| [summary]: #summary | ||
| Generic zero knowledge libraries such as bellman, bulletproof, etc come with two components |
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.
would like to see references to bellman and bulletproof libraries
| frontends and backends. Application programmers write their business logic using the frontend. The | ||
| backend handles the underlying cryptography. Traditional zero knowledge libraries do not allow mix | ||
| and match between different backends and frontends. In this zero knowledge framework one can use | ||
| rust r1cs library as front end and either bellman or bulletproof as backend. In future versions we |
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.
'a' rust R1CS library or 'the' R1CS library? In other words, does this sentence talk about how this zero knowledge framework is a rust library for R1CS, or how this zero knowledge framework uses an R1CS library?
If it is the former, that should be clarified, if it is the latter, there should be a reference to the R1CS library.
| on deployment options which might not be obvious to the application developer. Moreover, application | ||
| developers can have different preferences for different frontends. To make life easy for application | ||
| developers, ideally we should have seemless compatibility between different frontend and backends. In | ||
| the first version of this framework we support rust r1cs as front end, bellman and bullet proof as |
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.
another place for links
| # Guide-level explanation | ||
| [guide-level-explanation]: #guide-level-explanation | ||
|
|
||
| This project demonstrates R1CS logic with backends to Bellman and Bulletproofs. |
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.
another place for links
|
|
||
| main.rs: this file contains sample code on how to use the framework, get_circuit function defines | ||
| the circuit for which zero knowledge proofs will be generated | ||
| r1csbellman.rs, r1csbulletproof.rs: modules that convert r1cs spec to underlying backends |
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.
Above it talked about an R1CS library. This talks about an R1CS spec. In my mind those are two very different things. I don't see references here to either.
| # Drawbacks | ||
| [drawbacks]: #drawbacks | ||
|
|
||
| We have chosen r1cs rust library (https://github.com/mir-protocol/r1cs) as frontend, which might |
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.
here we have a link to the R1CS library!
I suppose this probably answers a question I had earlier about 'a' library vs 'the' library.
I recommend creating a # References section and putting all of the references in there, then linking to that section everywhere you need to in the text.
| - Why is this design the best in the space of possible designs? | ||
| - What other designs have been considered and what is the rationale for not | ||
| choosing them? | ||
| - What is the impact of not doing this? | ||
| - For incorporating new protocol implementations what other implementations | ||
| exist and why were they not selected? | ||
| - For new protocols, what related protocols exist and why do the not satisfy | ||
| requirements? |
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.
these are questions that should be answered in the alternatives section. The snippet below is okay, but doesn't go into the detail that answering these questions would.
While I would prefer to have actual answers to these questions, if the text below is intended to be sufficient, then the questions themselves should be removed.
| - What parts of the design do you expect to resolve through the RFC process | ||
| before this gets merged? | ||
| - What parts of the design do you expect to resolve through the implementation | ||
| of this feature before stabilization? | ||
| - What related issues do you consider out of scope for this RFC that could be | ||
| addressed in the future independently of the solution that comes out of this | ||
| RFC? |
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.
same comments here as in the alternatives section. The text is okay, but doesn't directly answer the questions.
It is also a good place to talk about all the things this RFC doesn't do (and why).
No description provided.