removing ssz-only flag ( reverting feature) and fix accept header middleware (#15433)

* removing ssz-only flag

* gaz

* reverting other uses of sszonly

* gaz

* adding kasey and radek's suggestions

* update changelog

* adding test

* radek advice with new headers and tests

* adding logs and fixing comments

* adding logs and fixing comments

* gaz

* Update validator/client/beacon-api/rest_handler_client.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/apiutil/header.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* Update api/apiutil/header.go

Co-authored-by: Radosław Kapka <rkapka@wp.pl>

* radek's comments

* adding another failing case based on radek's suggestion

* another unit test

---------

Co-authored-by: Radosław Kapka <rkapka@wp.pl>
This commit is contained in:
james-prysm
2025-07-22 11:06:51 -05:00
committed by GitHub
parent c21fae239f
commit 77958022e7
17 changed files with 394 additions and 152 deletions

View File

@@ -46,7 +46,6 @@ go_library(
"//api/server/structs:go_default_library",
"//beacon-chain/core/helpers:go_default_library",
"//beacon-chain/core/signing:go_default_library",
"//config/features:go_default_library",
"//config/fieldparams:go_default_library",
"//config/params:go_default_library",
"//consensus-types/primitives:go_default_library",
@@ -128,7 +127,6 @@ go_test(
"//api/server/structs:go_default_library",
"//beacon-chain/core/helpers:go_default_library",
"//beacon-chain/rpc/eth/shared/testing:go_default_library",
"//config/features:go_default_library",
"//config/params:go_default_library",
"//consensus-types/primitives:go_default_library",
"//consensus-types/validator:go_default_library",

View File

@@ -9,7 +9,6 @@ import (
"github.com/OffchainLabs/prysm/v6/api"
"github.com/OffchainLabs/prysm/v6/api/server/structs"
"github.com/OffchainLabs/prysm/v6/config/features"
"github.com/OffchainLabs/prysm/v6/consensus-types/primitives"
ethpb "github.com/OffchainLabs/prysm/v6/proto/prysm/v1alpha1"
"github.com/OffchainLabs/prysm/v6/testing/assert"
@@ -215,11 +214,6 @@ func TestGetBeaconBlock_SSZ_BellatrixValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBellatrixBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -262,11 +256,6 @@ func TestGetBeaconBlock_SSZ_BlindedBellatrixValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBlindedBellatrixBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -309,11 +298,6 @@ func TestGetBeaconBlock_SSZ_CapellaValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoCapellaBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -356,11 +340,6 @@ func TestGetBeaconBlock_SSZ_BlindedCapellaValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBlindedCapellaBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -403,11 +382,6 @@ func TestGetBeaconBlock_SSZ_DenebValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoDenebBeaconBlockContents()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -450,11 +424,6 @@ func TestGetBeaconBlock_SSZ_BlindedDenebValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBlindedDenebBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -497,11 +466,6 @@ func TestGetBeaconBlock_SSZ_ElectraValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoElectraBeaconBlockContents()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -544,11 +508,6 @@ func TestGetBeaconBlock_SSZ_BlindedElectraValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBlindedElectraBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -591,11 +550,6 @@ func TestGetBeaconBlock_SSZ_UnsupportedVersion(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
const slot = primitives.Slot(1)
randaoReveal := []byte{2}
graffiti := []byte{3}
@@ -625,11 +579,6 @@ func TestGetBeaconBlock_SSZ_InvalidBlindedHeader(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBellatrixBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -663,11 +612,6 @@ func TestGetBeaconBlock_SSZ_InvalidVersionHeader(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoBellatrixBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -701,11 +645,6 @@ func TestGetBeaconBlock_SSZ_GetSSZError(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
const slot = primitives.Slot(1)
randaoReveal := []byte{2}
graffiti := []byte{3}
@@ -731,11 +670,6 @@ func TestGetBeaconBlock_SSZ_Phase0Valid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoPhase0BeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)
@@ -778,11 +712,6 @@ func TestGetBeaconBlock_SSZ_AltairValid(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
resetFn := features.InitWithReset(&features.Flags{
SSZOnly: true,
})
defer resetFn()
proto := testhelpers.GenerateProtoAltairBeaconBlock()
bytes, err := proto.MarshalSSZ()
require.NoError(t, err)

View File

@@ -7,15 +7,19 @@ import (
"fmt"
"io"
"net/http"
"os"
"strings"
"github.com/OffchainLabs/prysm/v6/api"
"github.com/OffchainLabs/prysm/v6/config/features"
"github.com/OffchainLabs/prysm/v6/api/apiutil"
"github.com/OffchainLabs/prysm/v6/config/params"
"github.com/OffchainLabs/prysm/v6/network/httputil"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)
type reqOption func(*http.Request)
type RestHandler interface {
Get(ctx context.Context, endpoint string, resp interface{}) error
GetSSZ(ctx context.Context, endpoint string) ([]byte, http.Header, error)
@@ -26,16 +30,30 @@ type RestHandler interface {
}
type BeaconApiRestHandler struct {
client http.Client
host string
client http.Client
host string
reqOverrides []reqOption
}
// NewBeaconApiRestHandler returns a RestHandler
func NewBeaconApiRestHandler(client http.Client, host string) RestHandler {
return &BeaconApiRestHandler{
brh := &BeaconApiRestHandler{
client: client,
host: host,
}
brh.appendAcceptOverride()
return brh
}
// appendAcceptOverride enables the Accept header to be customized at runtime via an environment variable.
// This is specified as an env var because it is a niche option that prysm may use for performance testing or debugging
// bug which users are unlikely to need. Using an env var keeps the set of user-facing flags cleaner.
func (c *BeaconApiRestHandler) appendAcceptOverride() {
if accept := os.Getenv(params.EnvNameOverrideAccept); accept != "" {
c.reqOverrides = append(c.reqOverrides, func(req *http.Request) {
req.Header.Set("Accept", accept)
})
}
}
// HttpClient returns the underlying HTTP client of the handler
@@ -56,7 +74,6 @@ func (c *BeaconApiRestHandler) Get(ctx context.Context, endpoint string, resp in
if err != nil {
return errors.Wrapf(err, "failed to create request for endpoint %s", url)
}
httpResp, err := c.client.Do(req)
if err != nil {
return errors.Wrapf(err, "failed to perform request for endpoint %s", url)
@@ -76,13 +93,16 @@ func (c *BeaconApiRestHandler) GetSSZ(ctx context.Context, endpoint string) ([]b
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to create request for endpoint %s", url)
}
primaryAcceptType := fmt.Sprintf("%s;q=%s", api.OctetStreamMediaType, "0.95")
secondaryAcceptType := fmt.Sprintf("%s;q=%s", api.JsonMediaType, "0.9")
acceptHeaderString := fmt.Sprintf("%s,%s", primaryAcceptType, secondaryAcceptType)
if features.Get().SSZOnly {
acceptHeaderString = api.OctetStreamMediaType
}
req.Header.Set("Accept", acceptHeaderString)
for _, o := range c.reqOverrides {
o(req)
}
httpResp, err := c.client.Do(req)
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to perform request for endpoint %s", url)
@@ -92,16 +112,17 @@ func (c *BeaconApiRestHandler) GetSSZ(ctx context.Context, endpoint string) ([]b
return
}
}()
accept := req.Header.Get("Accept")
contentType := httpResp.Header.Get("Content-Type")
body, err := io.ReadAll(httpResp.Body)
if err != nil {
return nil, nil, errors.Wrapf(err, "failed to read response body for %s", httpResp.Request.URL)
}
if !strings.Contains(primaryAcceptType, contentType) {
if !apiutil.PrimaryAcceptMatches(accept, contentType) {
log.WithFields(logrus.Fields{
"primaryAcceptType": primaryAcceptType,
"secondaryAcceptType": secondaryAcceptType,
"receivedAcceptType": contentType,
"Accept": accept,
"Content-Type": contentType,
}).Debug("Server responded with non primary accept type")
}
@@ -115,10 +136,6 @@ func (c *BeaconApiRestHandler) GetSSZ(ctx context.Context, endpoint string) ([]b
return nil, nil, errorJson
}
if features.Get().SSZOnly && contentType != api.OctetStreamMediaType {
return nil, nil, errors.Errorf("server responded with non primary accept type %s", contentType)
}
return body, httpResp.Header, nil
}

View File

@@ -7,11 +7,13 @@ import (
"io"
"net/http"
"net/http/httptest"
"os"
"testing"
"time"
"github.com/OffchainLabs/prysm/v6/api"
"github.com/OffchainLabs/prysm/v6/api/server/structs"
"github.com/OffchainLabs/prysm/v6/config/params"
"github.com/OffchainLabs/prysm/v6/network/httputil"
"github.com/OffchainLabs/prysm/v6/testing/assert"
"github.com/OffchainLabs/prysm/v6/testing/require"
@@ -143,6 +145,25 @@ func TestGetSSZ(t *testing.T) {
})
}
func TestAcceptOverrideSSZ(t *testing.T) {
name := "TestAcceptOverride"
orig := os.Getenv(params.EnvNameOverrideAccept)
defer func() {
require.NoError(t, os.Setenv(params.EnvNameOverrideAccept, orig))
}()
require.NoError(t, os.Setenv(params.EnvNameOverrideAccept, name))
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, name, r.Header.Get("Accept"))
w.WriteHeader(200)
_, err := w.Write([]byte("ok"))
require.NoError(t, err)
}))
defer srv.Close()
c := NewBeaconApiRestHandler(http.Client{Timeout: time.Second * 5}, srv.URL)
_, _, err := c.GetSSZ(t.Context(), "/test")
require.NoError(t, err)
}
func TestPost(t *testing.T) {
ctx := t.Context()
const endpoint = "/example/rest/api/endpoint"