[kimchi][doc] update readme example with circuit-construction API #555

katat · 2022-05-12T07:59:54Z

Refer to #527

This PR aims to fix the compiling of the example in the Kimchi readme in the cargo test command while demonstrating how to use the library from a high-level perspective.

At the moment, the updates were inspired by the tests in the kimchi package, mainly to get the compiling passes. So probably it has not yet reflected the original goal.

If I may, here are a couple of questions about the goals of the example:

public inputs and witness setups - Can I assume the code for these setups would need to be included in the readme as well so that the gates variable can be valid in the scope? The problem is setting up these variables would make the script a bit long, but it would make it easier to follow where these variables come from.
b_poly_coefficients, ceil_log2 - These modules are in the example, but they are not being used. I guess they were intended to demonstrate some math with the library?
URS - The comment says to create URS, but the actual code is to create SRS. I may need to clarify if the code was diverging from the original purposes.
As you see from the updates in the current PR, the public and witness inputs are empty. I would assume it'd better come up with an example with non-empty inputs so as to demonstrate the math behind how the prover and verifier interact.

mimoo · 2022-05-12T10:06:10Z

README.md

-let cs = ConstraintSystem::<Fp>::create(gates, vec![], fp_sponge_params, public_size).unwrap();
+let public = vec![Fp::from(0); 0];
+let mut witness: [Vec<Fp>; COLUMNS] = array_init(|_| vec![Fp::zero(); gates.len()]);
+fill_in_witness(0, &mut witness, &public);


idea we wouldn't use testing functions like create_circuit and fill_in_witness but I guess without the circuit writer PR in we don't have much choices. cc @imeckler

mimoo · 2022-05-12T10:06:23Z

README.md


 // create an URS
-let mut urs = SRS::<Affine>::create(cs.domain.d1.size as usize);
+let mut srs = SRS::<Affine>::create(cs.domain.d1.size as usize);


if you rename this you can rename the comment just above as well :o

mimoo · 2022-05-12T10:14:06Z

Thanks a lot for the PR! This is great :) here are some answers:

public inputs and witness setups - Can I assume the code for these setups would need to be included in the readme as well so that the gates variable can be valid in the scope? The problem is setting up these variables would make the script a bit long, but it would make it easier to follow where these variables come from.

the problem is that we don't have the circuit writer yet (I believe it's a matter of days though). For now, it's probably fine to use the testing functions to create the circuit and generate the witness, but once we have the circuit writer we'll need to revisit this README :)

b_poly_coefficients, ceil_log2 - These modules are in the example, but they are not being used. I guess they were intended to demonstrate some math with the library?

ah, we should remove them from the example. They're here for the recursion but we removed it from the example as it's not really exposed in kimchi yet.

URS - The comment says to create URS, but the actual code is to create SRS. I may need to clarify if the code was diverging from the original purposes.

This is actually an URS (as there's no trusted setup) but we call it SRS in the code for legacy reason. Might be a good idea to rename SRS here and not care too much about that distinction. I think it should be called something user friendly anyway, like CommonParameters or something.

As you see from the updates in the current PR, the public and witness inputs are empty. I would assume it'd better come up with an example with non-empty inputs so as to demonstrate the math behind how the prover and verifier interact.

that would mean writing an example of writing a circuit without the circuit writer, so same answer, maybe better if we wait for the PR to get in

katat · 2022-05-28T08:08:03Z

Thanks @mimoo for the feedback :)

While waiting for the circuit writer APIs to be ready, I have been trying to understand the maths behind kimchi, concepts such as polynomial commitment schemes, and how plonk works thanks to your videos and the book "the real world cryptography". I have to say these learning materials are really helpful, especially since there is a codebase from kimchi to confirm the understanding of the math concepts. Now at least I think I got some basic understanding of the pieces of kimchi that works together and mapping in my mind between the code and the "mina book".

Based on what I can see relevant to kimchi so far, here are a couple of suggestions relevant to the referenced issue #527, so that the readme can remain simple and serve as an introductory overview for the references to the examples and relevant documentation.

examples in readme
Alternatively, I think these examples could be provided under a specific folder (maybe a folder called examples?) or a separate Github repo. The reasoning is that even with the circuit construction API, based on what I see in the code in the PR, I guess it might be still too verbose to put all the example code into the readme since there are both high and low levels of concepts that need to explain to the starters to fully understand the how to use the kimchi API at both levels.

For the high level, examples with the circuit construction API for some common programs, such as hashing or sudoku, would be helpful for the starters to get an overview and dive into the implementation code or automated tests themselves.

For the low level, examples of how to create gates, piecing them up in a circuit for a simple program, and finally open a proof for verification, would be helpful. Although there are some existing automated tests against different gate types, adding more smaller granular tests would be also helpful as examples for the developers to understand the concepts along with the mina book.

In comparison with the snarkyjs, I think kimchi offers the flexibility to integrate with the other systems instead of mina. Some examples of these would be really cool, such as using kimchi to generate proofs for an ethereum contract to verify (not sure if this is possible yet, a research topic I have in mind at the moment. I know there is a bba proposal, and it seems something to relevant to this topic).

unit tests
Currently, from my understanding, most automated tests are at the integration level, which tests for workflows in which a number of different functions are put together. Integration tests are indeed cost-effective to create and serve demonstrative purposes at a high level. On the other hand, unit tests in a smaller granularity may provide additional benefits.

Facilitate understanding of how a small function works by decoupling it from the other functions during tests and making a variety of assertions for a single function. This makes it convenient for the developers to review the implementation expectations and their relationship with the code quality itself.

It may make the code easier to refactor in the long run since unit tests help decouple the code right the way at the beginning and could provide more "signals" in refactoring processes.

It may help understand the math concepts if math equations can be decoupled into smaller functions so that it would be easier for anyone, the ones without a math degree, to easily play around with the inputs and outputs to confirm the understanding of the maths from the mina book or somewhere else.

example ideas
As the circuit construction API is getting ready, I am currently thinking of starter use cases that I can experiment with in addition to the hash example in the circuit API PR. One of them is sudoku, which once implemented can be used as a reference to compare with the snarkyjs version while demonstrating the full life cycle of using circuit API in rust.

mimoo · 2022-06-02T12:41:31Z

Sorry for the late answers! And thanks for the kind words :)

examples in readme

I'm on the side of "every README should show some examples first". They don't have to be exhaustive examples, but they should give an idea of how the library is to be used. We can add more verbose and useful examples in an example folder.

unit tests

agree with everything you said :o

example ideas

I was actually writing one for fun, I can push my code somewhere if you want to work on that and use some inspiration?

EDIT: pushed it here. It was a heavy WIP that wasn't working yet :D

katat · 2022-06-02T13:29:04Z

I'm on the side of "every README should show some examples first". They don't have to be exhaustive examples, but they should give an idea of how the library is to be used. We can add more verbose and useful examples in an example folder.

Agree, this makes sense.

I was actually writing one for fun, I can push my code somewhere if you want to work on that and use some inspiration?

EDIT: pushed it here. It was a heavy WIP that wasn't working yet :D

Aha, in the last couple of days, I have been actually trying to do something similar by creating necessary additional gates, such as for checking whether values are equal while passing the result variable to the next gate (this seems missing in the current circuit API PR), in order to implement the sudoku in rust, having the verification logic corresponding to the existing JS example.

Yes, I would love to continue your work there. I believe working on examples like this is a great opportunity for me to get a deeper understanding of the kimchi code structure as well as digest the knowledge behind the mina book :D

katat · 2022-06-03T09:19:48Z

I was actually writing one for fun, I can push my code somewhere if you want to work on that and use some inspiration?

EDIT: pushed it here. It was a heavy WIP that wasn't working yet :D

After reading the code in your branch, I think it would be nice for the circuit API to add gates such as and or, in addition to your implemented equals, so that the example can be comparable with the JS implementation. Then the equals might be better to be decoupled into sub, invert is_zero etc.

Please let me know if this is the right path, then I will try to get them implemented in the next few days :)

mimoo · 2022-06-03T09:51:42Z

Yeah these sounds like excellent additions! BTW just pushed the latest changes there.

Also the equals gadget I added is probably inefficient. I'm not sure how it's implemented on the js side but we'd probably want to copy that :)

EDIT: just pushed that

katat · 2022-06-10T07:56:09Z

I am trying to make a simple example so as to keep the readme example minimum, as there will be other examples, such as the existing hash example and the upcoming sudoku in your branch, located in a separated folder.

There are a couple of questions commented as "TODO" in the example of the PR, wondering if it makes sense further simplify the setups and the arguments.

atm, the example in the readme fails to compile due to the cyclic dependency issue. That is because the readme is located in the kimchi package, to get the example compiled, kimchi needs to use the circuit-construction package, while the circuit-construction package already uses kimchi package the way around. I am figuring out if there is a better way to resolve this.

kimchi/Cargo.toml

mimoo · 2022-06-10T15:32:33Z

README.md

 use oracle::{
-    constants::PlonkSpongeConstantsKimchi,
+    constants::*,


In the future, we really ought to make it easy to the user by just having them do use circuit_writer::prologue::* or at least only have to import these things through circuit_writer (we’d have to re-export things there with pub use)

Updated the circuit-construction to do the re-export. Please have a check if that makes sense, otherwise I can revert this commit and leave it to a dedicated future refactor task.

btw, it seems to me it would look cleaner to have circuit-construction/src/lib.rs file separated into several different files and leave the lib.rs only holds the definitions of mods and re-exports.

btw, it seems to me it would look cleaner to have circuit-construction/src/lib.rs file separated into several different files and leave the lib.rs only holds the definitions of mods and re-exports.

agree, if you want to take a stab at it be my guest, otherwise I might get to it in a week or so

I am happy to :D
I will create a PR for it.

Here is an attempt to refactor the file structure. Let me know any suggestions :D katat#1

mimoo · 2022-06-10T15:33:18Z

README.md

@@ -19,44 +19,80 @@ We assume that you already have:

 Then, you can create an URS for your circuit in the following way:

-```rust,ignore


I guess the description above the example could be updated

Updated to outline the workflow and try to explain the code with a bit more comments. Please have a check.

Note I still use the name circuit-construction and the name kimchi still remains. I guess they can remain until a new name for circuit-construction is determined.

README.md

mimoo · 2022-06-10T15:49:31Z

README.md

+
+// create SRS
+let srs = {
+    //TODO how to determine depth value? it throws error when the depth is too large


Yeah basically lol, the usecase we’d want to support is either 1) the user doesn’t care and we can generate the correct srs for them or 2) the user creates a large srs and this will be the upperbound for all circuits that use it (that’s what mina does)

README.md

mimoo · 2022-06-11T23:59:33Z

README.md

+use ark_ff::{FftField, PrimeField};
+use ark_poly::{EvaluationDomain, Radix2EvaluationDomain};
+use circuit_construction::{
+    *, 


I think we should avoid importing *, unless it's a well-defined module (like prologue) that is meant to be opened in the root scope

Here is an attempt to use a prologue. It does simplify the import a lot, just use one line. But I am not sure if there are any potential issues when it comes to practice :D

katat#1

mimoo · 2022-06-12T00:02:03Z

circuit-construction/tests/example_proof.rs

-    poseidon::{ArithmeticSponge, Sponge},
-    sponge::{DefaultFqSponge, DefaultFrSponge},
+use circuit_construction::{
+    *, 


mimoo

Amazing :) thanks for working on making this library more usable! Let's see if CI passes and I'll merge. Thanks again!

[kimchi][doc] get the example pass through the compiling

acec78f

mimoo reviewed May 12, 2022

View reviewed changes

querolita mentioned this pull request May 16, 2022

Expose Kimchi APIs - Research MinaProtocol/mina#10604

Closed

katat added 2 commits June 10, 2022 10:28

Merge branch 'o1-labs:master' into kimchi-readme-test

6e092df

update readme example with circuit APIs

03ab234