From bc9c7193a9dee0b8729b5c89819f368f702485ce Mon Sep 17 00:00:00 2001 From: james-prysm <90280386+james-prysm@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:28:47 -0500 Subject: [PATCH] FIX: removing extra container for ssz blob sidecar response (#14451) * removing extra container for ssz blob sidecar response * gaz and changelog * gaz * fixing dependencies * fixing test * updating values based on feedback" --- CHANGELOG.md | 1 + beacon-chain/db/filesystem/cache.go | 2 +- beacon-chain/db/filesystem/pruner.go | 1 - beacon-chain/rpc/eth/blob/BUILD.bazel | 4 ++- beacon-chain/rpc/eth/blob/handlers.go | 33 +++++++++++++--------- beacon-chain/rpc/eth/blob/handlers_test.go | 28 +++++++++++++++++- config/fieldparams/mainnet.go | 1 + config/fieldparams/minimal.go | 1 + 8 files changed, 54 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b99fcea6ee45..d71b3efb074e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/beacon-chain/db/filesystem/cache.go b/beacon-chain/db/filesystem/cache.go index 46d1f694c8f9..98223f3779d5 100644 --- a/beacon-chain/db/filesystem/cache.go +++ b/beacon-chain/db/filesystem/cache.go @@ -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) } diff --git a/beacon-chain/db/filesystem/pruner.go b/beacon-chain/db/filesystem/pruner.go index 5ac4ac50d6fa..74ca0966f2e8 100644 --- a/beacon-chain/db/filesystem/pruner.go +++ b/beacon-chain/db/filesystem/pruner.go @@ -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") diff --git a/beacon-chain/rpc/eth/blob/BUILD.bazel b/beacon-chain/rpc/eth/blob/BUILD.bazel index 396eeec6ed5d..85f48f9bbc74 100644 --- a/beacon-chain/rpc/eth/blob/BUILD.bazel +++ b/beacon-chain/rpc/eth/blob/BUILD.bazel @@ -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", ], ) @@ -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", diff --git a/beacon-chain/rpc/eth/blob/handlers.go b/beacon-chain/rpc/eth/blob/handlers.go index 0fcaef08a848..4f4635372399 100644 --- a/beacon-chain/rpc/eth/blob/handlers.go +++ b/beacon-chain/rpc/eth/blob/handlers.go @@ -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 { @@ -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 := ð.BlobSidecars{ - Sidecars: sidecars, - } - sszResp, err := sidecarResp.MarshalSSZ() + sszResp, err := buildSidecarsSSZResponse(verifiedBlobs) if err != nil { httputil.HandleError(w, err.Error(), http.StatusInternalServerError) return @@ -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 @@ -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]) @@ -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 +} diff --git a/beacon-chain/rpc/eth/blob/handlers_test.go b/beacon-chain/rpc/eth/blob/handlers_test.go index 707cc567dd9f..1e66fbc8b487 100644 --- a/beacon-chain/rpc/eth/blob/handlers_test.go +++ b/beacon-chain/rpc/eth/blob/handlers_test.go @@ -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" @@ -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: ð.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 }) } diff --git a/config/fieldparams/mainnet.go b/config/fieldparams/mainnet.go index d66549e381d1..17aeed23f8ba 100644 --- a/config/fieldparams/mainnet.go +++ b/config/fieldparams/mainnet.go @@ -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. diff --git a/config/fieldparams/minimal.go b/config/fieldparams/minimal.go index ee48143e5262..ac022e5823ea 100644 --- a/config/fieldparams/minimal.go +++ b/config/fieldparams/minimal.go @@ -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.