cfrg / draft-irtf-cfrg-vdaf

VDAF specification
Other
18 stars 14 forks source link

Check types at runtime #361

Closed cjpatton closed 3 months ago

cjpatton commented 3 months ago

While working on #59 we found that the "class factory" pattern we're following for instantiating the Vdaf API is not enforceable by mypy. We still want type enforcement, so we'll do this at runtime instead. We think this will be easier/cleaner if we move away from the hacky class factory pattern to proper, Python-style OOP. E.g.:

class Vdaf:
   """An abstract base class."""
   def shard(self, measurement, nonce, rand):
       raise NotImplementedError()

   def shard_check_inputs(self, measurement, nonce, rand):
      assert type(measurement) == self.Measurement
      assert type(nonce) == bytes
      assert len(nonce) == self.NONCE_SIZE
      assert type(rand) == bytes
      assert len(rand) == self.RAND_SIZE      

Then a concrete VDAF would call this method explicitly:

class Prio3(Vdaf):
  def __init__(self, flp, field):
     self.field = field
     self.flp = flp
     self.Measurement = flp.Measurement
     ...

  def shard(self, measurement, nonce, rand):
      self.shard_check_inputs(measurement, nonce, rand)
      ...
divergentdave commented 3 months ago

For basic type checks, it would be better form to do assert isinstance(rand, bytes). For more complicated types, note that list[int] in the source turns into just list at runtime. Thus, we'd have to split up the assertions into, for example, assert isinstance(measurement, list) and assert all(isinstance(elem, int) for elem in measurement).

cjpatton commented 3 months ago

Which implies that the type would have to define its own type checking method :(

divergentdave commented 3 months ago

Yeah, at that point I think it would make sense to roll the asserts into the top of each VDAF method.

divergentdave commented 3 months ago

After #363, mypy's type checking is now working much better for us. While these type hints aren't checked at runtime, they still provide a good degree of assurance that our code is well-typed (especially with check_untyped_defs = True). Thus, there is no longer a need for manual runtime type assertions.