ZK-Garage / plonk

A pure Rust PLONK implementation using arkworks as a backend.
https://discord.gg/XWJdhVf37F
Mozilla Public License 2.0
289 stars 75 forks source link

Bug report: check_circuit_satisfied doesn't work #146

Closed XuyangSong closed 2 years ago

XuyangSong commented 2 years ago

panicked at here: index out of bounds https://github.com/ZK-Garage/plonk/blob/27733c60fbe4e32c21e88a344d450d29e977a4bf/plonk-core/src/constraint_system/composer.rs#L685

pi_vec.len() is less than self.n, I guess thatself.public_inputs.n is forgotten to update somewhere. The previous version is fine because pi_vec uses self. n as length.

davidnevadoc commented 2 years ago

Thanks for the report! The PublicInputs are typically initialized before knowing the final length of the circuit (n), so its size should be updated once all the constraints are added calling pi.update_size(n). Checkout this fix: https://github.com/ZK-Garage/plonk/pull/147/commits/56ade406f0f4e6f714e8f9f613fb6b4697b8c8fe

XuyangSong commented 2 years ago

Would it be better if adding test for it or adding check_circuit_satisfied in existing tests?

davidnevadoc commented 2 years ago

Yes, some test should be added to ensure this feature works.