feat: foldable chacha20 #51
Conversation
This reverts commit 8aa8b08.
0xJepsen
requested review from
lonerapier and
Autoparallel
and removed request for
lonerapier
|
lol the name here |
There was a problem hiding this comment.
Don't remove this please! No harm in keeping it in here for now.
There was a problem hiding this comment.
Don't remove this either.
There was a problem hiding this comment.
Comment for the future:
I think we may be able to byte pack all of this. Expanding 32 field elements for 32 bits is likely not necessary. We can fit that amount of data into 2 field elements!
I'm not sure if this would help given we have to do bitwise XOR, but I do think there are probably tricks to do bitwise XOR on bytes or, say, 128bit numbers. Maybe not. Food for thought!
This circuit already seems wildly more efficient, so I'm not really worried about it.
There was a problem hiding this comment.
This also goes for all the chacha circuits we use.
| var tmp[4][32] = in; | ||
|
|
||
| // a += b | ||
| component add1 = AddBits(32); |
There was a problem hiding this comment.
Like surely we can do this 32 bit add more efficiently, right?
| // counter => 32-bit word to apply w nonce | ||
| signal input counter[32]; | ||
|
|
||
| // the below can be both ciphertext or plaintext depending on the direction |
There was a problem hiding this comment.
I don't quite get this. Is this to say that the encryption algorithm is exactly the same as decryption?
|
|
||
|
|
||
| describe("chacha20-nivc", () => { | ||
| describe("2 block test", () => { |
There was a problem hiding this comment.
| describe("2 block test", () => { | |
| describe("16 block test", () => { |
Is this really testing two blocks? It looks like it is doing 16 at once. Maybe i misunderstand the naming between block/words.
| // Test case from RCF https://www.rfc-editor.org/rfc/rfc7539.html#section-2.4.2 | ||
| // the input encoding here is not the most intuitive. inputs are serialized as little endian. | ||
| // i.e. "e4e7f110" is serialized as "10 f1 e7 e4". So the way i am reading in inputs is | ||
| // to ensure that every 32 bit word is byte reversed before being turned into bits. |
| @@ -212,7 +206,7 @@ const json_key3_mask = [ | |||
| const json_key3_mask_hash = DataHasher(json_key3_mask); | |||
|
|
|||
| describe("NIVC_FULL", async () => { | |||
| let aesCircuit: WitnessTester<["key", "iv", "aad", "ctr", "plainText", "cipherText", "step_in"], ["step_out"]>; | |||
There was a problem hiding this comment.
I'm not a fan of deleting a whole test case. We may want to come back to AES. We don't know yet, but I don't want to remove this circuit and its test in this PR.
In general, PRs should do one thing -- this one adds fodlable ChaCha20. It should not remove AES-GCTR. This is the main reason this got reverted.
| signal input in[BITS]; | ||
| signal output out[BITS]; | ||
| for (var i = 0; i < BITS; i++) { | ||
| out[i] <== in[(i + L) % BITS]; |
There was a problem hiding this comment.
modular arithmetic is the best thing
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "name": "web-prover-circuits", | |||
| "description": "ZK Circuits for WebProofs", | |||
| "version": "0.5.4", | |||
| "version": "0.6.0", | |||
There was a problem hiding this comment.
I'm partial to putting this as v0.5.5 as this does not break an API here. It introduces an extended API.
This is less important to me though. I could go either way.
Reverts #50
This one is reopen so now you can review