feat(garble-core): randomize gate id and cipher key #348
Conversation
themighty1
changed the title
|
@sinui0 , could you ack the conceptual approach of this PR? Then I can resolve the conflicts. |
crates/garble-core/Cargo.toml
Outdated
| [dev-dependencies] | ||
| mpz-ot-core = { workspace = true, features = ["test-utils"] } | ||
|
|
||
| bincode = { workspace = true } |
There was a problem hiding this comment.
| bincode = { workspace = true } |
There was a problem hiding this comment.
we use bincode in a test which serializes and deserializes the SetupMsg to simulate a realistic scenario where the setup message is transmitted
There was a problem hiding this comment.
That is essentially just testing serde, I would omit that in unit tests unless you're specifically testing a custom serialization impl
There was a problem hiding this comment.
Sorry, I misspoke, it is actually used in crates/garble-core/benches/garble.rs
b.iter(|| {
let setup: SetupMsg = bincode::deserialize(&msg).unwrap();
let mut ev = Evaluator::default();
ev.setup(setup).unwrap();
I used it to avoid making SetupMsg Clone/Copy.
(oops, I noticed that I made SetupMsg Clone and Copy already which I shouldn't have)
do you agree with keeping bincode and removing Clone/Copy ?
There was a problem hiding this comment.
@sinui0 , i think you may have missed my last question here.
There was a problem hiding this comment.
I would just keep it Clone. Nothing wrong with that. Copy is too presumptuous. The serialization is unnecessary, IMO, but its just a nit
|
Addressed feedback, merged dev. |
This PR adds randomization to garbler's initial gate id and key.
The rationale for it was discussed in:
#341
#7