Closed cjpatton closed 4 months ago
One other example: GEN_ORDER has a type hint that says "Unsigned", but it is actually sage.rings.integer.Integer. This matters because there's a place it is used in single-slash division, and the result is used as an exponent. If it were Unsigned, this would be raising an element to a power of a floating point number, but in reality, the power is a sage rational number, with no fractional part, which is OK. (Thanks to @jbr for catching this)
Ugh, yeah the fact that Sage is effectively "compiled" to Python3 makes this issue quite hairy. Then intended semantics of __div__(self, other)
for Field
is indeed self * other.inv()
.
Still unclear as to whether Sage is really the best choice for the reference implementation/spec. @chris-wood had prepared a rant for IETF 113 that touches on this very question, but we unfortunately ran out of time to give him a full hearing.
I don't see an obvious way to enforce these types: https://github.com/cfrg/draft-irtf-cfrg-vdaf/blob/6684fcca6995e080e10bf852d68f3671bc568dca/poc/vdaf_prio3.py#L37-L54
Type annotations are used in the reference implementation in order to generate a more readable spec. Currently these type annotations aren't for correctness. For instance, in https://github.com/cfrg/draft-irtf-cfrg-vdaf/pull/57, @jbr noticed that type for the verification parameter for Prio3 is not defined.
In python3 it's possible to use a static type checker, like https://mypy.readthedocs.io/en/stable/, for this purpose. At the very least, we should extend the unit tests to check that class attributes that are expected to be non-
None
have a value.Note that, for the verification parameter in particular, a solution to https://github.com/cfrg/draft-irtf-cfrg-vdaf/issues/38 might make this a moot point. However there are many situations for which this would be useful.