Skip to content
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

add flag to encrypt_util for length delimited binary format #1509

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

eriktaubeneck
Copy link
Member

In order to support the new ability for helpers to load data from a file/url, we need the ability to produce length delimited encrypted data for testing. This adds a flag to the crypto_util called --length-delimited which will produce length delimited binary instead of newline delimited hex.

The next step is to use this with the integration test. The flow will be

  1. generate raw input data files (one for each shard)
  2. use crypto_util to encrypt each of these for each shard (and each helper)
  3. pass these files into the report collector (is this implemented yet @akoshelev ?)
  4. cat all input data files together and run in-the-clear
  5. compare results

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 99.20000% with 1 line in your changes missing coverage. Please review.

Project coverage is 93.24%. Comparing base (6531c97) to head (9760121).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
ipa-core/src/cli/crypto/hybrid_encrypt.rs 99.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1509      +/-   ##
==========================================
- Coverage   93.24%   93.24%   -0.01%     
==========================================
  Files         238      240       +2     
  Lines       43688    43915     +227     
==========================================
+ Hits        40739    40949     +210     
- Misses       2949     2966      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -52,15 +52,38 @@ pub struct HybridEncryptArgs {
/// Path to helper network configuration file
#[arg(long)]
network: PathBuf,
/// a flag to produce length delimited binary instead of newline delimited hex
#[arg(long)]
length_delimited: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of a boolean argument it might be clearer to have descriptive options (something like the FileFormat choices -- possibly clap can even be taught to parse into FileFormat).

Copy link
Member Author

Choose a reason for hiding this comment

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

I generally think of docopt as the canonical way to design CLIs, and this would probably fit as a mutually exclusive element better than an element with an arg. Clap probably has a way of defining mutually exclusive elements (and as you say, clap could likely be taught to coerce that into the enum.) This would be even more important if there were more than two options.

I likely won't get to this before I'm out, so up to @andyleiserson and @akoshelev if you want to refactor before merging.

@akoshelev akoshelev merged commit c49d3f9 into private-attribution:main Dec 20, 2024
12 checks passed
@akoshelev akoshelev deleted the encryption-mode branch December 20, 2024 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants