[node] Add StoreBlobs endpoint for minibatch streaming
#671
Conversation
ian-shim
force-pushed
the
node-store-blobs
branch
from
1666802
to
f71f1f1
Compare
ian-shim
force-pushed
the
node-store-blobs
branch
from
f71f1f1
to
3a6b2ee
Compare
| start := time.Now() | ||
|
|
||
| // Validate the request | ||
| if in.GetReferenceBlockNumber() == 0 { |
There was a problem hiding this comment.
Should we put some of this in a validation function?
There was a problem hiding this comment.
Can we generalize the validateStoreChunksRequest and reuse in both places?
There was a problem hiding this comment.
If this method is getting too long, I'd prefer creating another method validateStoreBlobsRequest instead of generalizing validateStoreChunksRequest and using it in both places.
After the migration, StoreChunks method will be deprecated. I think it's better to make two codepaths somewhat independent and clear for each use case, so that eventually when one codepath is removed, what's left is clear for those without context.
node/store.go
Outdated
|
|
||
| numDeleted, err = s.deleteNBatches(currentTimeUnixSec, numBatchesToDeleteAtomically) | ||
| if err != nil { | ||
| return numBatchesDeleted, numBlobsDeleted, err | ||
| } | ||
| // When there is no error and we didn't delete any batch, it means we have | ||
| // no obsolete batches to delete, so we can return. | ||
| if numDeleted == 0 { |
There was a problem hiding this comment.
This seems like it will return if there are no more batches to delete even if there are still blobs to delete.
There was a problem hiding this comment.
Thanks for catching this!
ian-shim
force-pushed
the
node-store-blobs
branch
2 times, most recently
from
6bac74c
to
aa3a31a
Compare
ian-shim
force-pushed
the
node-store-blobs
branch
from
aa3a31a
to
3d98e4f
Compare
| rawChunks := make(map[core.QuorumID][][]byte) | ||
| for i, bundle := range rawBlob.GetBundles() { | ||
| quorumID := uint8(rawBlob.GetHeader().GetQuorumHeaders()[i].GetQuorumId()) | ||
| if format == core.GnarkBundleEncodingFormat { |
There was a problem hiding this comment.
It seems unideal that we are processing the chunk format both here at when we process the raw blobs in GetBlobMessages of utils.go. I'd prefer it if GetBlobMessages or some other utility function did all of this processing and then returned all formats of the chunks needed by validation and storage, so that the storage functions can be format agnostic.
There was a problem hiding this comment.
Agree it's a bit ugly. I was mostly leaving the implementation same as StoreBatch to minimize risk
On the bright side, this branching based on encoding format will also go away soon when all blobs are encoded with gnark
ian-shim
force-pushed
the
node-store-blobs
branch
from
3d98e4f
to
cae16b2
Compare
Why are these changes needed?
This PR adds
StoreBlobsendpoint to DA node.This method can be used to disperse blobs and have signatures for each blob returned, instead of a batch containing those blobs.
Checks