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

[disperser] Return commitment as a struct #223

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

ian-shim
Copy link
Contributor

@ian-shim ian-shim commented Jan 30, 2024

Why are these changes needed?

In the external API, the commitment in blob header was being returned as serialized bytes. This serialization is done using glob library which is part of go's standard library. For those using other languages, it would be difficult to deserialize these bytes without glob library implemented in that language.

  • This PR updates the API so that it structures commitment as x and y coordinates of the commitment.
  • This PR also simplifies core data struct Commitment, LengthCommitment, and LengthProof

Note: this is a breaking change for rollups. While this kind of backward incompatible change in the API isn't ideal, this is added as an exception as we want to keep the API as close to what will be shipped on production (mainnet), and not many rollups have been integrated using the existing API yet.

Checks

  • I've made sure the lint is passing in this PR.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

@ian-shim ian-shim force-pushed the return-deserialized-commitment branch from c28ab33 to 19ef534 Compare January 30, 2024 22:43
@ian-shim ian-shim marked this pull request as ready for review January 30, 2024 23:25
@ian-shim ian-shim force-pushed the return-deserialized-commitment branch from 19ef534 to 7929d22 Compare February 2, 2024 05:01
@ian-shim ian-shim changed the title [disperser] Return deserialized commitment [disperser] Return commitment as a struct Feb 2, 2024
@ian-shim ian-shim requested a review from bxue-l2 February 2, 2024 05:02
@ian-shim ian-shim force-pushed the return-deserialized-commitment branch 3 times, most recently from 894ba5f to f38cf8a Compare February 5, 2024 06:03
@@ -179,6 +179,13 @@ enum BlobStatus {
INSUFFICIENT_SIGNATURES = 5;
}

message Commitment {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Disperser API change

@@ -105,15 +105,33 @@ message Bundle {
repeated bytes chunks = 1;
}

message G1Commitment {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Node API change

@@ -121,10 +121,10 @@ func (b *BlobHeader) EncodedSizeAllQuorums() int64 {

// BlomCommitments contains the blob's commitment, degree proof, and the actual degree.
type BlobCommitments struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BlobCommitments data structure change

type Commitment struct {
*bn254.G1Point
}
type G1Commitment bn254.G1Point
Copy link
Contributor Author

Choose a reason for hiding this comment

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

G1Commitment & G2Commitment are typed as G1Point and G2Point.
They're not being type aliased so that we can add instance methods such as Serialize and Deserialize.

@ian-shim ian-shim force-pushed the return-deserialized-commitment branch 2 times, most recently from d7ed800 to e449d6c Compare February 5, 2024 06:32
Copy link
Collaborator

@mooselumph mooselumph left a comment

Choose a reason for hiding this comment

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

looks good!

disperser/apiserver/server.go Outdated Show resolved Hide resolved
api/proto/disperser/disperser.proto Outdated Show resolved Hide resolved
api/proto/node/node.proto Outdated Show resolved Hide resolved
api/proto/node/node.proto Outdated Show resolved Hide resolved
@ian-shim ian-shim force-pushed the return-deserialized-commitment branch from e449d6c to 16230e2 Compare February 5, 2024 20:17
@ian-shim ian-shim force-pushed the return-deserialized-commitment branch from 16230e2 to b319aaf Compare February 5, 2024 20:24
@ian-shim ian-shim merged commit 50b12a7 into Layr-Labs:master Feb 5, 2024
4 checks passed
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