Skip to content

Commit

Permalink
FIX: removing extra container for ssz blob sidecar response (#14451)
Browse files Browse the repository at this point in the history
* removing extra container for ssz blob sidecar response

* gaz and changelog

* gaz

* fixing dependencies

* fixing test

* updating values based on feedback"
  • Loading branch information
james-prysm authored Sep 16, 2024
1 parent 7ac3c01 commit bc9c719
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 17 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
- `== nil` checks before calling `IsNil()` on interfaces to prevent panics.
- Core: Fixed slash processing causing extra hashing
- Core: Fixed extra allocations when processing slashings
- remove unneeded container in blob sidecar ssz response

### Security

Expand Down
2 changes: 1 addition & 1 deletion beacon-chain/db/filesystem/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,5 +114,5 @@ func (s *blobStorageCache) evict(key [32]byte) {
func (s *blobStorageCache) updateMetrics(delta float64) {
s.nBlobs += delta
blobDiskCount.Set(s.nBlobs)
blobDiskSize.Set(s.nBlobs * bytesPerSidecar)
blobDiskSize.Set(s.nBlobs * fieldparams.BlobSidecarSize)
}
1 change: 0 additions & 1 deletion beacon-chain/db/filesystem/pruner.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
)

const retentionBuffer primitives.Epoch = 2
const bytesPerSidecar = 131928

var (
errPruningFailures = errors.New("blobs could not be pruned for some roots")
Expand Down
4 changes: 3 additions & 1 deletion beacon-chain/rpc/eth/blob/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ go_library(
"//beacon-chain/rpc/core:go_default_library",
"//beacon-chain/rpc/lookup:go_default_library",
"//config/fieldparams:go_default_library",
"//consensus-types/blocks:go_default_library",
"//monitoring/tracing/trace:go_default_library",
"//network/httputil:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
"@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
"@com_github_pkg_errors//:go_default_library",
],
)

Expand All @@ -32,6 +33,7 @@ go_test(
"//beacon-chain/rpc/lookup:go_default_library",
"//beacon-chain/rpc/testutil:go_default_library",
"//beacon-chain/verification:go_default_library",
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//network/httputil:go_default_library",
"//proto/prysm/v1alpha1:go_default_library",
Expand Down
33 changes: 20 additions & 13 deletions beacon-chain/rpc/eth/blob/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,19 @@ import (
"strings"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/pkg/errors"
"github.com/prysmaticlabs/prysm/v5/api/server/structs"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/rpc/core"
field_params "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/consensus-types/blocks"
"github.com/prysmaticlabs/prysm/v5/monitoring/tracing/trace"
"github.com/prysmaticlabs/prysm/v5/network/httputil"
eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
)

// Blobs is an HTTP handler for Beacon API getBlobs.
func (s *Server) Blobs(w http.ResponseWriter, r *http.Request) {
ctx, span := trace.StartSpan(r.Context(), "beacon.Blobs")
defer span.End()
var sidecars []*eth.BlobSidecar

indices, err := parseIndices(r.URL)
if err != nil {
Expand Down Expand Up @@ -48,14 +48,9 @@ func (s *Server) Blobs(w http.ResponseWriter, r *http.Request) {
return
}
}
for i := range verifiedBlobs {
sidecars = append(sidecars, verifiedBlobs[i].BlobSidecar)
}

if httputil.RespondWithSsz(r) {
sidecarResp := &eth.BlobSidecars{
Sidecars: sidecars,
}
sszResp, err := sidecarResp.MarshalSSZ()
sszResp, err := buildSidecarsSSZResponse(verifiedBlobs)
if err != nil {
httputil.HandleError(w, err.Error(), http.StatusInternalServerError)
return
Expand All @@ -64,7 +59,7 @@ func (s *Server) Blobs(w http.ResponseWriter, r *http.Request) {
return
}

httputil.WriteJson(w, buildSidecarsResponse(sidecars))
httputil.WriteJson(w, buildSidecarsJsonResponse(verifiedBlobs))
}

// parseIndices filters out invalid and duplicate blob indices
Expand Down Expand Up @@ -97,9 +92,9 @@ loop:
return indices, nil
}

func buildSidecarsResponse(sidecars []*eth.BlobSidecar) *structs.SidecarsResponse {
resp := &structs.SidecarsResponse{Data: make([]*structs.Sidecar, len(sidecars))}
for i, sc := range sidecars {
func buildSidecarsJsonResponse(verifiedBlobs []*blocks.VerifiedROBlob) *structs.SidecarsResponse {
resp := &structs.SidecarsResponse{Data: make([]*structs.Sidecar, len(verifiedBlobs))}
for i, sc := range verifiedBlobs {
proofs := make([]string, len(sc.CommitmentInclusionProof))
for j := range sc.CommitmentInclusionProof {
proofs[j] = hexutil.Encode(sc.CommitmentInclusionProof[j])
Expand All @@ -115,3 +110,15 @@ func buildSidecarsResponse(sidecars []*eth.BlobSidecar) *structs.SidecarsRespons
}
return resp
}

func buildSidecarsSSZResponse(verifiedBlobs []*blocks.VerifiedROBlob) ([]byte, error) {
ssz := make([]byte, field_params.BlobSidecarSize*len(verifiedBlobs))
for i, sidecar := range verifiedBlobs {
sszrep, err := sidecar.MarshalSSZ()
if err != nil {
return nil, errors.Wrap(err, "failed to marshal sidecar ssz")
}
copy(ssz[i*field_params.BlobSidecarSize:(i+1)*field_params.BlobSidecarSize], sszrep)
}
return ssz, nil
}
28 changes: 27 additions & 1 deletion beacon-chain/rpc/eth/blob/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/prysmaticlabs/prysm/v5/beacon-chain/rpc/lookup"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/rpc/testutil"
"github.com/prysmaticlabs/prysm/v5/beacon-chain/verification"
fieldparams "github.com/prysmaticlabs/prysm/v5/config/fieldparams"
"github.com/prysmaticlabs/prysm/v5/config/params"
"github.com/prysmaticlabs/prysm/v5/network/httputil"
eth "github.com/prysmaticlabs/prysm/v5/proto/prysm/v1alpha1"
Expand Down Expand Up @@ -366,7 +367,32 @@ func TestBlobs(t *testing.T) {
}
s.Blobs(writer, request)
assert.Equal(t, http.StatusOK, writer.Code)
require.Equal(t, len(writer.Body.Bytes()), 131932)
require.Equal(t, len(writer.Body.Bytes()), fieldparams.BlobSidecarSize) // size of each sidecar
// can directly unmarshal to sidecar since there's only 1
var sidecar eth.BlobSidecar
require.NoError(t, sidecar.UnmarshalSSZ(writer.Body.Bytes()))
require.NotNil(t, sidecar.Blob)
})
t.Run("ssz multiple blobs", func(t *testing.T) {
u := "http://foo.example/finalized"
request := httptest.NewRequest("GET", u, nil)
request.Header.Add("Accept", "application/octet-stream")
writer := httptest.NewRecorder()
writer.Body = &bytes.Buffer{}
blocker := &lookup.BeaconDbBlocker{
ChainInfoFetcher: &mockChain.ChainService{FinalizedCheckPoint: &eth.Checkpoint{Root: blockRoot[:]}},
GenesisTimeFetcher: &testutil.MockGenesisTimeFetcher{
Genesis: time.Now(),
},
BeaconDB: db,
BlobStorage: bs,
}
s := &Server{
Blocker: blocker,
}
s.Blobs(writer, request)
assert.Equal(t, http.StatusOK, writer.Code)
require.Equal(t, len(writer.Body.Bytes()), fieldparams.BlobSidecarSize*4) // size of each sidecar
})
}

Expand Down
1 change: 1 addition & 0 deletions config/fieldparams/mainnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
LogMaxBlobCommitments = 12 // Log_2 of MaxBlobCommitmentsPerBlock
BlobLength = 131072 // BlobLength defines the byte length of a blob.
BlobSize = 131072 // defined to match blob.size in bazel ssz codegen
BlobSidecarSize = 131928 // defined to match blob sidecar size in bazel ssz codegen
KzgCommitmentInclusionProofDepth = 17 // Merkle proof depth for blob_kzg_commitments list item
NextSyncCommitteeBranchDepth = 5 // NextSyncCommitteeBranchDepth defines the depth of the next sync committee branch.
PendingBalanceDepositsLimit = 134217728 // Maximum number of pending balance deposits in the beacon state.
Expand Down
1 change: 1 addition & 0 deletions config/fieldparams/minimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const (
LogMaxBlobCommitments = 4 // Log_2 of MaxBlobCommitmentsPerBlock
BlobLength = 131072 // BlobLength defines the byte length of a blob.
BlobSize = 131072 // defined to match blob.size in bazel ssz codegen
BlobSidecarSize = 131928 // defined to match blob sidecar size in bazel ssz codegen
KzgCommitmentInclusionProofDepth = 17 // Merkle proof depth for blob_kzg_commitments list item
NextSyncCommitteeBranchDepth = 5 // NextSyncCommitteeBranchDepth defines the depth of the next sync committee branch.
PendingBalanceDepositsLimit = 134217728 // Maximum number of pending balance deposits in the beacon state.
Expand Down

0 comments on commit bc9c719

Please sign in to comment.