-
Notifications
You must be signed in to change notification settings - Fork 186
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
[7/N] Chunk encoding optimization: Node support for mixed encoding #654
Changes from all commits
b7c075c
b6cfb6b
6c199ce
0be5e3d
7978389
7378574
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,12 +59,16 @@ func GetBlobMessages(in *pb.StoreChunksRequest, numWorkers int) ([]*core.BlobMes | |
for j, bundle := range blob.GetBundles() { | ||
quorumID := blob.GetHeader().GetQuorumHeaders()[j].GetQuorumId() | ||
if format == core.GnarkBundleEncodingFormat { | ||
bundleMsg, err := new(core.Bundle).Deserialize(bundle.GetBundle()) | ||
if err != nil { | ||
resultChan <- err | ||
return | ||
if len(bundle.GetBundle()) > 0 { | ||
bundleMsg, err := new(core.Bundle).Deserialize(bundle.GetBundle()) | ||
if err != nil { | ||
resultChan <- err | ||
return | ||
} | ||
bundles[uint8(quorumID)] = bundleMsg | ||
} else { | ||
bundles[uint8(quorumID)] = make([]*encoding.Frame, 0) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch. I think it is handling the case, when there is quorum which the operator does not belong to. But I thought you already catch it here, https://github.com/Layr-Labs/eigenda/blob/master/disperser/batcher/grpc/dispatcher.go#L376 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the question is if the grpc would return a nil, or empty array. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the operator is not in this quorum, then the bundle is an empty []byte (not nil), so len(bundle.GetBundle()) is 0 |
||
} | ||
bundles[uint8(quorumID)] = bundleMsg | ||
} else if format == core.GobBundleEncodingFormat { | ||
bundles[uint8(quorumID)] = make([]*encoding.Frame, len(bundle.GetChunks())) | ||
for k, data := range bundle.GetChunks() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there would be a case when gnark blob is enabled but, the client forgot to set the flag. why not make two interfaces altogether, and remove the flag?
If this happens, the system might be able to store data, but not be able to retrieve. It would be a safety violation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a separate interface (from
RetrieveChunks
) will still have the issue of "old-new" communication: if a new client talks to an old server (which doesn't have the new API), it will fail.