-
Notifications
You must be signed in to change notification settings - Fork 48
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
feature: pre-garbling #79
Conversation
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.
Nice 🚀
@@ -60,6 +61,10 @@ struct State { | |||
received_values: HashMap<ValueId, ValueType>, | |||
/// Values which have been decoded | |||
decoded_values: HashSet<ValueId>, | |||
/// Pre-transferred garbled circuits | |||
/// | |||
/// (inputs, outputs) => garbled circuit |
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 do not understand this comment line.
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.
It is just indicating that the inputs + outputs are used to map to the garbled circuit if it exists
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.
Ok, then maybe
/// (inputs, outputs) => garbled circuit | |
/// inputs and outputs from `receive_garbled_circuit` are used for one of the circuits from `garbled_circuits` |
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 is pretty verbose 🙃
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.
That's true. Do you have a shorter but more understandable idea? I do not think that the current comment is easily understandable, because I looked for input
and output
in the state variables, could not find them and was confused.
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.
Maybe worth emphaizing that (inputs, outputs) is unique to each circuit by making the comment
// A map used to look up a garbled circuit by its unique (inputs, outputs) reference.
7168ba5
to
7669232
Compare
@@ -60,6 +61,10 @@ struct State { | |||
received_values: HashMap<ValueId, ValueType>, | |||
/// Values which have been decoded | |||
decoded_values: HashSet<ValueId>, | |||
/// Pre-transferred garbled circuits | |||
/// | |||
/// (inputs, outputs) => garbled circuit |
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.
Maybe worth emphaizing that (inputs, outputs) is unique to each circuit by making the comment
// A map used to look up a garbled circuit by its unique (inputs, outputs) reference.
@@ -229,8 +242,29 @@ impl Generator { | |||
sink: &mut S, | |||
hash: bool, | |||
) -> Result<(Vec<EncodedValue<encoding_state::Full>>, Option<Hash>), GeneratorError> { | |||
let refs = CircuitRefs { |
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.
Seems like for the same inputs/outputs we may be getting different 'CircuitRefs' if they are passed in a different order.
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.
Yes that is true, this order dependence exists elsewhere in the codebase as well. I'm going to punt that out of scope of this PR tho. This is not a security issue but could cause a "cache-miss" or a deadlock
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.
lgtm, a fix maybe needed
7669232
to
c1e39f9
Compare
This PR implements "pre-garbling", aka the offline-online paradigm, for garbled circuits.
Changes
Load
trait, an abstraction of this featureGenerator
andEvaluator
to support transferring of the encrypted gates prior to executionLoad
forDEAPVM