Use serialized bytes for commitments proto #851
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ian-shim
commented
Oct 31, 2024
| @@ -9,24 +9,13 @@ message G1Commitment { | |||
| bytes y = 2; | |||
| } | |||
|
|
|||
| message G2Commitment { | |||
There was a problem hiding this comment.
shouldn't be used other than in BlobCommitment in this file, which isn't in production use
| // The A1 element of the Y coordinate of G2 point. | ||
| bytes y_a1 = 4; | ||
| } | ||
|
|
||
| // BlobCommitment represents commitment of a specific blob, containing its | ||
| // KZG commitment, degree proof, the actual degree, and data length in number of symbols. | ||
| message BlobCommitment { |
There was a problem hiding this comment.
only used in v2 protos, not in production use
6 tasks
ian-shim
commented
Oct 31, 2024
| @@ -7,7 +7,8 @@ service Encoder { | |||
| rpc EncodeBlob(EncodeBlobRequest) returns (EncodeBlobReply) {} | |||
| } | |||
|
|
|||
| // BlomCommitments contains the blob's commitment, degree proof, and the actual degree | |||
| // BlobCommitments contains the blob's commitment, degree proof, and the actual degree | |||
| // DEPRECATED: use common.BlobCommitment instead | |||
There was a problem hiding this comment.
Copying this existing scheme in common instead of moving because this is currently being used by encoders
There was a problem hiding this comment.
Once we are fully on v2 we can remove this?
dmanc
approved these changes
Oct 31, 2024
| G2Commitment length_commitment = 2; | ||
| G2Commitment length_proof = 3; | ||
| uint32 data_length = 4; | ||
| bytes commitment = 1; |
There was a problem hiding this comment.
nit: can we make sure the spacing is consistent in the file
| @@ -7,7 +7,8 @@ service Encoder { | |||
| rpc EncodeBlob(EncodeBlobRequest) returns (EncodeBlobReply) {} | |||
| } | |||
|
|
|||
| // BlomCommitments contains the blob's commitment, degree proof, and the actual degree | |||
| // BlobCommitments contains the blob's commitment, degree proof, and the actual degree | |||
| // DEPRECATED: use common.BlobCommitment instead | |||
There was a problem hiding this comment.
Once we are fully on v2 we can remove this?
ian-shim
force-pushed
the
commitments-proto-serialized-bytes
branch
from
cc0f8b2 to
9856181
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why are these changes needed?
Instead of specifying each field of G1/G2 commitment, use serialized bytes to represent them like it does for encoding today.
Checks