viff-devel@viff.dk writes:

Hi Janus,

Some good old-fashioned code review coming up... :-)

> /rev/736ad1d97024
> changeset: 1361:736ad1d97024
> user:      Janus Dam Nielsen <janus.niel...@alexandra.dk>
> date:      Wed Oct 28 14:53:51 2009 +0100
> summary:   Generate_config_files:Added support NaCl implementation of 
> Paillier.

There's a space missing after the colon.

> diffstat:
>
>  apps/generate-config-files.py |  22 +++++++++++++++++++---
>  viff/paillierutil.py          |  20 +++++++++++++++++++-
>  2 files changed, 38 insertions(+), 4 deletions(-)
>
> diffs (90 lines):
>
> diff -r 3fe6e03541c1 -r 736ad1d97024 apps/generate-config-files.py
> --- a/apps/generate-config-files.py   Wed Oct 28 14:53:49 2009 +0100
> +++ b/apps/generate-config-files.py   Wed Oct 28 14:53:51 2009 +0100
> @@ -55,7 +55,17 @@
>  from optparse import OptionParser
>  
>  from viff.config import generate_configs
> -from viff.paillierutil import ViffPaillier
> +from viff.paillierutil import ViffPaillier, NaClPaillier
> +
> +try:
> +    import pypaillier
> +except ImportError:
> +    pypaillier = None

Are we getting a module called 'pypaillier' alongside the old module
called 'paillier'? I don't like that name very much. Perhaps we should
make a module called nacl so that you could do

  try:
      from viff.nacl import paillier
  except ImportError:
      from viff import paillier

and then make the interface identical for the two modules.

Also, can we please have that code put into VIFF? I don't like it that
we're getting more and more "secret" code floating around :-) Especially
not when we make changes to VIFF to accomodate this secret code -- it
would be different if we had simple drop-in Python replacements for it.

(I know you've said that this code can be made public since it's part of
NaCL, so this is more for the record...)

> +
> +paillier_choices = ['viff']
> +
> +if pypaillier:
> +    paillier_choices += ['nacl']

The append method is better for that.

> +paillier = ViffPaillier(options.keysize)
> +if "nacl" == options.paillier:
> +    paillier = NaClPaillier(options.keysize)

I think it's clearer if you write

  if options.paillier == "nacl":
      paillier = NaClPaillier(options.keysize)
  else:
      paillier = ViffPaillier(options.keysize)

> +try:
> +    import pypaillier
> +except ImportError:
> +    pypaillier = None
> +
> +
>  class Paillier:
>  
>      def __init__(self, keysize):
> @@ -35,8 +41,20 @@
>  
>      def generate_keys(self):
>          return paillier.generate_keys(self.keysize)
> +
> +class NaClPaillier:
> +
> +    def __init__(self, keysize):
> +        self.keysize = keysize
> +        self.type = 'nacl'
> +
> +    def generate_keys(self):
> +        return pypaillier.generate_keys(self.keysize)
>      
>  
>  def deserializer(paillier_type, str):
> -    return tuple(map(long, str))
> +    if paillier_type == "viff":
> +        return tuple(map(long, str))
> +    if paillier_type == "nacl":
> +        return str.dict()

I think that function would belong in the otherwise unnecessary classes
you define above? Or even better: make a function in two different
modules like I suggest above.

-- 
Martin Geisler

VIFF (Virtual Ideal Functionality Framework) brings easy and efficient
SMPC (Secure Multiparty Computation) to Python. See: http://viff.dk/.

Attachment: pgpgf17JAb0Xo.pgp
Description: PGP signature

_______________________________________________
viff-devel mailing list (http://viff.dk/)
viff-devel@viff.dk
http://lists.viff.dk/listinfo.cgi/viff-devel-viff.dk

Reply via email to