Skip to content

Commit

Permalink
Code hygiene: more properly name the chunk encoding format field (#661)
Browse files Browse the repository at this point in the history
  • Loading branch information
jianoaix authored Aug 6, 2024
1 parent 9d38331 commit 1377d37
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 185 deletions.
8 changes: 4 additions & 4 deletions api/clients/node_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,12 @@ func (c client) GetChunks(
chunks := make([]*encoding.Frame, len(reply.GetChunks()))
for i, data := range reply.GetChunks() {
var chunk *encoding.Frame
switch reply.GetEncoding() {
case node.ChunkEncoding_GNARK:
switch reply.GetChunkEncodingFormat() {
case node.ChunkEncodingFormat_GNARK:
chunk, err = new(encoding.Frame).DeserializeGnark(data)
case node.ChunkEncoding_GOB:
case node.ChunkEncodingFormat_GOB:
chunk, err = new(encoding.Frame).Deserialize(data)
case node.ChunkEncoding_UNKNOWN:
case node.ChunkEncodingFormat_UNKNOWN:
// For backward compatibility, we fallback the UNKNOWN to GOB
chunk, err = new(encoding.Frame).Deserialize(data)
if err != nil {
Expand Down
300 changes: 151 additions & 149 deletions api/grpc/node/node.pb.go

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions api/proto/node/node.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ message StoreBlobsRequest {

message StoreBlobsReply {
// The operator's BLS sgnature signed on the blob header hashes.
// The ordering of the signatures must match the ordering of the blobs sent
// The ordering of the signatures must match the ordering of the blobs sent
// in the request, with empty signatures in the places for discarded blobs.
repeated google.protobuf.BytesValue signatures = 1;
}
Expand Down Expand Up @@ -90,7 +90,7 @@ message RetrieveChunksRequest {

// This describes how the chunks returned in RetrieveChunksReply are encoded.
// Used to facilitate the decoding of chunks.
enum ChunkEncoding {
enum ChunkEncodingFormat {
UNKNOWN = 0;
GNARK = 1;
GOB = 2;
Expand All @@ -99,8 +99,8 @@ enum ChunkEncoding {
message RetrieveChunksReply {
// All chunks the Node is storing for the requested blob per RetrieveChunksRequest.
repeated bytes chunks = 1;
// How the above chunks encoded.
ChunkEncoding encoding = 2;
// How the above chunks are encoded.
ChunkEncodingFormat chunk_encoding_format = 2;
}

// See RetrieveChunksRequest for documentation of each parameter of GetBlobHeaderRequest.
Expand Down
6 changes: 3 additions & 3 deletions node/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,8 @@ func (s *Server) RetrieveChunks(ctx context.Context, in *pb.RetrieveChunksReques
s.node.Metrics.RecordRPCRequest("RetrieveChunks", "failure", time.Since(start))
return nil, fmt.Errorf("could not find chunks for batchHeaderHash %v, blob index: %v, quorumID: %v", hex.EncodeToString(batchHeaderHash[:]), in.GetBlobIndex(), in.GetQuorumId())
}
if !s.config.EnableGnarkBundleEncoding && format == pb.ChunkEncoding_GNARK {
format = pb.ChunkEncoding_GOB
if !s.config.EnableGnarkBundleEncoding && format == pb.ChunkEncodingFormat_GNARK {
format = pb.ChunkEncodingFormat_GOB
gobChunks := make([][]byte, 0, len(chunks))
for _, c := range chunks {
if len(c) == 0 {
Expand All @@ -329,7 +329,7 @@ func (s *Server) RetrieveChunks(ctx context.Context, in *pb.RetrieveChunksReques
chunks = gobChunks
}
s.node.Metrics.RecordRPCRequest("RetrieveChunks", "success", time.Since(start))
return &pb.RetrieveChunksReply{Chunks: chunks, Encoding: format}, nil
return &pb.RetrieveChunksReply{Chunks: chunks, ChunkEncodingFormat: format}, nil
}

func (s *Server) GetBlobHeader(ctx context.Context, in *pb.GetBlobHeaderRequest) (*pb.GetBlobHeaderReply, error) {
Expand Down
12 changes: 6 additions & 6 deletions node/grpc/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ func TestRetrieveChunks(t *testing.T) {
QuorumId: 0,
})
assert.NoError(t, err)
assert.Equal(t, pb.ChunkEncoding_GOB, retrievalReply.Encoding)
assert.Equal(t, pb.ChunkEncodingFormat_GOB, retrievalReply.ChunkEncodingFormat)
recovered, err := new(encoding.Frame).Deserialize(retrievalReply.GetChunks()[0])
assert.NoError(t, err)
chunk, err := new(encoding.Frame).Deserialize(encodedChunk)
Expand All @@ -439,7 +439,7 @@ func TestRetrieveChunks(t *testing.T) {
QuorumId: 1,
})
assert.NoError(t, err)
assert.Equal(t, pb.ChunkEncoding_UNKNOWN, retrievalReply.Encoding)
assert.Equal(t, pb.ChunkEncodingFormat_UNKNOWN, retrievalReply.ChunkEncodingFormat)
assert.Empty(t, retrievalReply.GetChunks())
}

Expand All @@ -463,7 +463,7 @@ func TestGnarkBundleEncoding(t *testing.T) {
QuorumId: 0,
})
assert.NoError(t, err)
assert.Equal(t, pb.ChunkEncoding_GNARK, retrievalReply.Encoding)
assert.Equal(t, pb.ChunkEncodingFormat_GNARK, retrievalReply.ChunkEncodingFormat)
recovered, err := new(encoding.Frame).DeserializeGnark(retrievalReply.GetChunks()[0])
assert.NoError(t, err)
chunk, err := new(encoding.Frame).DeserializeGnark(gnarkEncodedChunk)
Expand All @@ -476,7 +476,7 @@ func TestGnarkBundleEncoding(t *testing.T) {
QuorumId: 1,
})
assert.NoError(t, err)
assert.Equal(t, pb.ChunkEncoding_UNKNOWN, retrievalReply.Encoding)
assert.Equal(t, pb.ChunkEncodingFormat_UNKNOWN, retrievalReply.ChunkEncodingFormat)
assert.Empty(t, retrievalReply.GetChunks())
}

Expand All @@ -498,7 +498,7 @@ func TestMixedBundleEncoding(t *testing.T) {
QuorumId: 0,
})
assert.NoError(t, err)
assert.Equal(t, pb.ChunkEncoding_GOB, retrievalReply.Encoding)
assert.Equal(t, pb.ChunkEncodingFormat_GOB, retrievalReply.ChunkEncodingFormat)
recovered, err := new(encoding.Frame).Deserialize(retrievalReply.GetChunks()[0])
assert.NoError(t, err)
chunk, err := new(encoding.Frame).Deserialize(encodedChunk)
Expand All @@ -511,7 +511,7 @@ func TestMixedBundleEncoding(t *testing.T) {
QuorumId: 1,
})
assert.NoError(t, err)
assert.Equal(t, pb.ChunkEncoding_UNKNOWN, retrievalReply.Encoding)
assert.Equal(t, pb.ChunkEncodingFormat_UNKNOWN, retrievalReply.ChunkEncodingFormat)
assert.Empty(t, retrievalReply.GetChunks())
}

Expand Down
20 changes: 10 additions & 10 deletions node/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,16 +349,16 @@ func (s *Store) GetBlobHeader(ctx context.Context, batchHeaderHash [32]byte, blo

// GetChunks returns the list of byte arrays stored for given blobKey along with the encoding
// format of the bytes.
func (s *Store) GetChunks(ctx context.Context, batchHeaderHash [32]byte, blobIndex int, quorumID core.QuorumID) ([][]byte, node.ChunkEncoding, error) {
func (s *Store) GetChunks(ctx context.Context, batchHeaderHash [32]byte, blobIndex int, quorumID core.QuorumID) ([][]byte, node.ChunkEncodingFormat, error) {
log := s.logger

blobKey, err := EncodeBlobKey(batchHeaderHash, blobIndex, quorumID)
if err != nil {
return nil, node.ChunkEncoding_UNKNOWN, err
return nil, node.ChunkEncodingFormat_UNKNOWN, err
}
data, err := s.db.Get(blobKey)
if err != nil {
return nil, node.ChunkEncoding_UNKNOWN, err
return nil, node.ChunkEncodingFormat_UNKNOWN, err
}
log.Debug("Retrieved chunk", "blobKey", hexutil.Encode(blobKey), "length", len(data))

Expand Down Expand Up @@ -469,28 +469,28 @@ func parseHeader(data []byte) (core.BundleEncodingFormat, uint64, error) {

// DecodeChunks converts a flattened array of chunks into an array of its constituent chunks,
// throwing an error in case the chunks were not serialized correctly.
func DecodeChunks(data []byte) ([][]byte, node.ChunkEncoding, error) {
func DecodeChunks(data []byte) ([][]byte, node.ChunkEncodingFormat, error) {
// Empty chunk is valid, but there is nothing to decode.
if len(data) == 0 {
return [][]byte{}, node.ChunkEncoding_UNKNOWN, nil
return [][]byte{}, node.ChunkEncodingFormat_UNKNOWN, nil
}
format, _, err := parseHeader(data)
if err != nil {
return nil, node.ChunkEncoding_UNKNOWN, err
return nil, node.ChunkEncodingFormat_UNKNOWN, err
}

// Note: the encoding format IDs may not be the same as the field ID in protobuf.
// For example, GobBundleEncodingFormat is 1 but node.ChunkEncoding_GOB has proto
// For example, GobBundleEncodingFormat is 1 but node.ChunkEncodingFormat_GOB has proto
// field ID 2.
switch format {
case 0:
chunks, err := DecodeGobChunks(data)
return chunks, node.ChunkEncoding_GOB, err
return chunks, node.ChunkEncodingFormat_GOB, err
case 1:
chunks, err := DecodeGnarkChunks(data)
return chunks, node.ChunkEncoding_GNARK, err
return chunks, node.ChunkEncodingFormat_GNARK, err
default:
return nil, node.ChunkEncoding_UNKNOWN, errors.New("invalid data encoding format")
return nil, node.ChunkEncodingFormat_UNKNOWN, errors.New("invalid data encoding format")
}
}

Expand Down
18 changes: 9 additions & 9 deletions node/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func createStore(t *testing.T) *node.Store {
func TestEncodeDecodeChunks(t *testing.T) {
decoded, format, err := node.DecodeChunks([]byte{})
assert.Nil(t, err)
assert.Equal(t, pb.ChunkEncoding_UNKNOWN, format)
assert.Equal(t, pb.ChunkEncodingFormat_UNKNOWN, format)
assert.Equal(t, 0, len(decoded))

numSamples := 32
Expand All @@ -244,7 +244,7 @@ func TestEncodeDecodeChunks(t *testing.T) {
assert.Nil(t, err)
decoded, format, err := node.DecodeChunks(encoded)
assert.Nil(t, err)
assert.Equal(t, pb.ChunkEncoding_GOB, format)
assert.Equal(t, pb.ChunkEncodingFormat_GOB, format)
for i := 0; i < numChunks; i++ {
assert.True(t, bytes.Equal(decoded[i], chunks[i]))
}
Expand Down Expand Up @@ -329,18 +329,18 @@ func TestStoringBlob(t *testing.T) {
assert.False(t, s.HasKey(ctx, blobKey2))
}

func decodeChunks(t *testing.T, s *node.Store, batchHeaderHash [32]byte, blobIdx int, chunkEncoding pb.ChunkEncoding) []*encoding.Frame {
func decodeChunks(t *testing.T, s *node.Store, batchHeaderHash [32]byte, blobIdx int, chunkEncoding pb.ChunkEncodingFormat) []*encoding.Frame {
ctx := context.Background()
chunks, format, err := s.GetChunks(ctx, batchHeaderHash, blobIdx, 0)
assert.Nil(t, err)
assert.Equal(t, 1, len(chunks))
assert.Equal(t, chunkEncoding, format)
var f *encoding.Frame
switch chunkEncoding {
case pb.ChunkEncoding_GOB:
case pb.ChunkEncodingFormat_GOB:
f, err = new(encoding.Frame).Deserialize(chunks[0])
assert.Nil(t, err)
case pb.ChunkEncoding_GNARK:
case pb.ChunkEncodingFormat_GNARK:
f, err = new(encoding.Frame).DeserializeGnark(chunks[0])
assert.Nil(t, err)
}
Expand Down Expand Up @@ -375,12 +375,12 @@ func TestBundleEncodingEquivalence(t *testing.T) {
batchHeaderHash, err := batchHeader1.GetBatchHeaderHash()
assert.Nil(t, err)
// The first blob
bundle1 := decodeChunks(t, s1, batchHeaderHash, 0, pb.ChunkEncoding_GNARK)
bundle2 := decodeChunks(t, s2, batchHeaderHash, 0, pb.ChunkEncoding_GOB)
bundle1 := decodeChunks(t, s1, batchHeaderHash, 0, pb.ChunkEncodingFormat_GNARK)
bundle2 := decodeChunks(t, s2, batchHeaderHash, 0, pb.ChunkEncodingFormat_GOB)
checkBundleEquivalence(t, bundle1, bundle2)
// The second blob
bundle1 = decodeChunks(t, s1, batchHeaderHash, 1, pb.ChunkEncoding_GNARK)
bundle2 = decodeChunks(t, s2, batchHeaderHash, 1, pb.ChunkEncoding_GOB)
bundle1 = decodeChunks(t, s1, batchHeaderHash, 1, pb.ChunkEncodingFormat_GNARK)
bundle2 = decodeChunks(t, s2, batchHeaderHash, 1, pb.ChunkEncodingFormat_GOB)
checkBundleEquivalence(t, bundle1, bundle2)
}

Expand Down

0 comments on commit 1377d37

Please sign in to comment.