radek's review comments to make things more clear

prysmaticlabs · Jan 18, 2024 · b813cde · b813cde
1 parent 98cdaf9
commit b813cde
Show file tree

Hide file tree

Showing 6 changed files with 63 additions and 64 deletions.
diff --git a/beacon-chain/rpc/eth/beacon/handlers.go b/beacon-chain/rpc/eth/beacon/handlers.go
@@ -62,7 +62,7 @@ func (s *Server) GetBlock(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if httputil.SszRequested(r) {
+	if httputil.RespondWithSsz(r) {
 		s.getBlockSSZ(ctx, w, blk)
 	} else {
 		s.getBlock(ctx, w, blk)
@@ -105,7 +105,7 @@ func (s *Server) GetBlockV2(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if httputil.SszRequested(r) {
+	if httputil.RespondWithSsz(r) {
 		s.getBlockSSZV2(ctx, w, blk)
 	} else {
 		s.getBlockV2(ctx, w, blk)
@@ -205,7 +205,7 @@ func (s *Server) GetBlindedBlock(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if httputil.SszRequested(r) {
+	if httputil.RespondWithSsz(r) {
 		s.getBlindedBlockSSZ(ctx, w, blk)
 	} else {
 		s.getBlindedBlock(ctx, w, blk)
@@ -953,8 +953,7 @@ func (s *Server) PublishBlindedBlock(w http.ResponseWriter, r *http.Request) {
 	if shared.IsSyncing(r.Context(), w, s.SyncChecker, s.HeadFetcher, s.TimeFetcher, s.OptimisticModeFetcher) {
 		return
 	}
-	isSSZ := httputil.SszRequested(r)
-	if isSSZ {
+	if httputil.IsRequestSsz(r) {
 		s.publishBlindedBlockSSZ(ctx, w, r, false)
 	} else {
 		s.publishBlindedBlock(ctx, w, r, false)
@@ -978,8 +977,7 @@ func (s *Server) PublishBlindedBlockV2(w http.ResponseWriter, r *http.Request) {
 	if shared.IsSyncing(r.Context(), w, s.SyncChecker, s.HeadFetcher, s.TimeFetcher, s.OptimisticModeFetcher) {
 		return
 	}
-	isSSZ := httputil.SszRequested(r)
-	if isSSZ {
+	if httputil.IsRequestSsz(r) {
 		s.publishBlindedBlockSSZ(ctx, w, r, true)
 	} else {
 		s.publishBlindedBlock(ctx, w, r, true)
@@ -1250,8 +1248,7 @@ func (s *Server) PublishBlock(w http.ResponseWriter, r *http.Request) {
 	if shared.IsSyncing(r.Context(), w, s.SyncChecker, s.HeadFetcher, s.TimeFetcher, s.OptimisticModeFetcher) {
 		return
 	}
-	isSSZ := httputil.SszRequested(r)
-	if isSSZ {
+	if httputil.IsRequestSsz(r) {
 		s.publishBlockSSZ(ctx, w, r, false)
 	} else {
 		s.publishBlock(ctx, w, r, false)
@@ -1273,8 +1270,7 @@ func (s *Server) PublishBlockV2(w http.ResponseWriter, r *http.Request) {
 	if shared.IsSyncing(r.Context(), w, s.SyncChecker, s.HeadFetcher, s.TimeFetcher, s.OptimisticModeFetcher) {
 		return
 	}
-	isSSZ := httputil.SszRequested(r)
-	if isSSZ {
+	if httputil.IsRequestSsz(r) {
 		s.publishBlockSSZ(ctx, w, r, true)
 	} else {
 		s.publishBlock(ctx, w, r, true)

diff --git a/beacon-chain/rpc/eth/blob/handlers.go b/beacon-chain/rpc/eth/blob/handlers.go
@@ -46,8 +46,7 @@ func (s *Server) Blobs(w http.ResponseWriter, r *http.Request) {
 	for i := range verifiedBlobs {
 		sidecars = append(sidecars, verifiedBlobs[i].BlobSidecar)
 	}
-	ssz := httputil.SszRequested(r)
-	if ssz {
+	if httputil.RespondWithSsz(r) {
 		sidecarResp := &eth.BlobSidecars{
 			Sidecars: sidecars,
 		}

diff --git a/beacon-chain/rpc/eth/debug/handlers.go b/beacon-chain/rpc/eth/debug/handlers.go
@@ -54,7 +54,7 @@ func (s *Server) GetBeaconStateV2(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if httputil.SszRequested(r) {
+	if httputil.RespondWithSsz(r) {
 		s.getBeaconStateSSZV2(ctx, w, []byte(stateId))
 	} else {
 		s.getBeaconStateV2(ctx, w, []byte(stateId))

diff --git a/beacon-chain/rpc/eth/validator/handlers_block.go b/beacon-chain/rpc/eth/validator/handlers_block.go
@@ -201,7 +201,7 @@ func (s *Server) ProduceBlockV3(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) produceBlockV3(ctx context.Context, w http.ResponseWriter, r *http.Request, v1alpha1req *eth.BlockRequest, requiredType blockType) {
-	isSSZ := httputil.SszRequested(r)
+	isSSZ := httputil.RespondWithSsz(r)
 	v1alpha1resp, err := s.V1Alpha1Server.GetBeaconBlock(ctx, v1alpha1req)
 	if err != nil {
 		httputil.HandleError(w, err.Error(), http.StatusInternalServerError)

diff --git a/network/httputil/reader.go b/network/httputil/reader.go
@@ -12,45 +12,47 @@ import (
 // match a number with optional decimals
 var priorityRegex = regexp.MustCompile(`q=(\d+(?:\.\d+)?)`)
 
-// SszRequested takes a http request and checks to see if it should be requesting a ssz response.
-func SszRequested(req *http.Request) bool {
-	if req.Method == http.MethodGet {
-		accept := req.Header.Values("Accept")
-		if len(accept) == 0 {
-			return false
+// RespondWithSsz takes a http request and checks to see if it should be requesting a ssz response.
+func RespondWithSsz(req *http.Request) bool {
+	accept := req.Header.Values("Accept")
+	if len(accept) == 0 {
+		return false
+	}
+	types := strings.Split(accept[0], ",")
+	currentType, currentPriority := "", 0.0
+	for _, t := range types {
+		values := strings.Split(t, ";")
+		name := values[0]
+		if name != api.JsonMediaType && name != api.OctetStreamMediaType {
+			continue
 		}
-		types := strings.Split(accept[0], ",")
-		currentType, currentPriority := "", 0.0
-		for _, t := range types {
-			values := strings.Split(t, ";")
-			name := values[0]
-			if name != api.JsonMediaType && name != api.OctetStreamMediaType {
-				continue
-			}
-			// no params specified
-			if len(values) == 1 {
-				priority := 1.0
-				if priority > currentPriority {
-					currentType, currentPriority = name, priority
-				}
-				continue
-			}
-			params := values[1]
-
-			match := priorityRegex.FindAllStringSubmatch(params, 1)
-			if len(match) != 1 {
-				continue
-			}
-			priority, err := strconv.ParseFloat(match[0][1], 32)
-			if err != nil {
-				return false
-			}
+		// no params specified
+		if len(values) == 1 {
+			priority := 1.0
 			if priority > currentPriority {
 				currentType, currentPriority = name, priority
 			}
+			continue
 		}
+		params := values[1]
 
-		return currentType == api.OctetStreamMediaType
+		match := priorityRegex.FindAllStringSubmatch(params, 1)
+		if len(match) != 1 {
+			continue
+		}
+		priority, err := strconv.ParseFloat(match[0][1], 32)
+		if err != nil {
+			return false
+		}
+		if priority > currentPriority {
+			currentType, currentPriority = name, priority
+		}
 	}
+
+	return currentType == api.OctetStreamMediaType
+}
+
+// IsRequestSsz checks if the request object should be interpreted as ssz
+func IsRequestSsz(req *http.Request) bool {
 	return req.Header.Get("Content-Type") == api.OctetStreamMediaType
 }
diff --git a/network/httputil/reader_test.go b/network/httputil/reader_test.go
@@ -12,110 +12,112 @@ import (
 	"github.com/prysmaticlabs/prysm/v4/testing/require"
 )
 
-func TestSSZRequested(t *testing.T) {
+func TestRespondWithSsz(t *testing.T) {
 	t.Run("ssz_requested", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{api.OctetStreamMediaType}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, true, result)
 	})
 
 	t.Run("ssz_content_type_first", func(t *testing.T) {
 		request := httptest.NewRequest("GET", "http://foo.example", nil)
 		request.Header["Accept"] = []string{fmt.Sprintf("%s,%s", api.OctetStreamMediaType, api.JsonMediaType)}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, true, result)
 	})
 
 	t.Run("ssz_content_type_preferred_1", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{fmt.Sprintf("%s;q=0.9,%s", api.JsonMediaType, api.OctetStreamMediaType)}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, true, result)
 	})
 
 	t.Run("ssz_content_type_preferred_2", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{fmt.Sprintf("%s;q=0.95,%s;q=0.9", api.OctetStreamMediaType, api.JsonMediaType)}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, true, result)
 	})
 
 	t.Run("other_content_type_preferred", func(t *testing.T) {
 		request := httptest.NewRequest("GET", "http://foo.example", nil)
 		request.Header["Accept"] = []string{fmt.Sprintf("%s,%s;q=0.9", api.JsonMediaType, api.OctetStreamMediaType)}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("other_params", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{fmt.Sprintf("%s,%s;q=0.9,otherparam=xyz", api.JsonMediaType, api.OctetStreamMediaType)}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("no_header", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("empty_header", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("empty_header_value", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{""}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("other_content_type", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{"application/other"}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("garbage", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodGet, "http://foo.example", nil)
 		request.Header["Accept"] = []string{"This is Sparta!!!"}
-		result := SszRequested(request)
+		result := RespondWithSsz(request)
 		assert.Equal(t, false, result)
 	})
+}
 
+func TestIsRequestSsz(t *testing.T) {
 	t.Run("ssz Post happy path", func(t *testing.T) {
 		var body bytes.Buffer
 		_, err := body.WriteString("something")
 		require.NoError(t, err)
 		request := httptest.NewRequest(http.MethodPost, "http://foo.example", &body)
 		request.Header["Content-Type"] = []string{api.OctetStreamMediaType}
-		result := SszRequested(request)
+		result := IsRequestSsz(request)
 		assert.Equal(t, true, result)
 	})
 
 	t.Run("ssz Post missing header", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodPost, "http://foo.example", nil)
-		result := SszRequested(request)
+		result := IsRequestSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("ssz Post wrong content type", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodPost, "http://foo.example", nil)
 		request.Header["Content-Type"] = []string{"application/other"}
-		result := SszRequested(request)
+		result := IsRequestSsz(request)
 		assert.Equal(t, false, result)
 	})
 
 	t.Run("ssz Post json content type", func(t *testing.T) {
 		request := httptest.NewRequest(http.MethodPost, "http://foo.example", nil)
 		request.Header["Content-Type"] = []string{api.JsonMediaType}
-		result := SszRequested(request)
+		result := IsRequestSsz(request)
 		assert.Equal(t, false, result)
 	})
 }