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

Fix: Send only blobs relevant to operators #301

Merged

Conversation

ian-shim
Copy link
Contributor

@ian-shim ian-shim commented Mar 1, 2024

Why are these changes needed?

For a given encoded blob, it keeps track of blob messages for given operator. If an operator is not part of a quorum and if a blob is dispersed to a quorum the operator is not part of, the encoded blob should not include the operator's blob message, but the blob header should be the same and sent to the operators even if the operator is not part of the quorum the blob is getting dispersed.
This PR makes BlobHeader field in EncodedBlob common while having assigning different bundles for different operators. Also, it sends the blob headers to all operators (makes all operators store the blob headers). This is to allow operators to properly validate the batch root for all blob headers. If an operator isn't part of the quorum the blob is dispersed to, it will receive an empty bundle.

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 requested review from mooselumph and jianoaix March 1, 2024 23:29
@ian-shim ian-shim marked this pull request as ready for review March 1, 2024 23:29
QuorumThreshold: uint32(header.QuorumThreshold),
Ratelimit: header.QuorumRate,
for _, header := range blob.BlobHeader.QuorumInfos {
if _, ok := blob.Bundles[header.QuorumID]; ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shoudn't this always be ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The way it's implemented today, blob.BlobHeader is the same for all blobs sent to all operators. So, when an operator is only registered in quorum 1 and blob is dispersed to quorums 0 and 1, blob.BlobHeader.QuorumInfos will contain both quorums.
However, blob.Bundles is operator specific, so for operator only in quorum 1, blob.Bundles[0] does not exist.

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.

It makes sense to me. Good catch. Just one question.

@ian-shim ian-shim requested a review from mooselumph March 2, 2024 01:49
@@ -138,7 +138,10 @@ type BatchHeader struct {
}

// EncodedBlob contains the messages to be sent to a group of DA nodes corresponding to a single blob
type EncodedBlob = map[OperatorID]*BlobMessage
type EncodedBlob 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.

makes BlobHeader common across all operators

bundles[i] = &node.Bundle{
Chunks: data[quorum],
}
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sure len(quorumHeaders) == len(bundles)
Bundles will be empty for quorums the operator is not part of

@ian-shim ian-shim force-pushed the multiquorum-send-only-relevant-blobs branch from c11458e to 782cf91 Compare March 2, 2024 02:00
@ian-shim ian-shim force-pushed the multiquorum-send-only-relevant-blobs branch from 782cf91 to 77f2bb4 Compare March 2, 2024 02:07
@@ -153,28 +156,35 @@ func getBlobMessage(blob *core.BlobMessage) (*node.Blob, error) {
lengthProofData.YA1 = blob.BlobHeader.LengthProof.Y.A1.Marshal()
}

quorumHeaders := make([]*node.BlobQuorumInfo, len(blob.BlobHeader.QuorumInfos))
quorumHeaders := make([]*node.BlobQuorumInfo, 0)
Copy link
Collaborator

@mooselumph mooselumph Mar 4, 2024

Choose a reason for hiding this comment

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

Would it better just to revert the changes here through L168?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

@mooselumph
Copy link
Collaborator

looks good!

@ian-shim ian-shim merged commit 780e1be into Layr-Labs:master Mar 4, 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