update to new mp spdz version #19

grafjo · 2022-03-15T15:36:42Z

codecov · 2022-03-18T17:13:24Z

Codecov Report

Merging #19 (61eaae3) into master (b59cd0a) will increase coverage by 0.14%.
The diff coverage is 75.00%.

@@             Coverage Diff              @@
##             master      #19      +/-   ##
============================================
+ Coverage     83.48%   83.63%   +0.14%     
- Complexity      385      388       +3     
============================================
  Files            69       69              
  Lines          1447     1454       +7     
  Branches         73       74       +1     
============================================
+ Hits           1208     1216       +8     
+ Misses          189      188       -1     
  Partials         50       50

Impacted Files	Coverage Δ
...estack/cli/client/castor/util/TupleFileParser.java	`68.42% <75.00%> (+26.75%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b59cd0a...61eaae3. Read the comment docs.

grafjo · 2022-04-21T09:59:22Z

@sbckr have rebased to current workflow files

src/main/java/io/carbynestack/cli/client/castor/util/TupleFileParser.java

sbckr · 2022-04-21T11:18:56Z

src/main/java/io/carbynestack/cli/client/castor/util/TupleFileParser.java

@@ -35,12 +37,36 @@ public static TupleChunk parse(TupleType tupleType, File tupleFile, UUID chunkId

    byte[] bytes;
    try (FileInputStream fileInputStream = new FileInputStream(tupleFile)) {
-      bytes = toByteArray(fileInputStream);
+      final byte[] fileBytes = toByteArray(fileInputStream);
+      bytes = removeTupleHeader(fileBytes);


This adds support for Tuple files generated with MP-SPDZ v0.2.8 or later, but is thereafter no longer compatible with tuple files without headers from earlier versions. For me this seems acceptable, but should definitely be mentioned explicitly.
@strieflin other opinions?

@sbckr where is the right place for that - in release notes or here in code?

I'd propose release notes and readme

src/test/java/io/carbynestack/cli/client/castor/util/TupleFileParserTest.java

src/main/java/io/carbynestack/cli/client/castor/util/TupleFileParser.java

sbckr · 2022-04-21T12:10:37Z

@grafjo THank you for rebasing the PR.

I've added a few comments you might want to look at.

In addition, this PR requires carbynestack/ephemeral#15 to be merged first, doesn't it?

grafjo · 2022-04-21T16:05:13Z

yes carbynestack/ephemeral#15 is required for this PR

sbckr

Please have a look at the provided comments and Codacy findings

src/test/java/io/carbynestack/cli/client/castor/util/TupleFileParserTest.java

src/main/java/io/carbynestack/cli/client/castor/util/TupleFileParser.java

kindlich · 2022-05-09T10:29:12Z

Are there any open tasks for this issue?
For the release notes, how do we add things there, or is that something you need to do?

sbckr · 2022-05-09T12:41:06Z

PR was blocked by carbynestack/ephemeral#15

@kindlich Release changelog is derived from commit message headers. As single features are directly merged into master, the change log only contains a single entry for now.
Usually the PR title is used as a basis for commit title. Please let me know if this should be changed

kindlich · 2022-05-09T13:27:56Z

PR was blocked by carbynestack/ephemeral#15

@kindlich Release changelog is derived from commit message headers. As single features are directly merged into master, the change log only contains a single entry for now. Usually the PR title is used as a basis for commit title. Please let me know if this should be changed

Context was that you asked us to put information about the incompatibility with the old pregenerated tuple files into the changelog.
#19 (comment)

What should we name the PR then?

kindlich · 2022-05-11T07:20:45Z

Added a Notice to the README and updated the version.
This will now need this PR to be merged beforehand, so that the links work

Create Tuple archives carbynestack#37

strieflin · 2022-05-11T09:14:24Z

Also requires an update to the tutorial on the website (see carbynestack/carbynestack.github.io#50).

kindlich · 2022-05-16T07:34:59Z

So, is there still something to do for this PR?
The update on the website is now added as draft PR, but it needs a built artifact of the CLI first.

kindlich · 2022-05-16T11:11:28Z

Rebased

Removes the File Header that was introduced in MP-SPDZ v0.2.8 Co-authored-by: Petra Scherer <[email protected]> Co-authored-by: Timo Klenk <[email protected]> Signed-off-by: Johannes Graf <[email protected]> Signed-off-by: Timo Klenk <[email protected]>

Signed-off-by: Johannes Graf <[email protected]> Signed-off-by: Timo Klenk <[email protected]>

Signed-off-by: Timo Klenk <[email protected]>

sbckr

LGTM

sbckr · 2022-05-17T07:03:08Z

as project code test coverage remains >80% and complexity is low, the failed quality check for delta test coverage (75%<80%) will be ignored

grafjo force-pushed the feature/upstream-new-mp-spdz branch from 2500a94 to b5663fc Compare March 15, 2022 15:38

grafjo mentioned this pull request Mar 15, 2022

update to new mp spdz version carbynestack/ephemeral#15

Merged

grafjo force-pushed the feature/upstream-new-mp-spdz branch from b5663fc to 1d4f818 Compare March 17, 2022 09:28

grafjo force-pushed the feature/upstream-new-mp-spdz branch from 1d4f818 to 486a9b9 Compare April 21, 2022 09:58

sbckr reviewed Apr 21, 2022

View reviewed changes

src/main/java/io/carbynestack/cli/client/castor/util/TupleFileParser.java Outdated Show resolved Hide resolved

sbckr reviewed Apr 21, 2022

View reviewed changes

src/test/java/io/carbynestack/cli/client/castor/util/TupleFileParserTest.java Outdated Show resolved Hide resolved

sbckr reviewed Apr 21, 2022

View reviewed changes

src/main/java/io/carbynestack/cli/client/castor/util/TupleFileParser.java Outdated Show resolved Hide resolved

strieflin mentioned this pull request Apr 25, 2022

Ensure that / modify tutorial code to work with updated MP-SPDZ version carbynestack/carbynestack.github.io#50

Closed

grafjo force-pushed the feature/upstream-new-mp-spdz branch from 486a9b9 to 878cbf5 Compare April 26, 2022 09:02

sbckr requested changes Apr 26, 2022

View reviewed changes

grafjo force-pushed the feature/upstream-new-mp-spdz branch from 878cbf5 to f39164e Compare April 26, 2022 11:19

kindlich mentioned this pull request May 9, 2022

Provide Pregenerated Tuple files for new MP-SPDZ Version carbynestack/carbynestack#36

Closed

kindlich mentioned this pull request May 16, 2022

Feature/upstream new mp spdz carbynestack/carbynestack.github.io#63

Merged

kindlich force-pushed the feature/upstream-new-mp-spdz branch from 586624a to b4e59fd Compare May 16, 2022 11:11

kindlich and others added 3 commits May 16, 2022 13:12

update NOTICE.md

9bcaa4c

Signed-off-by: Johannes Graf <[email protected]> Signed-off-by: Timo Klenk <[email protected]>

Add information on Incompatibility with old Tuple Files

61eaae3

Signed-off-by: Timo Klenk <[email protected]>

kindlich force-pushed the feature/upstream-new-mp-spdz branch from b4e59fd to 61eaae3 Compare May 16, 2022 11:12

sbckr approved these changes May 17, 2022

View reviewed changes

sbckr merged commit a4260ab into carbynestack:master May 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update to new mp spdz version #19

update to new mp spdz version #19

grafjo commented Mar 15, 2022

codecov bot commented Mar 18, 2022 •

edited

Loading

grafjo commented Apr 21, 2022

sbckr Apr 21, 2022

grafjo Apr 26, 2022

sbckr Apr 26, 2022

sbckr commented Apr 21, 2022

grafjo commented Apr 21, 2022

sbckr left a comment

kindlich commented May 9, 2022

sbckr commented May 9, 2022 •

edited

Loading

kindlich commented May 9, 2022

kindlich commented May 11, 2022 •

edited

Loading

strieflin commented May 11, 2022

kindlich commented May 16, 2022

kindlich commented May 16, 2022

sbckr left a comment

sbckr commented May 17, 2022

update to new mp spdz version #19

update to new mp spdz version #19

Conversation

grafjo commented Mar 15, 2022

codecov bot commented Mar 18, 2022 • edited Loading

Codecov Report

grafjo commented Apr 21, 2022

sbckr Apr 21, 2022

Choose a reason for hiding this comment

grafjo Apr 26, 2022

Choose a reason for hiding this comment

sbckr Apr 26, 2022

Choose a reason for hiding this comment

sbckr commented Apr 21, 2022

grafjo commented Apr 21, 2022

sbckr left a comment

Choose a reason for hiding this comment

kindlich commented May 9, 2022

sbckr commented May 9, 2022 • edited Loading

kindlich commented May 9, 2022

kindlich commented May 11, 2022 • edited Loading

strieflin commented May 11, 2022

kindlich commented May 16, 2022

kindlich commented May 16, 2022

sbckr left a comment

Choose a reason for hiding this comment

sbckr commented May 17, 2022

codecov bot commented Mar 18, 2022 •

edited

Loading

sbckr commented May 9, 2022 •

edited

Loading

kindlich commented May 11, 2022 •

edited

Loading