IPA uses srs

[arijitdutta67]

Description

This PR enables the IPA scheme to use kzg commitment key. This helps the scheme to use kzg srs elements as the set of generators. It also enables the scheme to use a single pippenger_runtime_state for performing all multi-scalar-multiplications. In particular, it resolves the issues #163 , #164.

Checklist:

• I have reviewed my diff in github, line by line.
• Every change is related to the PR description.
• I have linked this pull request to the issue(s) that it resolves.
• There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
• There are no circuit changes, OR specifications in /markdown/specs have been updated.
• There are no circuit changes, OR a cryptographer has been assigned for review.
• I've updated any terraform that needs updating (e.g. environment variables) for deployment.
• The branch has been rebased against the head of its merge target.
• I'm happy for the PR to be merged at the reviewer's next convenience.
• New functions, classes, etc. have been documented according to the doxygen comment format. Classes and structs must have @brief describing the intended functionality.
• If existing code has been modified, such documentation has been added or updated.

[codygunton]

Regarding your points:

I had to make srs and pippenger_runtime_state public inside commitment_key.hpp to use them inside IPA. Would like to have a discussion if that is concerning.

I left a suggestion for this in my review.

The srs is not generic. For now it seems ok to me as we are not going to use the grumpkin srs immediately. But I wish to know if some improvement on this is possible with minimal changes.

The aforementioned suggestion should also handle this. The commitment key is a flexible parameter that can contain/point to whatever sort of SRS you like.

I have kept the commit function inside IPA unlike other pcs as it uses pippenger_without_endomorphism_basis_points method instead of pippengen_unsafe method.

I would prefer that we keep the same structure as in honk/pcs/kzg, unless there's a good reason for departing from what's there. I think your point is that we are somehow not obligated to insulate the user from a call to an unsafe Pippenger method? This to me doesn't seem like a good reason to use different interfaces.

[arijitdutta67]

Regarding your points:

I had to make srs and pippenger_runtime_state public inside commitment_key.hpp to use them inside IPA. Would like to have a discussion if that is concerning.

I left a suggestion for this in my review.

The srs is not generic. For now it seems ok to me as we are not going to use the grumpkin srs immediately. But I wish to know if some improvement on this is possible with minimal changes.

The aforementioned suggestion should also handle this. The commitment key is a flexible parameter that can contain/point to whatever sort of SRS you like.

I have kept the commit function inside IPA unlike other pcs as it uses pippenger_without_endomorphism_basis_points method instead of pippengen_unsafe method.

I would prefer that we keep the same structure as in honk/pcs/kzg, unless there's a good reason for departing from what's there. I think your point is that we are somehow not obligated to insulate the user from a call to an unsafe Pippenger method? This to me doesn't seem like a good reason to use different interfaces.

Thanks Cody for you timely suggestions which helped me a lot. I have tried my best to adhere to your suggestions. Please let me know if you have any more concerns.

[codygunton]

You raised the following issues in our review call:

• You were unsure whether it is secure to use unsafe Pippenger methods here.
• You were concerned that it may be inefficient to use the same Pippenger runtime state for different sets of generators and different MSM lengths.

Will you briefly look into these questions, if you haven't, and make github issues as appropriate?

[arijitdutta67]

You raised the following issues in our review call:

• You were unsure whether it is secure to use unsafe Pippenger methods here.
• You were concerned that it may be inefficient to use the same Pippenger runtime state for different sets of generators and different MSM lengths.

Will you briefly look into these questions, if you haven't, and make github issues as appropriate?

Thanks. Created the issue #237 for this.

I have addressed all of Cody's suggestions and we decided to merge this PR in our review call.