-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
w&b error correction interpolation algorithm implemented #4
Conversation
polynomial.py
Outdated
@@ -17,7 +20,7 @@ def __init__(self, coeffs): | |||
self.coeffs = strip_trailing_zeros(coeffs) | |||
self.field = field | |||
|
|||
def isZero(self): return self.coeffs == [] | |||
def isZero(self): return self.coeffs == [] or (len(self.coeffs) == 1 and self.coeffs[0] == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This shouldn't be necessary if strip_trailing_zeros
was called earlier
wb_interpolate.py
Outdated
from polynomial import polynomialsOver | ||
|
||
# n is the total number of messages send out == total number of nodes | ||
# k is the total number of shared secret, k = t + 1 with degree t polinomial |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slight terminology correct - this is Reed Solomon not secret share encoding. So k
is not the number of shared secrets, but it could be called the number of code symbols.
Polinomial -> Polynomial
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed!
Codecov Report
@@ Coverage Diff @@
## miniviff #4 +/- ##
==================================================
+ Coverage 58.7886% 63.74502% +4.95641%
==================================================
Files 11 13 +2
Lines 842 1004 +162
Branches 131 182 +51
==================================================
+ Hits 495 640 +145
- Misses 330 336 +6
- Partials 17 28 +11 |
tests/test_wb_interpolate.py
Outdated
return message | ||
|
||
|
||
test_decoding() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do not need to explicitly invoke the test function (test_decoding
) as pytest
takes care of finding and executing the test functions and classes.
So, when you run
pytest test_wb_interpolate.py
all functions starting with test_
and classes starting with Test
will be executed.
More information about this behavior can be found under https://docs.pytest.org/en/latest/goodpractices.html#conventions-for-python-test-discovery.
return random.randint(0, order) | ||
|
||
|
||
def simple_router(N): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should import this simple_router from somewhere rather than copy-pasting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably put it in a pytest
fixture.
Right now it is defined in test-asyncio-simplerouter.py
I think. We cannot import things from it I think because the module name contains "dashes/hyphens". A workaround is to use something like:
from importlib import import_module
simple_router = getattr(
import_module('honeybadgermpc.test-asyncio-simplerouter'), 'simple_router')
or in two steps, if you need more things from it
from importlib import import_module
async_simple_router_test_module = import_module('honeybadgermpc.test-asyncio-simplerouter')
simple_router = async_simple_router_test_module.simple_router
Share = async_simple_router_test_module.Share
# etc
also note that the above will work only if the Runtime
class is fixed, e.g.:
diff --git a/honeybadgermpc/test-asyncio-simplerouter.py b/honeybadgermpc/test-asyncio-simplerouter.py
index 90e64d5..14c8dee 100644
--- a/honeybadgermpc/test-asyncio-simplerouter.py
+++ b/honeybadgermpc/test-asyncio-simplerouter.py
@@ -51,7 +51,7 @@ class Runtime():
async def _run(self):
while True:
- await # noqa TODO fix: await ?; e.g.: await asyncio.sleep(1)
+ await asyncio.sleep(1) # noqa TODO fix: await ?; e.g.: await asyncio.sleep(1)
def createshare(self, val):
s = Share(self)
In any case, I think we should spend the time to put such things in fixtures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest the following:
- Move the test module
test_batch_reconstruction.py
under thetests
dir. - Add a
simple_router
fixture intests/conftest.py
, e.g.:
# tests/conftest.py
import asyncio
import random
from pytest import fixture
@fixture
def simple_router():
def _simple_router(N):
"""
Builds a set of connected channels
@return (receives, sends)
"""
# Create a mailbox for each party
mbox = [asyncio.Queue() for _ in range(N)]
def makeSend(i):
def _send(j, o):
# print('SEND %8s [%2d -> %2d]' % (o[0], i, j))
# random delay
asyncio.get_event_loop().call_later(
random.random()*1, mbox[j].put_nowait, (i, o))
return _send
def makeRecv(j):
async def _recv():
(i, o) = await mbox[j].get()
# print('RECV %8s [%2d -> %2d]' % (o[0], i, j))
return (i, o)
return _recv
sends = {}
receives = {}
for i in range(N):
sends[i] = makeSend(i)
receives[i] = makeRecv(i)
return (sends, receives)
return _simple_router
- Remove the
simple_router
function definition intests/test_batch_reconstruction.py
and use thesimple_router
fixture (defined inconftest.py
) in the test like so:
def test(simple_router):
N = 4
p = 73
t = 1
async def _test():
# Test with simple case: n = 4, t =1
# After AVSS, poly1 = x + 2, poly2 = 3x + 4, secret1 = 2, secret2 = 4
# Hard code the shared secret value as input into batch_reconstruction function
# The final constructed polynomial should be p = 4x + 2
sends, recvs = simple_router(N)
towait = []
for i in range(N):
# ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@changxu2 what is left to do before this can be merged? I think you need to add some integration with an MPC program, maybe |
@changxu2 One thing missing from batch_reconstruction is that if a fault is detected, we should return (along with the solution) a list of the player ids corresponding to any faults. Then we can ignore subsequent messages from them and effectively remove them from future comparisons |
|
@changxu2 I think you mean you'll leave c) the fft computation for after merging? If so yes that's fine |
What is the status of this @changxu2 it looks like a test is failing due to code style but that is it? |
Update with current miniviff
Not ready for merge