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

Refactor/breakup pedersen #383

Closed
wants to merge 5 commits into from
Closed

Conversation

Tabaie
Copy link
Contributor

@Tabaie Tabaie commented Apr 20, 2023

Similar to #378 but for Pedersen commitments instead of KZG. When commitments are used, the verifier needs to check a proof of knowledge of commitment. However, only two G2 elements are required for this, and we should thus avoid storing a long slice of G1 elements inside the verifying key.

@Tabaie Tabaie requested a review from gbotrel April 20, 2023 03:10
@@ -209,15 +209,17 @@ func (z *E12) DecompressKarabina(x *E12) *E12 {
var one E2
one.SetOne()

// g3 == 0
if x.C1.B2.IsZero() {
if x.C1.B2.IsZero() /* g3 == 0 */ {
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you measure these changes on e6.go ? That may affect pairing perf --

Also not sure why this is in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I must have merged in something from an experimental branch. Will try to "unbase".

}

func randomOnG2() (curve.G2Affine, error) { // TODO: Add to G2.go?
if gBytes, err := randomFrSizedBytes(); err == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is me nit picking but, the usual pattern we see in go code bases is
if err != nil {return error} --> (else) return result.
(and not the opposite :p )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought since I was using both conditions it would be nice to consider the full half of the glass first for a change. 😁

return enc.BytesWritten(), err
}

for i := range pk.basis {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can encode the slice directly (ie pass the slice as an argument to Encode) rather than a for loop on each element.

Same for decode.

return {{.CurvePackage}}.G1Affine{}, err
func randomOnG1() (curve.G1Affine, error) { // TODO: Add to G1.go?
if gBytes, err := randomFrSizedBytes(); err == nil {
return curve.HashToG1(gBytes, []byte("random on g1"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

same remark; usual pattern is "if error -> return error"

@Tabaie
Copy link
Contributor Author

Tabaie commented Apr 20, 2023

See #384 instead

@gbotrel gbotrel deleted the refactor/breakup-pedersen branch August 21, 2023 14:49
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.

2 participants