From 854f4bc9a3c81d3a1e313836582f4fbba14ab7fc Mon Sep 17 00:00:00 2001 From: Muzry Date: Mon, 8 Sep 2025 22:45:08 +0800 Subject: [PATCH] fix getBlockAttestationsV2 to return [] instead of null when data is empty (#15651) --- beacon-chain/rpc/eth/beacon/handlers.go | 10 +- beacon-chain/rpc/eth/beacon/handlers_test.go | 94 +++++++++++++++++++ .../muzry_fix_get_block_attestation_v2.md | 3 + 3 files changed, 102 insertions(+), 5 deletions(-) create mode 100644 changelog/muzry_fix_get_block_attestation_v2.md diff --git a/beacon-chain/rpc/eth/beacon/handlers.go b/beacon-chain/rpc/eth/beacon/handlers.go index 2ce840bf33..0959c9c7b1 100644 --- a/beacon-chain/rpc/eth/beacon/handlers.go +++ b/beacon-chain/rpc/eth/beacon/handlers.go @@ -334,26 +334,26 @@ func (s *Server) GetBlockAttestationsV2(w http.ResponseWriter, r *http.Request) consensusAtts := blk.Block().Body().Attestations() v := blk.Block().Version() - var attStructs []interface{} + attStructs := make([]interface{}, len(consensusAtts)) if v >= version.Electra { - for _, att := range consensusAtts { + for index, att := range consensusAtts { a, ok := att.(*eth.AttestationElectra) if !ok { httputil.HandleError(w, fmt.Sprintf("unable to convert consensus attestations electra of type %T", att), http.StatusInternalServerError) return } attStruct := structs.AttElectraFromConsensus(a) - attStructs = append(attStructs, attStruct) + attStructs[index] = attStruct } } else { - for _, att := range consensusAtts { + for index, att := range consensusAtts { a, ok := att.(*eth.Attestation) if !ok { httputil.HandleError(w, fmt.Sprintf("unable to convert consensus attestation of type %T", att), http.StatusInternalServerError) return } attStruct := structs.AttFromConsensus(a) - attStructs = append(attStructs, attStruct) + attStructs[index] = attStruct } } diff --git a/beacon-chain/rpc/eth/beacon/handlers_test.go b/beacon-chain/rpc/eth/beacon/handlers_test.go index 673f3b795d..c13759b8f7 100644 --- a/beacon-chain/rpc/eth/beacon/handlers_test.go +++ b/beacon-chain/rpc/eth/beacon/handlers_test.go @@ -910,6 +910,100 @@ func TestGetBlockAttestations(t *testing.T) { }) }) }) + + t.Run("empty-attestations", func(t *testing.T) { + t.Run("v1", func(t *testing.T) { + b := util.NewBeaconBlock() + b.Block.Body.Attestations = []*eth.Attestation{} // Explicitly set empty attestations + sb, err := blocks.NewSignedBeaconBlock(b) + require.NoError(t, err) + + mockChainService := &chainMock.ChainService{ + FinalizedRoots: map[[32]byte]bool{}, + } + + s := &Server{ + OptimisticModeFetcher: mockChainService, + FinalizationFetcher: mockChainService, + Blocker: &testutil.MockBlocker{BlockToReturn: sb}, + } + + request := httptest.NewRequest(http.MethodGet, "http://foo.example/eth/v1/beacon/blocks/{block_id}/attestations", nil) + request.SetPathValue("block_id", "head") + writer := httptest.NewRecorder() + writer.Body = &bytes.Buffer{} + + s.GetBlockAttestations(writer, request) + require.Equal(t, http.StatusOK, writer.Code) + resp := &structs.GetBlockAttestationsResponse{} + require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp)) + + // Ensure data is empty array, not null + require.NotNil(t, resp.Data) + assert.Equal(t, 0, len(resp.Data)) + }) + + t.Run("v2-pre-electra", func(t *testing.T) { + b := util.NewBeaconBlock() + b.Block.Body.Attestations = []*eth.Attestation{} // Explicitly set empty attestations + sb, err := blocks.NewSignedBeaconBlock(b) + require.NoError(t, err) + mockChainService := &chainMock.ChainService{ + FinalizedRoots: map[[32]byte]bool{}, + } + + s := &Server{ + OptimisticModeFetcher: mockChainService, + FinalizationFetcher: mockChainService, + Blocker: &testutil.MockBlocker{BlockToReturn: sb}, + } + + request := httptest.NewRequest(http.MethodGet, "http://foo.example/eth/v2/beacon/blocks/{block_id}/attestations", nil) + request.SetPathValue("block_id", "head") + writer := httptest.NewRecorder() + writer.Body = &bytes.Buffer{} + + s.GetBlockAttestationsV2(writer, request) + require.Equal(t, http.StatusOK, writer.Code) + resp := &structs.GetBlockAttestationsV2Response{} + require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp)) + // Ensure data is "[]", not null + require.NotNil(t, resp.Data) + assert.Equal(t, string(json.RawMessage("[]")), string(resp.Data)) + }) + + t.Run("v2-electra", func(t *testing.T) { + eb := util.NewBeaconBlockFulu() + eb.Block.Body.Attestations = []*eth.AttestationElectra{} // Explicitly set empty attestations + esb, err := blocks.NewSignedBeaconBlock(eb) + require.NoError(t, err) + + mockChainService := &chainMock.ChainService{ + FinalizedRoots: map[[32]byte]bool{}, + } + + s := &Server{ + OptimisticModeFetcher: mockChainService, + FinalizationFetcher: mockChainService, + Blocker: &testutil.MockBlocker{BlockToReturn: esb}, + } + + request := httptest.NewRequest(http.MethodGet, "http://foo.example/eth/v2/beacon/blocks/{block_id}/attestations", nil) + request.SetPathValue("block_id", "head") + writer := httptest.NewRecorder() + writer.Body = &bytes.Buffer{} + + s.GetBlockAttestationsV2(writer, request) + require.Equal(t, http.StatusOK, writer.Code) + resp := &structs.GetBlockAttestationsV2Response{} + require.NoError(t, json.Unmarshal(writer.Body.Bytes(), resp)) + + // Ensure data is "[]", not null + require.NotNil(t, resp.Data) + assert.Equal(t, string(json.RawMessage("[]")), string(resp.Data)) + assert.Equal(t, "fulu", resp.Version) + }) + }) } func TestGetBlindedBlock(t *testing.T) { diff --git a/changelog/muzry_fix_get_block_attestation_v2.md b/changelog/muzry_fix_get_block_attestation_v2.md new file mode 100644 index 0000000000..e5e2046279 --- /dev/null +++ b/changelog/muzry_fix_get_block_attestation_v2.md @@ -0,0 +1,3 @@ +### Fixed + +- Fix getBlockAttestationsV2 to return [] instead of null when data is empty