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

Sum To Zero Check Protocol #1040

Merged

Conversation

danielmasny
Copy link
Collaborator

Include Sum To Zero Check Protocol since it is needed for DZKP verification.

It was originally a part of #930

It can also be used to check the consistency between shares, i.e. as a base protocol for #936

I moved Hashing to Prss, since it is similar (basic cryptographic operation). The only difference is that it is not keyed.

@danielmasny danielmasny requested a review from akoshelev May 3, 2024 23:16
Copy link

codecov bot commented May 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.16%. Comparing base (dfe14ff) to head (46bbf96).
Report is 8 commits behind head on main.

❗ Current head 46bbf96 differs from pull request most recent head e0c3317. Consider uploading reports for the commit e0c3317 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1040      +/-   ##
==========================================
+ Coverage   90.56%   91.16%   +0.60%     
==========================================
  Files         173      175       +2     
  Lines       26082    26285     +203     
==========================================
+ Hits        23622    23964     +342     
+ Misses       2460     2321     -139     

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

Copy link
Collaborator

@akoshelev akoshelev left a comment

Choose a reason for hiding this comment

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

It can also be used to check the consistency between shares, i.e. as a base protocol for #936

I can't see how it is compatible with the protocol described in that PR. This proposes having the entire input ready before validation starts, #936 can work with streams and it makes it more attractive to use because it can be just another stream combinator that is easy to plug in.

ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
/// propagates errors from send and receive
pub async fn validate_replicated_shares<C, S>(
ctx: C,
input_left: &[S],
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there a reason why you're not taking Replicated<S> instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. This check is for the verification of the proofs. I will have 3 provers and 6 verifiers. The two verifiers for each prover need to perform a zero check. You could considers this a 2 party computation where we have 2 out of 2 non-replicated shares and they want to check whether the shares sum up to zero.

We could interpret the shares as replicated secret shares. One issue might be that the shares won't be consistent so if we add an automated consistency check for replicated shares, it would fail here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right that #936 is quite different. I am not sure whether we should try to make it compatible. In my use-case, we really don't need a stream since I only need to zero check log N many elements, i.e. the dzkp proof size. But the protocol does a very similar thing, i.e. hash the shares and send them over the network for a consistency check.

ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@benjaminsavage benjaminsavage left a comment

Choose a reason for hiding this comment

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

Unless I am much mistaken, this is not a protocol to check if something "sums to zero" and it should be renamed as such.

///
/// We use a hash based approach that is secure in the random oracle model
/// further, only one of left and right helper check that it is zero
/// this is might not be sufficient in some applications to prevent malicious behavior
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't understand where the typo is

Copy link
Collaborator

Choose a reason for hiding this comment

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

extra "is"

/// this might not be sufficient in some applications to prevent malicious behavior

ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
ipa-core/src/protocol/basics/share_validation.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@benjaminsavage benjaminsavage 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 to me!

@benjaminsavage benjaminsavage merged commit 73cf9b5 into private-attribution:main May 10, 2024
9 checks passed
@danielmasny danielmasny deleted the share_verification branch May 10, 2024 18:20
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