This repository has been archived by the owner on May 3, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BLAKE2b hash function and BLAKE2b-512 compression function of EIP-152 #56
base: main
Are you sure you want to change the base?
BLAKE2b hash function and BLAKE2b-512 compression function of EIP-152 #56
Changes from 29 commits
9dbcce9
4e867da
e5d6ec2
b0045c5
0678ec5
23ed14d
7c82788
8d82e42
4b4f8bb
a012bcf
c9150c3
43305b9
2c2bead
1481510
587964e
ba48da2
bc9527b
f8197a1
b3959f4
92e15c3
06a42ca
0f6d4ce
de96f71
edde7c6
e7b5b7d
5090ee8
394ef0f
b946c7e
0a5185c
8d62c17
b083887
34e0961
e2dd84c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm missing the part of this test that we assert the output equals the expected test vector output. Did I overlook something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we cannot obtain the output directly from the MockProver, we have to check it in a different way. The witnesses generated as the result of circuit synthesis are compared with the results computed by the function not used for witness generation:
// Checking the correctness of the computation
let (h_ex, rlc_ex) = RLCTable::compress_with_rlc(challenge, &input.h, &input.m, input.t, input.f, input.r);
let correctness = (h == h_ex) && (format!("{:?}", rlc) == format!("{:?}", rlc_ex));
assert!(correctness, "Processing of a BLAKE2b compression function input was incorrect! This input is {:?}", input);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a suggestion of a possible solution.
Given the
blake2b
circuit takes two advice inputs/witnesses (besides, ofc, the internal ones): the input of the hash, and its output.First we fetch the input and output from the advice
$$(input, output) \gets \mathbb{A}$$
Then we compute the blake hash into result as the circuit synthesize
$$result \gets \mathbb{C}$$
Finally, we assert the computed result to be the expected witness
$$result \equiv output$$
This approach forces the prover to compute the expected output himself, and allows the users of the circuit to pass this
output
advice column to different circuits. With that, we can easily pass the expected input/output from the test vectorsThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we do not need this, because in the current architecture the supercircuit will compute input/output pair (outside the circuit) during the interaction and hash them by means of random linear combination hash. The input/output pairs and their rlc hashes are also computed by the current circuit (inside it). The rlc hash computed by the supercircuit is going to be constrained to be into the lookup table containing all rlc's computed by the current circuit. Thus, if some i/o pair produced by this circuit is incorrect, then lookup constraint will not be satisfied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood; however, it is very expensive to test the super circuit. In case a bug is introduced here, it is preferable to test the blake circuit individually rather than depending on the full integrated test.
This is ofc not a blocker, so I'll open an issue to add a unit test in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#127
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do no need to test it in the supercircuit. The current test are sufficient. If MockProver doesn't panic during performing the test, this means that the computations performed during the circuit synthesis produce the expected output and all witness data satisfy the constraints. The correctness of the algorithm computing the expected compression function (outside the circuit) is checked by its own test.