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

/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.
Ok.

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.
Agree, this is a goal to be pursued soonish, but I would like Marc to make a distribution of his work that can be accessed somewhere on the internet.
I believe the interfaces are identical

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...)
The code will not be a part of VIFF, but a prerequisite like Zope interfaces or Twisted. I will issue warning on the mailling list when I submit the changeset that will require it.

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

The append method is better for that.
Ok.


+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)
I slightly disagree.
 if "nacl" == options.paillier:
     paillier = NaClPaillier(options.keysize)
 else:
     paillier = ViffPaillier(options.keysize)
Is more natural in the current case.


+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.
This is a good question. I choose the current design because it leaves you with only one place you should change if you want to add another paillier implementation. I need to think some more before I comment further on this.

____________________________________________________

Janus Dam Nielsen

Research and Innovationspecialist, PhD.
CENTRE FOR IT-SECURITY

THE ALEXANDRA INSTITUTE LTD.

T +45 42 22 93 56
E janus.niel...@alexandra.dk
W alexandra.dk
____________________________________________________

_______________________________________________
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