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/break kzg srs #378

Merged
merged 21 commits into from
Apr 20, 2023
Merged

Refactor/break kzg srs #378

merged 21 commits into from
Apr 20, 2023

Conversation

Tabaie
Copy link
Contributor

@Tabaie Tabaie commented Apr 7, 2023

The KZG SRS breaks up naturally into a prover component (G1[:]) and a verifier component (G1[0], G2[0:2]), which only have one G1 point in common (which is usually standardized and can be omitted anyway).
Treating it as a single object results in very long Plonk verifying keys for example, which can be problematic in Smart Contracts.

This PR implements the break-up.

WARNING: I've removed pretty much all pass-by-pointers I encountered. In case some of that was performance-critical, lmk and I'll add it back in.

// encode the ProvingKey
enc := {{ .CurvePackage }}.NewEncoder(w)

toEncode := []interface{}{
Copy link
Collaborator

Choose a reason for hiding this comment

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

no need for slice and for loop, just enc.Encode(pk.G1)

return enc.BytesWritten(), nil
}

// WriteRawTo writes binary encoding of Proof to w without point compression
Copy link
Collaborator

Choose a reason for hiding this comment

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

Proof -> VerifyingKey

func (srs *SRS) WriteTo(w io.Writer) (int64, error) {
// encode the SRS
// encode the VerifyingKey
Copy link
Collaborator

Choose a reason for hiding this comment

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

bad comment.

Here I would rather call srs.Pk.WriteTo, srs.Vk.WriteTo, easier to maintain for future changes.

There is a duplicate point but it's not a crazy overhead.

dec := {{ .CurvePackage }}.NewDecoder(r)

toDecode := []interface{}{
&pk.G1,
Copy link
Collaborator

Choose a reason for hiding this comment

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

just dec.Decode(&pk.G1)

&srs.G2[0],
&srs.G2[1],
&srs.G1,
&srs.Vk.G2[0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

same srs.Pk.ReadFrom... srs.Vk.ReadFrom(...)

@ThomasPiellard
Copy link
Contributor

Could you modify

        _, err := foldedQuotients.MultiExp(quotients, randomNumbers, config)
	if err != nil {
		return nil
	}

to

        _, err := foldedQuotients.MultiExp(quotients, randomNumbers, config)
 	if err != nil {
		return err
	}

in BatchVerifyMultiPoints in the kzg.go files (e.g. l.419 for bls377) ? (Spotted by @kevaundray )

@Tabaie Tabaie requested a review from gbotrel April 16, 2023 19:14
Copy link
Collaborator

@gbotrel gbotrel left a comment

Choose a reason for hiding this comment

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

minor comment on godoc format but lgtm 👍

// SRS stores the result of the MPC
type SRS struct {
// Proving and Verifying keys together constitute the SRS (result of the MPC)
type ProvingKey struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

godoc format: struct doc should be // ProvingKey ...

(else it isn't render properly in godoc)

@Tabaie Tabaie merged commit eaeb6db into develop Apr 20, 2023
@Tabaie Tabaie deleted the refactor/break-kzg-srs branch April 20, 2023 02:56
@Tabaie Tabaie mentioned this pull request Apr 20, 2023
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