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