Refactor SMPC protocol implementation #876
Conversation
| struct SharedIrisesBody { | ||
| points: Vec<GaloisRingSharedIris>, | ||
| } | ||
|
|
||
| impl std::fmt::Debug for Aby3NgStorePlayer { | ||
| #[derive(Clone)] | ||
| pub struct SharedIrises { | ||
| body: Arc<RwLock<SharedIrisesBody>>, | ||
| } |
There was a problem hiding this comment.
I'm a bit concerned about these struct names causing confusion because there are two different types of sharing involved -- secret shares and Arc references. One possibility that would be more explicit about this distinction would be SharedIrises for the struct with points, and SharedIrisesRef for the Arc<RwLock<_>>-wrapped version. These make it clear at least to me that the first contains actual shared irises, and the second is a reference to the shared irises. Thoughts?
| } | ||
| } | ||
|
|
||
| impl SharedIrises { | ||
| pub fn new_with_shared_db(data: Vec<GaloisRingSharedIris>) -> Self { |
There was a problem hiding this comment.
Can this constructor name be changed to just new? The with_shared_db suffix is a bit confusing because every instance of this type represents a shared database, and also again there's this ambiguity of the term "shared" -- secret sharing, Arc sharing, the input data being shared, or the data being shared after being moved, or...? I guess the purpose of the data type is clear enough by itself, so that the extra with_shared_db part of the name looks like a signpost for some complexity that isn't actually there.
| @@ -90,33 +112,32 @@ impl Aby3NgStorePlayer { | |||
| }) | |||
| } | |||
|
|
|||
| pub fn get_vector(&self, vector: &VectorId) -> &GaloisRingPoint { | |||
| &self.points[vector.id] | |||
| pub fn get_vector(&self, vector: &VectorId) -> GaloisRingSharedIris { | |||
There was a problem hiding this comment.
The return type change from an immutable ref to a cloned copy seems fine at the moment (all current uses of the method do a clone on the returned ref afterwards already), but since the clone happens internally, we should remove the extra clones that happen after calling get_vector, e.g. on lines 258, 275, ...
| } | ||
|
|
||
| /// Implementation of VectorStore based on the ABY3 framework (https://eprint.iacr.org/2018/403.pdf). |
There was a problem hiding this comment.
It looks like to fix this warning from building the docs you need to make this an "automatic link" by surrounding the url with angle brackets, as in
/// Implementation of VectorStore based on the ABY3 framework (<https://eprint.iacr.org/2018/403.pdf>).
| LocalNetAby3NgStoreProtocol, | ||
| GraphMem<LocalNetAby3NgStoreProtocol>, | ||
| )>, | ||
| Vec<(Aby3Store, GraphMem<Aby3Store>)>, |
There was a problem hiding this comment.
The new type signature is refreshingly concise! 😄
The main goal of this change is to make an implementation of the SMPC protocol (
Aby3Storeformerly known asLocalNetAby3NgStoreProtocol) more atomic, lightweight and independent of implementation details not related to the protocol. In particular,Arcto avoid unnecessary copying;In addition,
LocalRuntimeis simplified by removing unused fields.