Implement new range check technique#1349
Conversation
There was a problem hiding this comment.
This change LGTM compared to the spec changes in cfrg/draft-irtf-cfrg-vdaf#576 and cfrg/draft-irtf-cfrg-vdaf#581.
The awkward thing is that the test vectors don't correspond to draft-irtf-cfrg-vdaf-18, but those vectors also account for the Rhizomes work to evaluate polynomials in the Lagrange basis, which we don't have yet. So I think we should merge this as-is, and we'll sync up with the VDAF test vectors in the next change we take.
It's also unfortunate to have to disable the Mastic test vector tests, but we'd need that specification to update before we can do anything about it, and I don't want to hold up shipping draft-irtf-cfrg-vdaf-18's Prio3 on that.
| ))); | ||
| } | ||
| flattened.extend(F::encode_as_bitvector(*summand, self.bits)?); | ||
| encode_range_checked_int(*summand, self.bits, self.last_weight, &mut flattened)?; |
There was a problem hiding this comment.
nit: kind of a shame to trade the real clear "summand exceeds maximum of 2^{}-1" for a FieldError::InputSizeMismatch.
We could put back in an explicit bounds check, similar to how Sum::encode_measurement checks at line 245.
There was a problem hiding this comment.
I'm going to add a new error variant for this. InputSizeMismatch isn't appropriate, that's intended for trying to add two vectors of field elements with different lengths.
divergentdave
force-pushed
the
david/range-check-changes
branch
from
b08e007 to
6d95937
Compare
jcjones
left a comment
jcjones
left a comment
There was a problem hiding this comment.
These are some great improvements on top of my comments -- looks great to me now!
divergentdave
left a comment
divergentdave
left a comment
There was a problem hiding this comment.
Oops, forgot to submit my pending comment
| ))); | ||
| } | ||
| flattened.extend(F::encode_as_bitvector(*summand, self.bits)?); | ||
| encode_range_checked_int(*summand, self.bits, self.last_weight, &mut flattened)?; |
There was a problem hiding this comment.
I'm going to add a new error variant for this. InputSizeMismatch isn't appropriate, that's intended for trying to add two vectors of field elements with different lengths.
3 participants
This implements the technique from cfrg/draft-irtf-cfrg-vdaf#549 in Prio3Sum, Prio3SumVec, and Prio3MultihotCountVec. See spec PRs cfrg/draft-irtf-cfrg-vdaf#576 and cfrg/draft-irtf-cfrg-vdaf#581.
I updated the test vectors with files from those PRs, and this implementation matches (though I had to unwind some field name changes that haven't been adopted here yet). I implemented the common encoding/decoding step in a helper function, and used that in the implementation of each FLP type.
Note that I had to add some new trait bounds from the subtle crate to our
Integerassociated type, in order to make encoding constant time. I added them here instead of on the relatedIntegertrait because they each imply aCopybound. We previously left theCopybound on this associated type, and not theIntegertrait, because we reuse the trait in tests where we implement it for a non-Copyarbitrary precision integer type, to help withGF(2^255-19)tests.I'm opening this as a draft for now until we arrive at a decision on the spec changes.