fix: check eval_proof for all evaluations #657
Conversation
akonring
changed the title
There was a problem hiding this comment.
Thanks Anders! Some comments below.
General comment: I'm having a hard time reading this code after such a long break since we last examined it. We need more comments about multiplicity, and that applies not just to code in this PR.
Example: each of the Share struct fields that is a Vec should have a comment explaining why it's a Vec like Vec length equals multiplicity or something.
|
Thanks for the review, @ggutoski. Trying to summarize issues so far:
Let me know if you agree with this assessment and scope of this PR.
We should add additional tests and comments when (fixing 660 and 659) as per your comment. |
vid/src/advz/test.rs
Outdated
| .expect_err("bad share value should fail verification"); | ||
| } | ||
|
|
||
| // corrupt the first eval proof of this share by assigning it the value of last |
There was a problem hiding this comment.
I thought the purpose of this test is to corrupt evals (and their proofs) for shares beyond the first, no?
Also, maybe add a comment for this test explaining that it's a regression test for #654 .
closes: #654 , #661
This PR:
See comment here:
#657 (comment)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
CHANGELOG.mdof touched crates.Files changedin the GitHub PR explorer