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

Optimization for open for linear-code based PCS #128

Closed
4 tasks done
mmagician opened this issue Oct 23, 2023 · 8 comments · Fixed by #134
Closed
4 tasks done

Optimization for open for linear-code based PCS #128

mmagician opened this issue Oct 23, 2023 · 8 comments · Fixed by #134
Assignees
Labels
breaking-change Breaking change D-medium Difficulty: medium T-design Type: discuss API design and/or research T-performance Type: performance improvements

Comments

@mmagician
Copy link
Member

Summary

The current interface doesn't allow room for certain optimizations to open.

Problem Definition

Specifically, for linear code schemes such as Ligero, during commit we compute the merkle root, and to create the proof in open we again recompute the merkle tree to then send merkle proofs for a bunch of leaf indices. Ideally the open stage shouldn't need to recompute the tree.

Proposal

We introduce a gap between the output of commit and the input to check.

Namely, commit would return an ExtendedCommitment (aka PrivateCommitment?) , and verify would only receive the Committment. We also introduce a trivial into() method that's a no-op in all current schemes, but for linear codes it sheds a few fields, such as the coefficients matrix, extended coefficients matrix and the merkle tree, saving the need to recompute them.

We can then leverage this new interface to further simplify the commit and open trait methods: instead of having a separate argument rands, we place the rands on the ExtendedComittment. Then the into() method would shed the rands field when converting it to be used in check.

@Antonio95 @autquis @Pratyush


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@mmagician mmagician added D-medium Difficulty: medium T-performance Type: performance improvements T-design Type: discuss API design and/or research breaking-change Breaking change labels Oct 23, 2023
@mmagician mmagician self-assigned this Oct 23, 2023
@Antonio95
Copy link
Contributor

Yes! I would propose the name CommitmentData for the larger structure containing the commitment and auxiliary data (whether it be randomness, previously computed values, or anything else). This also makes it less likely to be thought of as an actual "commitment on steroids", which I could see happening with ExtendedCommitment.

@mmagician
Copy link
Member Author

As discussed with @autquis and @Antonio95 , an alternative approach would be to turn the rands into an aux_data structure, and have commit return a tuple of (commitment, aux_data). This has the advantage of being less likely to mis-use and accidentally pass the ExtendedCommitment to the verifier.

@Pratyush
Copy link
Member

Sounds like a good idea to have commit output the commitment and an additional "commitment state" (I presume that this is used in opening?). If so, a descriptive name would could be CommitterState?

@mmagician
Copy link
Member Author

Correct, that would be used in open.

@Pratyush
Copy link
Member

Hm then perhaps CommitmentOpeningData would be slightly better? (Because if I call commit in a sequence it's not like I'll be passing the state from one call to the next)

@mmagician
Copy link
Member Author

Sure, we can start with that and see how the name feels once it's implemented :)

@autquis autquis mentioned this issue Nov 1, 2023
6 tasks
@autquis
Copy link
Contributor

autquis commented Nov 2, 2023

Would you want all schemes in the repo to have the hiding property?

In #134 , we have added PCCommitmentState along with PCRandomness. Ideally, we should merge these two in one. One way to do so is to remove the methods from PCRandomness. This implies that we are not enforcing hiding property on the schemes of the repo. If a scheme is hiding, it implements these methods for its Randomness. However, if a scheme is not hiding, implementing these methods is unnecessary (e.g., #131 and #132).

@Pratyush
Copy link
Member

Pratyush commented Nov 6, 2023

We can merge PCCommitmentState and PCRandomness, I think, and put the methods from PCRandomness into PCCommitmentState. For schemes where we haven't implemented hiding, these methods can be no-ops.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Breaking change D-medium Difficulty: medium T-design Type: discuss API design and/or research T-performance Type: performance improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants