Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

PB-662 simplify pet messages #383

Merged
merged 10 commits into from
Apr 21, 2020
Merged

Conversation

janpetschexain
Copy link
Contributor

@janpetschexain janpetschexain commented Apr 17, 2020

References

Summary

  • simplify messages: the old structure was roughly speaking a two-fold construction SumMessage {RoundBox, SumBox} using a sealedbox and a box_, each with their respective buffers. this is now simplified to a SumMessage using only a sealedbox (same for update and sum2). the only minor drawback is that each message must now contain its receiver (the coordinator pk) to avoid surreptitious forwarding (but that information is available anyway and only contributes to an additional 32 bytes, which is way less than the previous construction).
  • simplify/unify the buffer trait and structures
  • adopt the new type naming: extends the type aliases which were introduced with Coordinator refactoring #380/coordinator: introduce type aliases & some renaming #381. some things are still dummies like the masks and models, which will change when masking is implemented. also make the types available where necessary by moving them to the crate root.
  • adjust the tests accordingly
  • remove the encryption keys from the participant: the simplification allows us to go with signature keys only for the participant.
  • remove the signature keys from the coordinator: a change in the round parameter update allows us to go with encryption keys only for the coordinator. we maybe have to reintroduce them later, depending on the certificate solution we will choose (but these signature keys are going to remain the same over the course of the rounds then.)
  • consistify variable naming in some places
  • remove use of then_some and then from the unstable bool_to_option feature
  • add a length-value encoding for variable-sized buffer fields: this applies currently to the certificate, masked_model, local_dict_seed and mask fields
  • implement and use the From/Into and TryFrom/TryInto traits instead of custom from implementations

Notes

  • the sum2 message goes with the naming of mask instead of mask_hash: we are submitting whole masks directly via the messages for the first implementation, uploading to buckets might come later. the coordinator still stores only hashed masks in the mask_dict for efficiency.
  • same holds for masked_model instead of model_url
  • access to the buffer fields is not yet time-constant wrt the number of fields, this should be part of future optimization

@little-dude little-dude self-requested a review April 20, 2020 06:10
Copy link
Contributor

@little-dude little-dude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice :) This is such a simplification.

Copy link
Contributor

@finiteprods finiteprods left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. i like the use of _range functions defined in terms of the previous range in the sequence. just as a minor suggestion: maybe you could define a helper e.g.

fn range(prev_range, width) {
    prev_range.end .. prev_range.end + width
}

since you use this pattern a lot, and currently you are making 2 function calls (to the previous range) per range call.

rust/src/message/sum.rs Show resolved Hide resolved
@janpetschexain
Copy link
Contributor Author

looks good. i like the use of _range functions defined in terms of the previous range in the sequence. just as a minor suggestion: maybe you could define a helper e.g.

fn range(prev_range, width) {
    prev_range.end .. prev_range.end + width
}

since you use this pattern a lot, and currently you are making 2 function calls (to the previous range) per range call.

i'm currently working on some optimizations to make the access to the message buffer fields time-constant wrt to the number of variable-sized fields. the suggested change will be included in the next mr. the range checks in the try_from impl will change as well.

@janpetschexain janpetschexain merged commit 1f0f214 into pet Apr 21, 2020
@janpetschexain janpetschexain deleted the PB-662-simplify_pet_messages branch April 21, 2020 20:27
little-dude pushed a commit that referenced this pull request Apr 22, 2020
* PB-662 simplify pet messages

* PB-662 adopt new naming

* use Borrow to implement messages (#384)

* PB-662 adjust tests for borrow impl

* PB-662 add variable-sized buffer fields
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants