A simple GM Halo2 circuit for testing #17
Conversation
0xisk
force-pushed
the
feature/halo2-gm-circuit
branch
from
126be94 to
cb6f0b8
Compare
There was a problem hiding this comment.
Thank you @0xisk. The PR looks good to me, I left a few comments. Also, would it be better if we add probabilistic-encryption as a submodule instead since we are not modifying the code in anyway?
There was a problem hiding this comment.
A few comments on how these constants were chosen would be useful.
There was a problem hiding this comment.
These are the default values commonly used in various Halo2 examples. However, I plan to add a detailed explanation of how these values are chosen next week. I'm currently benchmarking another Halo2 project, which will provide deeper insights into the rationale behind these selections.
| // #[test] | ||
| // fn test_gm_encryption_mock() -> Result<()> { | ||
| // let path = "configs/gm_encryption.config"; | ||
| // let circuit_params: CircuitConfig = serde_json::from_reader( | ||
| // File::open(path) | ||
| // .map_err(|e| anyhow!(e)) | ||
| // .with_context(|| format!("The circuit config file does not exist: {}", path))?, | ||
| // ) | ||
| // .map_err(|e| anyhow!(e)) | ||
| // .with_context(|| format!("Failed to read the circuit config file: {}", path))?; | ||
|
|
||
| // let gm_verification_inputs = mock_gm_encryption()?; | ||
|
|
||
| // let mut halo2_wasm = Halo2Wasm::new(); | ||
|
|
||
| // halo2_wasm.config(circuit_params); | ||
|
|
||
| // let mut circuit = GmVerificationCircuit::new(&halo2_wasm, gm_verification_inputs); | ||
|
|
||
| // circuit.verify_encryption(); | ||
|
|
||
| // halo2_wasm.mock(); | ||
|
|
||
| // Ok(()) | ||
| // } |
There was a problem hiding this comment.
This snippet seems to be the same as the one above it? Was it left by mistake or we are keeping it for a reason?
There was a problem hiding this comment.
No it wasn't left by mistake, I was thinking how to get this test successful at the end as the expected constraint is that the expected_cipher is not equal to the actual_cipher. But I didn't find a way for doing such a constraint yet. So I will remove this test unit for now.
I would keep this forked version, because the original version is outdated. Some of the packages have conflicts like |
ndrwnaguib
force-pushed
the
feature/halo2-gm-circuit
branch
from
58f6bfb to
fbc397a
Compare
|
@0xisk As we have just decided during the meeting, would it be possible to export the changes as a patch file and add |
Adding submodules details
@ndrwnaguib I have pushed the changes into a patch file, also have updated the README docs for dealing with submodules. |
|
@ndrwnaguib I have updated the README, just added a simple project description. But imo I think it worth adding all your written doc in https://ndrwnaguib.com/zk-auctions/ in the README, I think it is easy to generate markdown out of it. |
|
After team discussions, we've decided to switch from Halo2 to Risc0. That is because at the moment it seems to us the most ready one and it provides good proving experience (runnable on laptops), and verifying experience (blockchain integration use groth16, same as circom). (@ndrwnaguib) |
Changes in this PR:
halo2-libandhalo2-wasm.