zksecurity / noname

Noname: a programming language to write zkapps
https://zksecurity.github.io/noname/
178 stars 44 forks source link

Refactor using backend associated CellVar trait #69

Closed katat closed 3 months ago

katat commented 3 months ago

Here are the places requiring the index of cell var. We can think of a way to make them opaque.

https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/var.rs#L128-L133

https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/backends/mod.rs#L113-L119

https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/circuit_writer/writer.rs#L739

mimoo commented 3 months ago

I think all these places could be fixed :o I think it'd make sense for compute_var to fully live in the backend, and for the private input indices to either live there or store cellvar instead of indices (but I think we end up using them in the backend anyway no? So might as well have it there)

katat commented 3 months ago

It should be trivial for the private indices to fully live in the backend.

For the compute_var, we would wrap the index as CellVar and call the compute_var directly in the following code.

https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/backends/r1cs/mod.rs#L304-L316

So the compute_val can be deprecated, as it exposes the index usize.

Otherwise, we may have to override the compute_var with the same code for each backend, in order to have direct access to the var index instead of index().

katat commented 3 months ago

If we can get rid of the Value::Hint, adding static might not be necessary, and the compute_var can be completely lived in the backend.

The challenge I think is how we can replace the following without a Value::Hint: https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/constraints/field.rs#L199-L210

https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/constraints/field.rs#L338-L352

https://github.com/zksecurity/noname/blob/8196e389535fd60889432561da2729d34a472cc6/src/backends/kimchi/builtin.rs#L99-L117

mimoo commented 3 months ago

for the Hint::Value I propose that we either find linear combinations to express these OR add a new Value::Poseidon() => which contains the logic