From 8ed5f68b9ed9e328a9e85388c03e126c59c8da7f Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 09:24:53 -0600 Subject: [PATCH 01/52] Add ssz support for getHeader --- .golangci.yml | 1 + server/get_header.go | 85 ++++++++++++++++++++++++++++++++++++++------ server/service.go | 7 +++- server/utils.go | 4 +++ 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ed23268a..1a7d0aff 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -30,6 +30,7 @@ linters: - goconst - gosec - ireturn + - maintidx - noctx - tagliatelle - perfsprint diff --git a/server/get_header.go b/server/get_header.go index 4bd9fb5c..6d33fb3f 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -2,12 +2,15 @@ package server import ( "context" + "encoding/json" "fmt" + "io" "net/http" "sync" "time" builderSpec "github.com/attestantio/go-builder-client/spec" + "github.com/attestantio/go-eth2-client/spec" "github.com/attestantio/go-eth2-client/spec/phase0" "github.com/flashbots/mev-boost/config" "github.com/flashbots/mev-boost/server/types" @@ -16,7 +19,7 @@ import ( ) // getHeader requests a bid from each relay and returns the most profitable one -func (m *BoostService) getHeader(log *logrus.Entry, ua UserAgent, slot phase0.Slot, pubkey, parentHashHex string) (bidResp, error) { +func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, header http.Header) (bidResp, error) { // Ensure arguments are valid if len(pubkey) != 98 { return bidResp{}, errInvalidPubkey @@ -44,12 +47,6 @@ func (m *BoostService) getHeader(log *logrus.Entry, ua UserAgent, slot phase0.Sl "msIntoSlot": msIntoSlot, }).Infof("getHeader request start - %d milliseconds into slot %d", msIntoSlot, slot) - // Add request headers - headers := map[string]string{ - HeaderKeySlotUID: slotUID.String(), - HeaderStartTimeUnixMS: fmt.Sprintf("%d", time.Now().UTC().UnixMilli()), - } - var ( mu sync.Mutex wg sync.WaitGroup @@ -71,18 +68,58 @@ func (m *BoostService) getHeader(log *logrus.Entry, ua UserAgent, slot phase0.Sl url := relay.GetURI(fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", slot, parentHashHex, pubkey)) log := log.WithField("url", url) + // Build the new request + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + log.WithError(err).Warn("error creating new request") + return + } + + // Extend the request header with our values + for key, values := range header { + req.Header[key] = values + } + // Send the get bid request to the relay - bid := new(builderSpec.VersionedSignedBuilderBid) - code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url, ua, headers, nil, bid) + resp, err := m.httpClientGetHeader.Do(req) + if err != nil { + log.WithError(err).Warn("error calling getHeader on relay") + return + } + defer resp.Body.Close() + + // Get the resp body content + respBytes, err := io.ReadAll(resp.Body) if err != nil { - log.WithError(err).Warn("error making request to relay") + log.WithError(err).Warn("error reading response body") return } - if code == http.StatusNoContent { + + // Check if no header is available + if resp.StatusCode == http.StatusNoContent { log.Debug("no-content response") return } + // Check that the response was successful + if resp.StatusCode != http.StatusOK { + err = fmt.Errorf("%w: %d", errHTTPErrorResponse, resp.StatusCode) + log.WithError(err).Warn("error status code") + return + } + + // Get the optional version, used with SSZ decoding + ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + log = log.WithField("eth-consensus-version", ethConsensusVersion) + + // Decode bid + bid := new(builderSpec.VersionedSignedBuilderBid) + err = decodeBid(respBytes, ethConsensusVersion, bid) + if err != nil { + log.WithError(err).Warn("error decoding bid") + return + } + // Skip if bid is empty if bid.IsEmpty() { return @@ -189,3 +226,29 @@ func (m *BoostService) getHeader(log *logrus.Entry, ua UserAgent, slot phase0.Sl result.relays = relays[BlockHashHex(result.bidInfo.blockHash.String())] return result, nil } + +// decodeBid decodes a bid by SSZ if ethConsensusVersion is valid, otherwise JSON +func decodeBid(respBytes []byte, ethConsensusVersion string, bid *builderSpec.VersionedSignedBuilderBid) error { + if ethConsensusVersion != "" { + // Do SSZ decoding + switch ethConsensusVersion { + case "bellatrix": + bid.Version = spec.DataVersionBellatrix + return bid.Bellatrix.UnmarshalSSZ(respBytes) + case "capella": + bid.Version = spec.DataVersionCapella + return bid.Capella.UnmarshalSSZ(respBytes) + case "deneb": + bid.Version = spec.DataVersionDeneb + return bid.Deneb.UnmarshalSSZ(respBytes) + case "electra": + bid.Version = spec.DataVersionElectra + return bid.Electra.UnmarshalSSZ(respBytes) + default: + return errInvalidForkVersion + } + } else { + // Do JSON decoding + return json.Unmarshal(respBytes, bid) + } +} diff --git a/server/service.go b/server/service.go index 0df3b42b..584882a7 100644 --- a/server/service.go +++ b/server/service.go @@ -315,8 +315,13 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) }) log.Debug("getHeader") + // Additional header fields + header := req.Header + header.Set("User-Agent", wrapUserAgent(ua)) + header.Set(HeaderStartTimeUnixMS, fmt.Sprintf("%d", time.Now().UTC().UnixMilli())) + // Query the relays for the header - result, err := m.getHeader(log, ua, slot, pubkey, parentHashHex) + result, err := m.getHeader(log, slot, pubkey, parentHashHex, header) if err != nil { m.respondError(w, http.StatusBadRequest, err.Error()) return diff --git a/server/utils.go b/server/utils.go index 38c414bc..3c3660c6 100644 --- a/server/utils.go +++ b/server/utils.go @@ -266,3 +266,7 @@ func getPayloadResponseIsEmpty(payload *builderApi.VersionedSubmitBlindedBlockRe } return false } + +func wrapUserAgent(ua UserAgent) string { + return strings.TrimSpace(fmt.Sprintf("mev-boost/%s %s", config.Version, ua)) +} From 498d8fcf6c165f84a7a799f380e5520120e81e34 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 09:41:59 -0600 Subject: [PATCH 02/52] Request SSZ then JSON --- server/get_header.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/server/get_header.go b/server/get_header.go index 6d33fb3f..b37f880a 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -81,7 +81,17 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } // Send the get bid request to the relay + // Request SSZ first + req.Header.Set("Accept", "application/octet-stream") resp, err := m.httpClientGetHeader.Do(req) + if err != nil { + // The relay will return NotAcceptable if it doesn't support SSZ + if resp.StatusCode == http.StatusNotAcceptable { + // Try again, but request JSON + req.Header.Set("Accept", "application/json") + resp, err = m.httpClientGetHeader.Do(req) + } + } if err != nil { log.WithError(err).Warn("error calling getHeader on relay") return From a0cc8a0586aca2c512f796ca8d700451a2d87941 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 09:45:51 -0600 Subject: [PATCH 03/52] Read response body after checking for success --- server/get_header.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index b37f880a..c8876e53 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -80,14 +80,13 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header[key] = values } - // Send the get bid request to the relay - // Request SSZ first + // Send the get bid request to the relay. Request SSZ first. req.Header.Set("Accept", "application/octet-stream") resp, err := m.httpClientGetHeader.Do(req) if err != nil { - // The relay will return NotAcceptable if it doesn't support SSZ + // The relay will return NotAcceptable if it does not support SSZ if resp.StatusCode == http.StatusNotAcceptable { - // Try again, but request JSON + // Try again, but request JSON instead req.Header.Set("Accept", "application/json") resp, err = m.httpClientGetHeader.Do(req) } @@ -98,13 +97,6 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } defer resp.Body.Close() - // Get the resp body content - respBytes, err := io.ReadAll(resp.Body) - if err != nil { - log.WithError(err).Warn("error reading response body") - return - } - // Check if no header is available if resp.StatusCode == http.StatusNoContent { log.Debug("no-content response") @@ -118,6 +110,13 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa return } + // Get the resp body content + respBytes, err := io.ReadAll(resp.Body) + if err != nil { + log.WithError(err).Warn("error reading response body") + return + } + // Get the optional version, used with SSZ decoding ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") log = log.WithField("eth-consensus-version", ethConsensusVersion) From 3005de80bf4bf09faedc0db55596ff4399d595bd Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 10:03:29 -0600 Subject: [PATCH 04/52] Request what the client asks for first --- server/get_header.go | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index c8876e53..9bd054c1 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -80,16 +80,29 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header[key] = values } - // Send the get bid request to the relay. Request SSZ first. - req.Header.Set("Accept", "application/octet-stream") - resp, err := m.httpClientGetHeader.Do(req) - if err != nil { - // The relay will return NotAcceptable if it does not support SSZ - if resp.StatusCode == http.StatusNotAcceptable { - // Try again, but request JSON instead - req.Header.Set("Accept", "application/json") - resp, err = m.httpClientGetHeader.Do(req) + // Send the get bid request to the relay. + // Try what the client requests first. + // If no accept type is specified, request JSON. + var resp *http.Response + acceptFromClient := req.Header.Get("Accept") + switch acceptFromClient { + case "application/octet-stream": + log.Debug("requesting header in SSZ") + req.Header.Set("Accept", "application/octet-stream") + resp, err = m.httpClientGetHeader.Do(req) + if resp.StatusCode != http.StatusNotAcceptable { + // The relay didn't complain about the accept value. + // This means we should try processing the response. + log.Debug("response indicated SSZ is not accepted") + break } + // The response status was NotAcceptable. + // This means we should try again with JSON. + fallthrough + default: + log.Debug("requesting header in JSON") + req.Header.Set("Accept", "application/json") + resp, err = m.httpClientGetHeader.Do(req) } if err != nil { log.WithError(err).Warn("error calling getHeader on relay") From 3afd060f50a6580437b50f626e2bdb6bc78d4e52 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 10:22:10 -0600 Subject: [PATCH 05/52] Decode based on resp content type --- server/get_header.go | 52 +++++++++++++++++++++++++----------------- server/types/errors.go | 6 +++++ 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 9bd054c1..aebb23cc 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -132,11 +132,15 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // Get the optional version, used with SSZ decoding ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") - log = log.WithField("eth-consensus-version", ethConsensusVersion) + log = log.WithField("ethConsensusVersion", ethConsensusVersion) + + // Get the response's content type + respContentType := resp.Header.Get("Content-Type") + log = log.WithField("respContentType", respContentType) // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) - err = decodeBid(respBytes, ethConsensusVersion, bid) + err = decodeBid(respBytes, ethConsensusVersion, respContentType, bid) if err != nil { log.WithError(err).Warn("error decoding bid") return @@ -250,27 +254,33 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } // decodeBid decodes a bid by SSZ if ethConsensusVersion is valid, otherwise JSON -func decodeBid(respBytes []byte, ethConsensusVersion string, bid *builderSpec.VersionedSignedBuilderBid) error { - if ethConsensusVersion != "" { - // Do SSZ decoding - switch ethConsensusVersion { - case "bellatrix": - bid.Version = spec.DataVersionBellatrix - return bid.Bellatrix.UnmarshalSSZ(respBytes) - case "capella": - bid.Version = spec.DataVersionCapella - return bid.Capella.UnmarshalSSZ(respBytes) - case "deneb": - bid.Version = spec.DataVersionDeneb - return bid.Deneb.UnmarshalSSZ(respBytes) - case "electra": - bid.Version = spec.DataVersionElectra - return bid.Electra.UnmarshalSSZ(respBytes) - default: - return errInvalidForkVersion +func decodeBid(respBytes []byte, ethConsensusVersion string, respContentType string, bid *builderSpec.VersionedSignedBuilderBid) error { + switch respContentType { + case "application/octet-stream": + if ethConsensusVersion != "" { + // Do SSZ decoding + switch ethConsensusVersion { + case "bellatrix": + bid.Version = spec.DataVersionBellatrix + return bid.Bellatrix.UnmarshalSSZ(respBytes) + case "capella": + bid.Version = spec.DataVersionCapella + return bid.Capella.UnmarshalSSZ(respBytes) + case "deneb": + bid.Version = spec.DataVersionDeneb + return bid.Deneb.UnmarshalSSZ(respBytes) + case "electra": + bid.Version = spec.DataVersionElectra + return bid.Electra.UnmarshalSSZ(respBytes) + default: + return errInvalidForkVersion + } + } else { + return types.ErrMissingEthConsensusVersion } - } else { + case "application/json": // Do JSON decoding return json.Unmarshal(respBytes, bid) } + return types.ErrInvalidContentType } diff --git a/server/types/errors.go b/server/types/errors.go index 54fdcd99..478cdd68 100644 --- a/server/types/errors.go +++ b/server/types/errors.go @@ -7,3 +7,9 @@ var ErrMissingRelayPubkey = errors.New("missing relay public key") // ErrPointAtInfinityPubkey is returned if a new RelayEntry URL has point-at-infinity public key. var ErrPointAtInfinityPubkey = errors.New("relay public key cannot be the point-at-infinity") + +// ErrInvalidContentType is returned when the response content type is invalid. +var ErrInvalidContentType = errors.New("invalid content type") + +// ErrMissingEthConsensusVersion is returned when the response is octet-stream but there is no "Eth-Consensus-Version" header. +var ErrMissingEthConsensusVersion = errors.New("missing eth-consensus-version") From f65e83d711e727738a4f1c7c4c3ffc17cdc47547 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 10:48:19 -0600 Subject: [PATCH 06/52] Return the best bid in the request encoding --- server/get_header.go | 2 +- server/service.go | 44 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index aebb23cc..e0787c14 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -254,7 +254,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } // decodeBid decodes a bid by SSZ if ethConsensusVersion is valid, otherwise JSON -func decodeBid(respBytes []byte, ethConsensusVersion string, respContentType string, bid *builderSpec.VersionedSignedBuilderBid) error { +func decodeBid(respBytes []byte, ethConsensusVersion, respContentType string, bid *builderSpec.VersionedSignedBuilderBid) error { switch respContentType { case "application/octet-stream": if ethConsensusVersion != "" { diff --git a/server/service.go b/server/service.go index 584882a7..06cf49e8 100644 --- a/server/service.go +++ b/server/service.go @@ -21,6 +21,7 @@ import ( eth2ApiV1Capella "github.com/attestantio/go-eth2-client/api/v1/capella" eth2ApiV1Deneb "github.com/attestantio/go-eth2-client/api/v1/deneb" eth2ApiV1Electra "github.com/attestantio/go-eth2-client/api/v1/electra" + "github.com/attestantio/go-eth2-client/spec" "github.com/attestantio/go-eth2-client/spec/phase0" "github.com/flashbots/go-boost-utils/ssz" "github.com/flashbots/go-utils/httplogger" @@ -338,9 +339,13 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) m.bids[bidKey(slot, result.bidInfo.blockHash)] = result m.bidsLock.Unlock() + // How should we respond to the client + acceptFromClient := req.Header.Get("Accept") + // Log result valueEth := weiBigIntToEthBigFloat(result.bidInfo.value.ToBig()) log.WithFields(logrus.Fields{ + "acceptType": acceptFromClient, "blockHash": result.bidInfo.blockHash.String(), "blockNumber": result.bidInfo.blockNumber, "txRoot": result.bidInfo.txRoot.String(), @@ -349,7 +354,44 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) }).Info("best bid") // Return the bid - m.respondOK(w, &result.response) + switch acceptFromClient { + case "application/octet-stream": + w.Header().Set("Content-Type", "application/octet-stream") + w.WriteHeader(http.StatusOK) + + // Serialize the response + var sszData []byte + switch result.response.Version { + case spec.DataVersionBellatrix: + sszData, err = result.response.Bellatrix.MarshalSSZ() + case spec.DataVersionCapella: + sszData, err = result.response.Capella.MarshalSSZ() + case spec.DataVersionDeneb: + sszData, err = result.response.Deneb.MarshalSSZ() + case spec.DataVersionElectra: + sszData, err = result.response.Electra.MarshalSSZ() + case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: + err = errInvalidForkVersion + } + if err != nil { + m.log.WithError(err).Error("error serializing response as SSZ") + http.Error(w, "failed to serialize response", http.StatusInternalServerError) + return + } + + // Write SSZ data + if _, err := w.Write(sszData); err != nil { + m.log.WithError(err).Error("error writing SSZ response") + http.Error(w, "failed to write response", http.StatusInternalServerError) + } + default: + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(&result.response); err != nil { + m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") + http.Error(w, "", http.StatusInternalServerError) + } + } } // respondPayload responds to the proposer with the payload From 2073a417fe0b75635abd1daada4c764f37518581 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 10:54:26 -0600 Subject: [PATCH 07/52] Fix some nits --- server/get_header.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index e0787c14..d4609557 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -130,17 +130,17 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa return } - // Get the optional version, used with SSZ decoding - ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") - log = log.WithField("ethConsensusVersion", ethConsensusVersion) - // Get the response's content type respContentType := resp.Header.Get("Content-Type") log = log.WithField("respContentType", respContentType) + // Get the optional version, used with SSZ decoding + ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + log = log.WithField("ethConsensusVersion", ethConsensusVersion) + // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) - err = decodeBid(respBytes, ethConsensusVersion, respContentType, bid) + err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) if err != nil { log.WithError(err).Warn("error decoding bid") return @@ -253,8 +253,8 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa return result, nil } -// decodeBid decodes a bid by SSZ if ethConsensusVersion is valid, otherwise JSON -func decodeBid(respBytes []byte, ethConsensusVersion, respContentType string, bid *builderSpec.VersionedSignedBuilderBid) error { +// decodeBid decodes a bid by SSZ or JSON, depending on the provided respContentType +func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bid *builderSpec.VersionedSignedBuilderBid) error { switch respContentType { case "application/octet-stream": if ethConsensusVersion != "" { From b4b9088c233b0dc969189be16197719cb95e1418 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 10:56:29 -0600 Subject: [PATCH 08/52] Fix debug message --- server/get_header.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/get_header.go b/server/get_header.go index d4609557..a26e16c5 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -93,11 +93,12 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa if resp.StatusCode != http.StatusNotAcceptable { // The relay didn't complain about the accept value. // This means we should try processing the response. - log.Debug("response indicated SSZ is not accepted") + log.Debug("response indicated SSZ is accepted") break } // The response status was NotAcceptable. // This means we should try again with JSON. + log.Debug("response indicated SSZ is not accepted") fallthrough default: log.Debug("requesting header in JSON") From b1cda562335d6e2963a1fb111ba7ca24c2b22703 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 11:34:06 -0600 Subject: [PATCH 09/52] Always request header from relay in SSZ first --- server/get_header.go | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index a26e16c5..3ed2ec79 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -80,36 +80,32 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header[key] = values } - // Send the get bid request to the relay. - // Try what the client requests first. - // If no accept type is specified, request JSON. + // Send the get bid request to the relay. Start by requesting SSZ from + // the relay, even if the client requested JSON. We will return the + // best bid to the client in whichever format the client requested. var resp *http.Response - acceptFromClient := req.Header.Get("Accept") - switch acceptFromClient { - case "application/octet-stream": - log.Debug("requesting header in SSZ") - req.Header.Set("Accept", "application/octet-stream") - resp, err = m.httpClientGetHeader.Do(req) - if resp.StatusCode != http.StatusNotAcceptable { - // The relay didn't complain about the accept value. - // This means we should try processing the response. - log.Debug("response indicated SSZ is accepted") - break - } - // The response status was NotAcceptable. - // This means we should try again with JSON. + log.Debug("requesting header in SSZ") + req.Header.Set("Accept", "application/octet-stream") + resp, err = m.httpClientGetHeader.Do(req) + if err != nil { + log.WithError(err).WithField("encoding", "ssz").Warn("error calling getHeader on relay") + return + } + defer resp.Body.Close() + + // If the relay does not support SSZ, try again with JSON + if resp.StatusCode == http.StatusNotAcceptable { + resp.Body.Close() log.Debug("response indicated SSZ is not accepted") - fallthrough - default: log.Debug("requesting header in JSON") req.Header.Set("Accept", "application/json") resp, err = m.httpClientGetHeader.Do(req) + if err != nil { + log.WithError(err).WithField("encoding", "json").Warn("error calling getHeader on relay") + return + } + defer resp.Body.Close() } - if err != nil { - log.WithError(err).Warn("error calling getHeader on relay") - return - } - defer resp.Body.Close() // Check if no header is available if resp.StatusCode == http.StatusNoContent { From db3f598b48e8557f1e7018ffcbee189d8298a847 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 13:12:09 -0600 Subject: [PATCH 10/52] Revert "Always request header from relay in SSZ first" This reverts commit b1cda562335d6e2963a1fb111ba7ca24c2b22703. --- server/get_header.go | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 3ed2ec79..a26e16c5 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -80,32 +80,36 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header[key] = values } - // Send the get bid request to the relay. Start by requesting SSZ from - // the relay, even if the client requested JSON. We will return the - // best bid to the client in whichever format the client requested. + // Send the get bid request to the relay. + // Try what the client requests first. + // If no accept type is specified, request JSON. var resp *http.Response - log.Debug("requesting header in SSZ") - req.Header.Set("Accept", "application/octet-stream") - resp, err = m.httpClientGetHeader.Do(req) - if err != nil { - log.WithError(err).WithField("encoding", "ssz").Warn("error calling getHeader on relay") - return - } - defer resp.Body.Close() - - // If the relay does not support SSZ, try again with JSON - if resp.StatusCode == http.StatusNotAcceptable { - resp.Body.Close() + acceptFromClient := req.Header.Get("Accept") + switch acceptFromClient { + case "application/octet-stream": + log.Debug("requesting header in SSZ") + req.Header.Set("Accept", "application/octet-stream") + resp, err = m.httpClientGetHeader.Do(req) + if resp.StatusCode != http.StatusNotAcceptable { + // The relay didn't complain about the accept value. + // This means we should try processing the response. + log.Debug("response indicated SSZ is accepted") + break + } + // The response status was NotAcceptable. + // This means we should try again with JSON. log.Debug("response indicated SSZ is not accepted") + fallthrough + default: log.Debug("requesting header in JSON") req.Header.Set("Accept", "application/json") resp, err = m.httpClientGetHeader.Do(req) - if err != nil { - log.WithError(err).WithField("encoding", "json").Warn("error calling getHeader on relay") - return - } - defer resp.Body.Close() } + if err != nil { + log.WithError(err).Warn("error calling getHeader on relay") + return + } + defer resp.Body.Close() // Check if no header is available if resp.StatusCode == http.StatusNoContent { From 654554001fb9ca8d96afa5b6bfb675cb41f0d139 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 13:14:45 -0600 Subject: [PATCH 11/52] Update about request (SSZ and JSON) order --- server/get_header.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index a26e16c5..cb14a0c9 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -80,9 +80,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header[key] = values } - // Send the get bid request to the relay. - // Try what the client requests first. - // If no accept type is specified, request JSON. + // Send the get bid request to the relay. Try what the client + // accepts (either SSZ or JSON) first. If no accept type is specified, + // request JSON. We cannot request SSZ if the client does not, because + // the appropriate Eth-Consensus-Version header value is not known. var resp *http.Response acceptFromClient := req.Header.Get("Accept") switch acceptFromClient { From 735bcbd1e80350e5c27d49ce2a7e1ae1d1c0d90c Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 13:21:25 -0600 Subject: [PATCH 12/52] Update mock_relay.defaultHandleGetHeader to handle SSZ --- server/mock/mock_relay.go | 70 ++++++++++++++++++++++++++++----------- server/service_test.go | 62 +++++++++++++++++----------------- 2 files changed, 82 insertions(+), 50 deletions(-) diff --git a/server/mock/mock_relay.go b/server/mock/mock_relay.go index a93462fc..2e265893 100644 --- a/server/mock/mock_relay.go +++ b/server/mock/mock_relay.go @@ -259,30 +259,62 @@ func (m *Relay) handleGetHeader(w http.ResponseWriter, req *http.Request) { m.handlerOverrideGetHeader(w, req) return } - m.defaultHandleGetHeader(w) + m.defaultHandleGetHeader(w, req) } // defaultHandleGetHeader returns the default handler for handleGetHeader -func (m *Relay) defaultHandleGetHeader(w http.ResponseWriter) { - // By default, everything will be ok. - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) +func (m *Relay) defaultHandleGetHeader(w http.ResponseWriter, req *http.Request) { + if req.Header.Get("Accept") == "application/octet-stream" { + w.Header().Set("Content-Type", "application/octet-stream") + w.WriteHeader(http.StatusOK) + + // Build the default response. + response := m.MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + if m.GetHeaderResponse != nil { + response = m.GetHeaderResponse + } - // Build the default response. - response := m.MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - if m.GetHeaderResponse != nil { - response = m.GetHeaderResponse - } + // Encode response to SSZ + sszData, err := response.Deneb.MarshalSSZ() + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } - if err := json.NewEncoder(w).Encode(response); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return + // Write the version and data + w.Header().Set("Eth-Consensus-Version", "deneb") + _, err = w.Write(sszData) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + } else { + // By default, return JSON + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + + // Build the default response. + response := m.MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + if m.GetHeaderResponse != nil { + response = m.GetHeaderResponse + } + + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } } } diff --git a/server/service_test.go b/server/service_test.go index 999be70c..5f7b89ca 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -75,7 +75,7 @@ func newTestBackend(t *testing.T, numRelays int, relayTimeout time.Duration) *te return &backend } -func (be *testBackend) request(t *testing.T, method, path string, payload any) *httptest.ResponseRecorder { +func (be *testBackend) request(t *testing.T, method, path, accept string, payload any) *httptest.ResponseRecorder { t.Helper() var req *http.Request var err error @@ -172,7 +172,7 @@ func TestStatus(t *testing.T) { backend := newTestBackend(t, 1, time.Second) time.Sleep(time.Millisecond * 20) path := "/eth/v1/builder/status" - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, http.StatusOK, rr.Code) require.NotEmpty(t, rr.Header().Get("X-MEVBoost-Version")) @@ -184,7 +184,7 @@ func TestStatus(t *testing.T) { backend.relays[0].Server.Close() // makes the relay unavailable path := "/eth/v1/builder/status" - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, http.StatusServiceUnavailable, rr.Code) require.NotEmpty(t, rr.Header().Get("X-MEVBoost-Version")) @@ -208,7 +208,7 @@ func TestRegisterValidator(t *testing.T) { t.Run("Normal function", func(t *testing.T) { backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, http.StatusOK, rr.Code) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) }) @@ -219,7 +219,7 @@ func TestRegisterValidator(t *testing.T) { backend.relays[0].ResponseDelay = 5 * time.Millisecond backend.relays[1].ResponseDelay = 5 * time.Millisecond - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, http.StatusOK, rr.Code) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) @@ -228,7 +228,7 @@ func TestRegisterValidator(t *testing.T) { backend.relays[0].OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) }) - rr = backend.request(t, http.MethodPost, path, payload) + rr = backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, http.StatusOK, rr.Code) require.Equal(t, 2, backend.relays[0].GetRequestCount(path)) require.Equal(t, 2, backend.relays[1].GetRequestCount(path)) @@ -237,7 +237,7 @@ func TestRegisterValidator(t *testing.T) { backend.relays[1].OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) }) - rr = backend.request(t, http.MethodPost, path, payload) + rr = backend.request(t, http.MethodPost, path, "application/json", payload) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadGateway, rr.Code) require.Equal(t, 3, backend.relays[0].GetRequestCount(path)) @@ -246,12 +246,12 @@ func TestRegisterValidator(t *testing.T) { t.Run("mev-boost relay timeout works with slow relay", func(t *testing.T) { backend := newTestBackend(t, 1, 150*time.Millisecond) // 10ms max - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, http.StatusOK, rr.Code) // Now make the relay return slowly, mev-boost should return an error backend.relays[0].ResponseDelay = 180 * time.Millisecond - rr = backend.request(t, http.MethodPost, path, payload) + rr = backend.request(t, http.MethodPost, path, "application/json", payload) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadGateway, rr.Code) require.Equal(t, 2, backend.relays[0].GetRequestCount(path)) @@ -271,7 +271,7 @@ func TestGetHeader(t *testing.T) { t.Run("Okay response from relay", func(t *testing.T) { backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) }) @@ -286,7 +286,7 @@ func TestGetHeader(t *testing.T) { spec.DataVersionDeneb, ) backend.relays[0].GetHeaderResponse = resp - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) }) @@ -304,14 +304,14 @@ func TestGetHeader(t *testing.T) { // 1/2 failing responses are okay backend.relays[0].GetHeaderResponse = resp - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) // 2/2 failing responses are okay backend.relays[1].GetHeaderResponse = resp - rr = backend.request(t, http.MethodGet, path, nil) + rr = backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, 2, backend.relays[0].GetRequestCount(path)) require.Equal(t, 2, backend.relays[1].GetRequestCount(path)) require.Equal(t, http.StatusNoContent, rr.Code) @@ -332,7 +332,7 @@ func TestGetHeader(t *testing.T) { pk := phase0.BLSPubKey{} backend.boost.relays[0].PublicKey = pk - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) // Request should have no content @@ -353,7 +353,7 @@ func TestGetHeader(t *testing.T) { // Scramble the signature backend.relays[0].GetHeaderResponse.Deneb.Signature = phase0.BLSSignature{} - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) // Request should have no content @@ -366,7 +366,7 @@ func TestGetHeader(t *testing.T) { invalidSlotPath := fmt.Sprintf("/eth/v1/builder/header/%s/%s/%s", slot, hash.String(), pubkey.String()) backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, invalidSlotPath, nil) + rr := backend.request(t, http.MethodGet, invalidSlotPath, "application/json", nil) require.JSONEq(t, `{"code":400,"message":"invalid slot"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) @@ -376,7 +376,7 @@ func TestGetHeader(t *testing.T) { invalidPubkeyPath := fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", 1, hash.String(), "0x1") backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, invalidPubkeyPath, nil) + rr := backend.request(t, http.MethodGet, invalidPubkeyPath, "application/json", nil) require.JSONEq(t, `{"code":400,"message":"invalid pubkey"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) @@ -386,7 +386,7 @@ func TestGetHeader(t *testing.T) { invalidSlotPath := fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", 1, "0x1", pubkey.String()) backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, invalidSlotPath, nil) + rr := backend.request(t, http.MethodGet, invalidSlotPath, "application/json", nil) require.JSONEq(t, `{"code":400,"message":"invalid hash"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) @@ -396,7 +396,7 @@ func TestGetHeader(t *testing.T) { backend := newTestBackend(t, 1, time.Second) invalidParentHashPath := getHeaderPath(1, phase0.Hash32{}, pubkey) - rr := backend.request(t, http.MethodGet, invalidParentHashPath, nil) + rr := backend.request(t, http.MethodGet, invalidParentHashPath, "application/json", nil) require.Equal(t, http.StatusNoContent, rr.Code) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) }) @@ -441,7 +441,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -488,7 +488,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -524,7 +524,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -547,7 +547,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, nil) + rr := backend.request(t, http.MethodGet, path, "application/json", nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -600,7 +600,7 @@ func TestGetPayload(t *testing.T) { t.Run("Okay response from relay", func(t *testing.T) { backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -624,7 +624,7 @@ func TestGetPayload(t *testing.T) { // 1/2 failing responses are okay backend.relays[0].GetPayloadResponse = resp - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.GreaterOrEqual(t, backend.relays[1].GetRequestCount(path)+backend.relays[0].GetRequestCount(path), 1) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) @@ -632,7 +632,7 @@ func TestGetPayload(t *testing.T) { backend = newTestBackend(t, 2, time.Second) backend.relays[0].GetPayloadResponse = resp backend.relays[1].GetPayloadResponse = resp - rr = backend.request(t, http.MethodPost, path, payload) + rr = backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) @@ -654,7 +654,7 @@ func TestGetPayload(t *testing.T) { } count++ }) - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) }) @@ -675,7 +675,7 @@ func TestGetPayload(t *testing.T) { require.NoError(t, err, "failed to write error response") //nolint:testifylint // if we fail here the test is compromised } }) - rr := backend.request(t, http.MethodPost, path, payload) + rr := backend.request(t, http.MethodPost, path, "application/json", payload) require.Equal(t, 5, backend.relays[0].GetRequestCount(path)) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadGateway, rr.Code, rr.Body.String()) @@ -879,7 +879,7 @@ func TestGetPayloadForks(t *testing.T) { // Prepare getPayload response backend.relays[0].GetPayloadResponse = blindedBlockToBlockResponse(signedBlindedBeaconBlock) // call getPayload, ensure it's only called on relay 0 (origin of the bid) - rr := backend.request(t, http.MethodPost, params.PathGetPayload, signedBlindedBeaconBlock) + rr := backend.request(t, http.MethodPost, params.PathGetPayload, "application/json", signedBlindedBeaconBlock) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(params.PathGetPayload)) resp := new(builderApi.VersionedSubmitBlindedBlockResponse) @@ -911,7 +911,7 @@ func TestGetPayloadToAllRelays(t *testing.T) { "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", spec.DataVersionDeneb, ) - rr := backend.request(t, http.MethodGet, getHeaderPath, nil) + rr := backend.request(t, http.MethodGet, getHeaderPath, "application/json", nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(getHeaderPath)) require.Equal(t, 1, backend.relays[1].GetRequestCount(getHeaderPath)) @@ -920,7 +920,7 @@ func TestGetPayloadToAllRelays(t *testing.T) { backend.relays[0].GetPayloadResponse = blindedBlockToBlockResponse(signedBlindedBeaconBlock) // call getPayload, ensure it's called to all relays - rr = backend.request(t, http.MethodPost, params.PathGetPayload, signedBlindedBeaconBlock) + rr = backend.request(t, http.MethodPost, params.PathGetPayload, "application/json", signedBlindedBeaconBlock) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(params.PathGetPayload)) require.Equal(t, 1, backend.relays[1].GetRequestCount(params.PathGetPayload)) From 881cff0bb1f3dc2806bc0095dc7fe6f734a81676 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 13:58:07 -0600 Subject: [PATCH 13/52] Add a test & fix a couple bugs --- server/get_header.go | 16 +++- server/mock/mock_relay.go | 2 +- server/service_test.go | 168 ++++++++++++++++++++++++++++++-------- 3 files changed, 148 insertions(+), 38 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index cb14a0c9..e1cb8154 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -9,6 +9,10 @@ import ( "sync" "time" + builderApiBellatrix "github.com/attestantio/go-builder-client/api/bellatrix" + builderApiCapella "github.com/attestantio/go-builder-client/api/capella" + builderApiDeneb "github.com/attestantio/go-builder-client/api/deneb" + builderApiElectra "github.com/attestantio/go-builder-client/api/electra" builderSpec "github.com/attestantio/go-builder-client/spec" "github.com/attestantio/go-eth2-client/spec" "github.com/attestantio/go-eth2-client/spec/phase0" @@ -80,6 +84,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header[key] = values } + // Get the optional version, used with SSZ decoding + ethConsensusVersion := header.Get("Eth-Consensus-Version") + log = log.WithField("ethConsensusVersion", ethConsensusVersion) + // Send the get bid request to the relay. Try what the client // accepts (either SSZ or JSON) first. If no accept type is specified, // request JSON. We cannot request SSZ if the client does not, because @@ -136,10 +144,6 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa respContentType := resp.Header.Get("Content-Type") log = log.WithField("respContentType", respContentType) - // Get the optional version, used with SSZ decoding - ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") - log = log.WithField("ethConsensusVersion", ethConsensusVersion) - // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) @@ -264,15 +268,19 @@ func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bi switch ethConsensusVersion { case "bellatrix": bid.Version = spec.DataVersionBellatrix + bid.Bellatrix = new(builderApiBellatrix.SignedBuilderBid) return bid.Bellatrix.UnmarshalSSZ(respBytes) case "capella": bid.Version = spec.DataVersionCapella + bid.Capella = new(builderApiCapella.SignedBuilderBid) return bid.Capella.UnmarshalSSZ(respBytes) case "deneb": bid.Version = spec.DataVersionDeneb + bid.Deneb = new(builderApiDeneb.SignedBuilderBid) return bid.Deneb.UnmarshalSSZ(respBytes) case "electra": bid.Version = spec.DataVersionElectra + bid.Electra = new(builderApiElectra.SignedBuilderBid) return bid.Electra.UnmarshalSSZ(respBytes) default: return errInvalidForkVersion diff --git a/server/mock/mock_relay.go b/server/mock/mock_relay.go index 2e265893..6791107c 100644 --- a/server/mock/mock_relay.go +++ b/server/mock/mock_relay.go @@ -202,6 +202,7 @@ func (m *Relay) MakeGetHeaderResponse(value uint64, blockHash, parentHash, publi ParentHash: HexToHash(parentHash), WithdrawalsRoot: phase0.Root{}, BaseFeePerGas: uint256.NewInt(0), + ExtraData: make([]byte, 0), }, BlobKZGCommitments: make([]deneb.KZGCommitment, 0), Value: uint256.NewInt(value), @@ -288,7 +289,6 @@ func (m *Relay) defaultHandleGetHeader(w http.ResponseWriter, req *http.Request) } // Write the version and data - w.Header().Set("Eth-Consensus-Version", "deneb") _, err = w.Write(sszData) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/server/service_test.go b/server/service_test.go index 5f7b89ca..7fb840fb 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -75,7 +75,7 @@ func newTestBackend(t *testing.T, numRelays int, relayTimeout time.Duration) *te return &backend } -func (be *testBackend) request(t *testing.T, method, path, accept string, payload any) *httptest.ResponseRecorder { +func (be *testBackend) request(t *testing.T, method, path string, header http.Header, payload any) *httptest.ResponseRecorder { t.Helper() var req *http.Request var err error @@ -87,8 +87,11 @@ func (be *testBackend) request(t *testing.T, method, path, accept string, payloa require.NoError(t, err2) req, err = http.NewRequest(method, path, bytes.NewReader(payloadBytes)) } - require.NoError(t, err) + + // Set header + req.Header = header + rr := httptest.NewRecorder() be.boost.getRouter().ServeHTTP(rr, req) return rr @@ -169,10 +172,13 @@ func TestWebserverMaxHeaderSize(t *testing.T) { func TestStatus(t *testing.T) { t.Run("At least one relay is available", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) time.Sleep(time.Millisecond * 20) path := "/eth/v1/builder/status" - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code) require.NotEmpty(t, rr.Header().Get("X-MEVBoost-Version")) @@ -180,11 +186,14 @@ func TestStatus(t *testing.T) { }) t.Run("No relays available", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) backend.relays[0].Server.Close() // makes the relay unavailable path := "/eth/v1/builder/status" - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusServiceUnavailable, rr.Code) require.NotEmpty(t, rr.Header().Get("X-MEVBoost-Version")) @@ -207,19 +216,24 @@ func TestRegisterValidator(t *testing.T) { payload := []builderApiV1.SignedValidatorRegistration{reg} t.Run("Normal function", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, http.StatusOK, rr.Code) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) }) t.Run("Relay error response", func(t *testing.T) { - backend := newTestBackend(t, 2, time.Second) + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 2, time.Second) backend.relays[0].ResponseDelay = 5 * time.Millisecond backend.relays[1].ResponseDelay = 5 * time.Millisecond - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, http.StatusOK, rr.Code) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) @@ -228,7 +242,8 @@ func TestRegisterValidator(t *testing.T) { backend.relays[0].OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) }) - rr = backend.request(t, http.MethodPost, path, "application/json", payload) + + rr = backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, http.StatusOK, rr.Code) require.Equal(t, 2, backend.relays[0].GetRequestCount(path)) require.Equal(t, 2, backend.relays[1].GetRequestCount(path)) @@ -237,7 +252,7 @@ func TestRegisterValidator(t *testing.T) { backend.relays[1].OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { w.WriteHeader(http.StatusBadRequest) }) - rr = backend.request(t, http.MethodPost, path, "application/json", payload) + rr = backend.request(t, http.MethodPost, path, header, payload) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadGateway, rr.Code) require.Equal(t, 3, backend.relays[0].GetRequestCount(path)) @@ -245,13 +260,16 @@ func TestRegisterValidator(t *testing.T) { }) t.Run("mev-boost relay timeout works with slow relay", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, 150*time.Millisecond) // 10ms max - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, http.StatusOK, rr.Code) // Now make the relay return slowly, mev-boost should return an error backend.relays[0].ResponseDelay = 180 * time.Millisecond - rr = backend.request(t, http.MethodPost, path, "application/json", payload) + rr = backend.request(t, http.MethodPost, path, header, payload) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadGateway, rr.Code) require.Equal(t, 2, backend.relays[0].GetRequestCount(path)) @@ -270,13 +288,19 @@ func TestGetHeader(t *testing.T) { require.Equal(t, "/eth/v1/builder/header/1/0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7/0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", path) t.Run("Okay response from relay", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) }) t.Run("Okay response from relay deneb", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) resp := backend.relays[0].MakeGetHeaderResponse( 12345, @@ -286,12 +310,42 @@ func TestGetHeader(t *testing.T) { spec.DataVersionDeneb, ) backend.relays[0].GetHeaderResponse = resp - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) }) + t.Run("Okay response from relay deneb in ssz", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream") + + backend := newTestBackend(t, 1, time.Second) + resp := backend.relays[0].MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + backend.relays[0].GetHeaderResponse = resp + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + + bid := new(builderApiDeneb.SignedBuilderBid) + err := bid.UnmarshalSSZ(rr.Body.Bytes()) + require.NoError(t, err) + require.EqualValues(t, *resp.Deneb, *bid) + + rr.Body.Bytes() + + }) + t.Run("Bad response from relays", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 2, time.Second) resp := backend.relays[0].MakeGetHeaderResponse( 12345, @@ -304,20 +358,23 @@ func TestGetHeader(t *testing.T) { // 1/2 failing responses are okay backend.relays[0].GetHeaderResponse = resp - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) // 2/2 failing responses are okay backend.relays[1].GetHeaderResponse = resp - rr = backend.request(t, http.MethodGet, path, "application/json", nil) + rr = backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, 2, backend.relays[0].GetRequestCount(path)) require.Equal(t, 2, backend.relays[1].GetRequestCount(path)) require.Equal(t, http.StatusNoContent, rr.Code) }) t.Run("Invalid relay public key", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) backend.relays[0].GetHeaderResponse = backend.relays[0].MakeGetHeaderResponse( @@ -332,7 +389,7 @@ func TestGetHeader(t *testing.T) { pk := phase0.BLSPubKey{} backend.boost.relays[0].PublicKey = pk - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) // Request should have no content @@ -340,6 +397,9 @@ func TestGetHeader(t *testing.T) { }) t.Run("Invalid relay signature", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) backend.relays[0].GetHeaderResponse = backend.relays[0].MakeGetHeaderResponse( @@ -353,7 +413,7 @@ func TestGetHeader(t *testing.T) { // Scramble the signature backend.relays[0].GetHeaderResponse.Deneb.Signature = phase0.BLSSignature{} - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) // Request should have no content @@ -361,42 +421,54 @@ func TestGetHeader(t *testing.T) { }) t.Run("Invalid slot number", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + // Number larger than uint64 creates parsing error slot := fmt.Sprintf("%d0", uint64(math.MaxUint64)) invalidSlotPath := fmt.Sprintf("/eth/v1/builder/header/%s/%s/%s", slot, hash.String(), pubkey.String()) backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, invalidSlotPath, "application/json", nil) + rr := backend.request(t, http.MethodGet, invalidSlotPath, header, nil) require.JSONEq(t, `{"code":400,"message":"invalid slot"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) }) t.Run("Invalid pubkey length", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + invalidPubkeyPath := fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", 1, hash.String(), "0x1") backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, invalidPubkeyPath, "application/json", nil) + rr := backend.request(t, http.MethodGet, invalidPubkeyPath, header, nil) require.JSONEq(t, `{"code":400,"message":"invalid pubkey"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) }) t.Run("Invalid hash length", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + invalidSlotPath := fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", 1, "0x1", pubkey.String()) backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodGet, invalidSlotPath, "application/json", nil) + rr := backend.request(t, http.MethodGet, invalidSlotPath, header, nil) require.JSONEq(t, `{"code":400,"message":"invalid hash"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadRequest, rr.Code, rr.Body.String()) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) }) t.Run("Invalid parent hash", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) invalidParentHashPath := getHeaderPath(1, phase0.Hash32{}, pubkey) - rr := backend.request(t, http.MethodGet, invalidParentHashPath, "application/json", nil) + rr := backend.request(t, http.MethodGet, invalidParentHashPath, header, nil) require.Equal(t, http.StatusNoContent, rr.Code) require.Equal(t, 0, backend.relays[0].GetRequestCount(path)) }) @@ -410,6 +482,9 @@ func TestGetHeaderBids(t *testing.T) { require.Equal(t, "/eth/v1/builder/header/2/0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7/0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", path) t.Run("Use header with highest value", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + // Create backend and register 3 relays. backend := newTestBackend(t, 3, time.Second) @@ -441,7 +516,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -460,6 +535,9 @@ func TestGetHeaderBids(t *testing.T) { }) t.Run("Use header with lowest blockhash if same value", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + // Create backend and register 3 relays. backend := newTestBackend(t, 3, time.Second) @@ -488,7 +566,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -511,6 +589,9 @@ func TestGetHeaderBids(t *testing.T) { }) t.Run("Respect minimum bid cutoff", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + // Create backend and register relay. backend := newTestBackend(t, 1, time.Second) @@ -524,7 +605,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -534,6 +615,9 @@ func TestGetHeaderBids(t *testing.T) { }) t.Run("Allow bids which meet minimum bid cutoff", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + // Create backend and register relay. backend := newTestBackend(t, 1, time.Second) @@ -547,7 +631,7 @@ func TestGetHeaderBids(t *testing.T) { ) // Run the request. - rr := backend.request(t, http.MethodGet, path, "application/json", nil) + rr := backend.request(t, http.MethodGet, path, header, nil) // Each relay must have received the request. require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -599,8 +683,11 @@ func TestGetPayload(t *testing.T) { } t.Run("Okay response from relay", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -611,6 +698,9 @@ func TestGetPayload(t *testing.T) { }) t.Run("Bad response from relays", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 2, time.Second) resp := &builderApi.VersionedSubmitBlindedBlockResponse{ Version: spec.DataVersionDeneb, @@ -624,7 +714,7 @@ func TestGetPayload(t *testing.T) { // 1/2 failing responses are okay backend.relays[0].GetPayloadResponse = resp - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.GreaterOrEqual(t, backend.relays[1].GetRequestCount(path)+backend.relays[0].GetRequestCount(path), 1) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) @@ -632,7 +722,7 @@ func TestGetPayload(t *testing.T) { backend = newTestBackend(t, 2, time.Second) backend.relays[0].GetPayloadResponse = resp backend.relays[1].GetPayloadResponse = resp - rr = backend.request(t, http.MethodPost, path, "application/json", payload) + rr = backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) @@ -640,6 +730,9 @@ func TestGetPayload(t *testing.T) { }) t.Run("Retries on error from relay", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, 2*time.Second) count := 0 @@ -654,11 +747,14 @@ func TestGetPayload(t *testing.T) { } count++ }) - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) }) t.Run("Error after max retries are reached", func(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + backend := newTestBackend(t, 1, time.Second) count := 0 @@ -675,7 +771,7 @@ func TestGetPayload(t *testing.T) { require.NoError(t, err, "failed to write error response") //nolint:testifylint // if we fail here the test is compromised } }) - rr := backend.request(t, http.MethodPost, path, "application/json", payload) + rr := backend.request(t, http.MethodPost, path, header, payload) require.Equal(t, 5, backend.relays[0].GetRequestCount(path)) require.JSONEq(t, `{"code":502,"message":"no successful relay response"}`+"\n", rr.Body.String()) require.Equal(t, http.StatusBadGateway, rr.Code, rr.Body.String()) @@ -826,6 +922,9 @@ func denebExecutionPayloadAndBlobsBundle(header *deneb.ExecutionPayloadHeader, k } func TestGetPayloadForks(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + //nolint: forcetypeassert,thelper tests := []struct { fork string @@ -879,7 +978,7 @@ func TestGetPayloadForks(t *testing.T) { // Prepare getPayload response backend.relays[0].GetPayloadResponse = blindedBlockToBlockResponse(signedBlindedBeaconBlock) // call getPayload, ensure it's only called on relay 0 (origin of the bid) - rr := backend.request(t, http.MethodPost, params.PathGetPayload, "application/json", signedBlindedBeaconBlock) + rr := backend.request(t, http.MethodPost, params.PathGetPayload, header, signedBlindedBeaconBlock) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(params.PathGetPayload)) resp := new(builderApi.VersionedSubmitBlindedBlockResponse) @@ -892,6 +991,9 @@ func TestGetPayloadForks(t *testing.T) { } func TestGetPayloadToAllRelays(t *testing.T) { + header := make(http.Header) + header.Set("Accept", "application/json") + // Load the signed blinded beacon block used for getPayload jsonFile, err := os.Open("../testdata/signed-blinded-beacon-block-deneb.json") require.NoError(t, err) @@ -911,7 +1013,7 @@ func TestGetPayloadToAllRelays(t *testing.T) { "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", spec.DataVersionDeneb, ) - rr := backend.request(t, http.MethodGet, getHeaderPath, "application/json", nil) + rr := backend.request(t, http.MethodGet, getHeaderPath, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(getHeaderPath)) require.Equal(t, 1, backend.relays[1].GetRequestCount(getHeaderPath)) @@ -920,7 +1022,7 @@ func TestGetPayloadToAllRelays(t *testing.T) { backend.relays[0].GetPayloadResponse = blindedBlockToBlockResponse(signedBlindedBeaconBlock) // call getPayload, ensure it's called to all relays - rr = backend.request(t, http.MethodPost, params.PathGetPayload, "application/json", signedBlindedBeaconBlock) + rr = backend.request(t, http.MethodPost, params.PathGetPayload, header, signedBlindedBeaconBlock) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(params.PathGetPayload)) require.Equal(t, 1, backend.relays[1].GetRequestCount(params.PathGetPayload)) From 8d1073ff5c84a221c3b14a429eb8a04fdf2b2065 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 16:27:12 -0600 Subject: [PATCH 14/52] Fix lint --- server/mock/mock_relay.go | 42 +++++++++++---------------------------- server/service_test.go | 1 - 2 files changed, 12 insertions(+), 31 deletions(-) diff --git a/server/mock/mock_relay.go b/server/mock/mock_relay.go index 6791107c..3313d513 100644 --- a/server/mock/mock_relay.go +++ b/server/mock/mock_relay.go @@ -265,52 +265,34 @@ func (m *Relay) handleGetHeader(w http.ResponseWriter, req *http.Request) { // defaultHandleGetHeader returns the default handler for handleGetHeader func (m *Relay) defaultHandleGetHeader(w http.ResponseWriter, req *http.Request) { + // Build the default response. + response := m.MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + if m.GetHeaderResponse != nil { + response = m.GetHeaderResponse + } + if req.Header.Get("Accept") == "application/octet-stream" { w.Header().Set("Content-Type", "application/octet-stream") w.WriteHeader(http.StatusOK) - - // Build the default response. - response := m.MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - if m.GetHeaderResponse != nil { - response = m.GetHeaderResponse - } - - // Encode response to SSZ sszData, err := response.Deneb.MarshalSSZ() if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - - // Write the version and data _, err = w.Write(sszData) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } } else { - // By default, return JSON w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusOK) - - // Build the default response. - response := m.MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - if m.GetHeaderResponse != nil { - response = m.GetHeaderResponse - } - if err := json.NewEncoder(w).Encode(response); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/server/service_test.go b/server/service_test.go index 7fb840fb..23725d43 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -339,7 +339,6 @@ func TestGetHeader(t *testing.T) { require.EqualValues(t, *resp.Deneb, *bid) rr.Body.Bytes() - }) t.Run("Bad response from relays", func(t *testing.T) { From 5342231892422b1721ef8f99d099f4e14ae4cd93 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 16:50:21 -0600 Subject: [PATCH 15/52] Address review feedback --- server/get_header.go | 50 +++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index e1cb8154..a370928e 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -51,6 +51,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa "msIntoSlot": msIntoSlot, }).Infof("getHeader request start - %d milliseconds into slot %d", msIntoSlot, slot) + // Get the optional version, used with SSZ decoding + ethConsensusVersion := header.Get("Eth-Consensus-Version") + log = log.WithField("ethConsensusVersion", ethConsensusVersion) + var ( mu sync.Mutex wg sync.WaitGroup @@ -72,53 +76,57 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa url := relay.GetURI(fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", slot, parentHashHex, pubkey)) log := log.WithField("url", url) - // Build the new request - req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) - if err != nil { - log.WithError(err).Warn("error creating new request") - return - } - - // Extend the request header with our values - for key, values := range header { - req.Header[key] = values + // A method for sending the request with a specific accept header value + doRequest := func(accept string) (*http.Response, error) { + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + log.WithError(err).Warn("error creating new request") + return nil, err + } + for key, values := range header { + req.Header[key] = values + } + req.Header.Set("Accept", accept) + return m.httpClientGetHeader.Do(req) } - // Get the optional version, used with SSZ decoding - ethConsensusVersion := header.Get("Eth-Consensus-Version") - log = log.WithField("ethConsensusVersion", ethConsensusVersion) - // Send the get bid request to the relay. Try what the client // accepts (either SSZ or JSON) first. If no accept type is specified, // request JSON. We cannot request SSZ if the client does not, because // the appropriate Eth-Consensus-Version header value is not known. + var err error var resp *http.Response - acceptFromClient := req.Header.Get("Accept") - switch acceptFromClient { + switch header.Get("Accept") { case "application/octet-stream": log.Debug("requesting header in SSZ") - req.Header.Set("Accept", "application/octet-stream") - resp, err = m.httpClientGetHeader.Do(req) + resp, err = doRequest("application/octet-stream") + if err != nil { + log.WithError(err).Warn("error sending request") + return + } + defer resp.Body.Close() + + // Check if the relay supports SSZ requests if resp.StatusCode != http.StatusNotAcceptable { // The relay didn't complain about the accept value. // This means we should try processing the response. log.Debug("response indicated SSZ is accepted") break } + // The response status was NotAcceptable. // This means we should try again with JSON. log.Debug("response indicated SSZ is not accepted") fallthrough default: log.Debug("requesting header in JSON") - req.Header.Set("Accept", "application/json") - resp, err = m.httpClientGetHeader.Do(req) + resp, err = doRequest("application/json") + defer resp.Body.Close() } if err != nil { log.WithError(err).Warn("error calling getHeader on relay") return } - defer resp.Body.Close() // Check if no header is available if resp.StatusCode == http.StatusNoContent { From db848420fe00b5a12a1383e6db401ffbd04fc8c4 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 16:53:31 -0600 Subject: [PATCH 16/52] Fix double close --- server/get_header.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index a370928e..0839fb75 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -104,10 +104,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa log.WithError(err).Warn("error sending request") return } - defer resp.Body.Close() // Check if the relay supports SSZ requests if resp.StatusCode != http.StatusNotAcceptable { + resp.Body.Close() // The relay didn't complain about the accept value. // This means we should try processing the response. log.Debug("response indicated SSZ is accepted") @@ -121,12 +121,12 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa default: log.Debug("requesting header in JSON") resp, err = doRequest("application/json") - defer resp.Body.Close() } if err != nil { log.WithError(err).Warn("error calling getHeader on relay") return } + defer resp.Body.Close() // Check if no header is available if resp.StatusCode == http.StatusNoContent { From 1e5ff2983cf60037f5d72bf2bb374125eeb8fe2e Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 16:57:00 -0600 Subject: [PATCH 17/52] Use consistent log message --- server/get_header.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/get_header.go b/server/get_header.go index 0839fb75..08b95d3c 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -101,7 +101,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa log.Debug("requesting header in SSZ") resp, err = doRequest("application/octet-stream") if err != nil { - log.WithError(err).Warn("error sending request") + log.WithError(err).Warn("error calling getHeader on relay") return } From 357212a6fb1e79853678ab343d8acab8d73c1491 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 17:08:07 -0600 Subject: [PATCH 18/52] Fix body close --- server/get_header.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/get_header.go b/server/get_header.go index 08b95d3c..5d5508b7 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -107,7 +107,6 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // Check if the relay supports SSZ requests if resp.StatusCode != http.StatusNotAcceptable { - resp.Body.Close() // The relay didn't complain about the accept value. // This means we should try processing the response. log.Debug("response indicated SSZ is accepted") @@ -117,6 +116,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // The response status was NotAcceptable. // This means we should try again with JSON. log.Debug("response indicated SSZ is not accepted") + resp.Body.Close() fallthrough default: log.Debug("requesting header in JSON") From 8e5b644835eab121539c39d81784f2c3c02ee0c9 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 18:40:50 -0600 Subject: [PATCH 19/52] One request per relay --- server/get_header.go | 56 ++++++++++---------------------------------- 1 file changed, 13 insertions(+), 43 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 5d5508b7..204dcea1 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -76,52 +76,22 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa url := relay.GetURI(fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", slot, parentHashHex, pubkey)) log := log.WithField("url", url) - // A method for sending the request with a specific accept header value - doRequest := func(accept string) (*http.Response, error) { - req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) - if err != nil { - log.WithError(err).Warn("error creating new request") - return nil, err - } - for key, values := range header { - req.Header[key] = values - } - req.Header.Set("Accept", accept) - return m.httpClientGetHeader.Do(req) + // Make a new request + req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) + if err != nil { + log.WithError(err).Warn("error creating new request") + return } - // Send the get bid request to the relay. Try what the client - // accepts (either SSZ or JSON) first. If no accept type is specified, - // request JSON. We cannot request SSZ if the client does not, because - // the appropriate Eth-Consensus-Version header value is not known. - var err error - var resp *http.Response - switch header.Get("Accept") { - case "application/octet-stream": - log.Debug("requesting header in SSZ") - resp, err = doRequest("application/octet-stream") - if err != nil { - log.WithError(err).Warn("error calling getHeader on relay") - return - } - - // Check if the relay supports SSZ requests - if resp.StatusCode != http.StatusNotAcceptable { - // The relay didn't complain about the accept value. - // This means we should try processing the response. - log.Debug("response indicated SSZ is accepted") - break - } - - // The response status was NotAcceptable. - // This means we should try again with JSON. - log.Debug("response indicated SSZ is not accepted") - resp.Body.Close() - fallthrough - default: - log.Debug("requesting header in JSON") - resp, err = doRequest("application/json") + // Add headers from the request to this request. + // This includes Accept and Eth-Consensus-Version, if provided. + for key, values := range header { + req.Header[key] = values } + + // Send the request + log.Debug("requesting header") + resp, err := m.httpClientGetHeader.Do(req) if err != nil { log.WithError(err).Warn("error calling getHeader on relay") return From 5014440ae02cc4ff35e7515d605870a4efa9cad5 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 19:03:28 -0600 Subject: [PATCH 20/52] Respond with client's favorite except --- server/service.go | 90 ++++++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/server/service.go b/server/service.go index 58d0453a..3f14e545 100644 --- a/server/service.go +++ b/server/service.go @@ -38,6 +38,7 @@ var ( errInvalidSlot = errors.New("invalid slot") errInvalidHash = errors.New("invalid hash") errInvalidPubkey = errors.New("invalid pubkey") + errUnknownAcceptValue = errors.New("unknown accept value") errNoSuccessfulRelayResponse = errors.New("no successful relay response") errServerAlreadyRunning = errors.New("server already running") ) @@ -300,6 +301,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) ua = UserAgent(req.Header.Get("User-Agent")) ) + // Parse the slot slotValue, err := strconv.ParseUint(vars["slot"], 10, 64) if err != nil { m.respondError(w, http.StatusBadRequest, errInvalidSlot.Error()) @@ -307,6 +309,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) } slot := phase0.Slot(slotValue) + // log := m.log.WithFields(logrus.Fields{ "method": "getHeader", "slot": slot, @@ -314,7 +317,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "pubkey": pubkey, "ua": ua, }) - log.Debug("getHeader") + log.Debug("handling request") // Additional header fields header := req.Header @@ -328,6 +331,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) return } + // Bail if none of the relays returned a bid if result.response.IsEmpty() { log.Info("no bid received") w.WriteHeader(http.StatusNoContent) @@ -340,12 +344,12 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) m.bidsLock.Unlock() // How should we respond to the client - acceptFromClient := req.Header.Get("Accept") + clientAccepts := ParseAcceptHeader(req.Header.Get("Accept")) + log.Debug("clientAccepts", clientAccepts) // Log result valueEth := weiBigIntToEthBigFloat(result.bidInfo.value.ToBig()) log.WithFields(logrus.Fields{ - "acceptType": acceptFromClient, "blockHash": result.bidInfo.blockHash.String(), "blockNumber": result.bidInfo.blockNumber, "txRoot": result.bidInfo.txRoot.String(), @@ -354,44 +358,56 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) }).Info("best bid") // Return the bid - switch acceptFromClient { - case "application/octet-stream": - w.Header().Set("Content-Type", "application/octet-stream") - w.WriteHeader(http.StatusOK) - - // Serialize the response - var sszData []byte - switch result.response.Version { - case spec.DataVersionBellatrix: - sszData, err = result.response.Bellatrix.MarshalSSZ() - case spec.DataVersionCapella: - sszData, err = result.response.Capella.MarshalSSZ() - case spec.DataVersionDeneb: - sszData, err = result.response.Deneb.MarshalSSZ() - case spec.DataVersionElectra: - sszData, err = result.response.Electra.MarshalSSZ() - case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: - err = errInvalidForkVersion - } - if err != nil { - m.log.WithError(err).Error("error serializing response as SSZ") - http.Error(w, "failed to serialize response", http.StatusInternalServerError) + for _, accept := range clientAccepts { + if accept.MediaType == MediaTypeOctetStream { + w.Header().Set("Content-Type", MediaTypeOctetStream) + w.WriteHeader(http.StatusOK) + + // Serialize the response + var sszData []byte + switch result.response.Version { + case spec.DataVersionBellatrix: + sszData, err = result.response.Bellatrix.MarshalSSZ() + case spec.DataVersionCapella: + sszData, err = result.response.Capella.MarshalSSZ() + case spec.DataVersionDeneb: + sszData, err = result.response.Deneb.MarshalSSZ() + case spec.DataVersionElectra: + sszData, err = result.response.Electra.MarshalSSZ() + case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: + err = errInvalidForkVersion + } + if err != nil { + m.log.WithError(err).Error("error serializing response as SSZ") + http.Error(w, "failed to serialize response", http.StatusInternalServerError) + return + } + + // Write SSZ data + if _, err := w.Write(sszData); err != nil { + m.log.WithError(err).Error("error writing SSZ response") + http.Error(w, "failed to write response", http.StatusInternalServerError) + } + + // We're done here, return return - } + } else if accept.MediaType == MediaTypeJSON { + w.Header().Set("Content-Type", MediaTypeJSON) + w.WriteHeader(http.StatusOK) + + // Serialize and write the data + if err := json.NewEncoder(w).Encode(&result.response); err != nil { + m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") + http.Error(w, "", http.StatusInternalServerError) + } - // Write SSZ data - if _, err := w.Write(sszData); err != nil { - m.log.WithError(err).Error("error writing SSZ response") - http.Error(w, "failed to write response", http.StatusInternalServerError) - } - default: - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - if err := json.NewEncoder(w).Encode(&result.response); err != nil { - m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") - http.Error(w, "", http.StatusInternalServerError) + // We're done here, return + return } } + + // If this is reached, none of the client's accept values were valid + m.respondError(w, http.StatusNotAcceptable, errUnknownAcceptValue.Error()) } // respondPayload responds to the proposer with the payload From 6171a13438794e9f4a38e5f79894a97820bc1dda Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 19:03:55 -0600 Subject: [PATCH 21/52] Add accept file --- server/accept.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 server/accept.go diff --git a/server/accept.go b/server/accept.go new file mode 100644 index 00000000..b64008c5 --- /dev/null +++ b/server/accept.go @@ -0,0 +1,45 @@ +package server + +import ( + "sort" + "strconv" + "strings" +) + +const ( + MediaTypeJSON = "application/json" + MediaTypeOctetStream = "application/octet-stream" +) + +// AcceptEntry represents a parsed Accept header entry with q-weight. +type AcceptEntry struct { + MediaType string + QValue float64 +} + +// ParseAcceptHeader parses and sorts the Accept header by q-values. +func ParseAcceptHeader(header string) []AcceptEntry { + rawAcceptValues := strings.Split(header, ",") + entries := make([]AcceptEntry, 0, len(rawAcceptValues)) + + for _, part := range rawAcceptValues { + mediaQ := strings.Split(strings.TrimSpace(part), ";") + mediaType := mediaQ[0] + qValue := 1.0 // Default q-value if not specified + + if len(mediaQ) > 1 && strings.HasPrefix(mediaQ[1], "q=") { + if q, err := strconv.ParseFloat(strings.TrimPrefix(mediaQ[1], "q="), 64); err == nil { + qValue = q + } + } + + entries = append(entries, AcceptEntry{MediaType: mediaType, QValue: qValue}) + } + + // Sort by q-value (highest first) + sort.Slice(entries, func(i, j int) bool { + return entries[i].QValue > entries[j].QValue + }) + + return entries +} From a860bf23e8530acec5c2cac46a9f927901458374 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 19:05:06 -0600 Subject: [PATCH 22/52] Move content types to new file --- server/content_types.go | 1 + 1 file changed, 1 insertion(+) create mode 100644 server/content_types.go diff --git a/server/content_types.go b/server/content_types.go new file mode 100644 index 00000000..abb4e431 --- /dev/null +++ b/server/content_types.go @@ -0,0 +1 @@ +package server From 5761355326ad1fd4582dda2a4d6134780fbf2b60 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 19:23:52 -0600 Subject: [PATCH 23/52] Default respond with JSON --- server/accept.go | 5 --- server/content_types.go | 5 +++ server/get_header.go | 4 +- server/service.go | 95 ++++++++++++++++++++++------------------- server/service_test.go | 62 ++++++++++++++++++++++++++- 5 files changed, 119 insertions(+), 52 deletions(-) diff --git a/server/accept.go b/server/accept.go index b64008c5..99e12f2c 100644 --- a/server/accept.go +++ b/server/accept.go @@ -6,11 +6,6 @@ import ( "strings" ) -const ( - MediaTypeJSON = "application/json" - MediaTypeOctetStream = "application/octet-stream" -) - // AcceptEntry represents a parsed Accept header entry with q-weight. type AcceptEntry struct { MediaType string diff --git a/server/content_types.go b/server/content_types.go index abb4e431..be414971 100644 --- a/server/content_types.go +++ b/server/content_types.go @@ -1 +1,6 @@ package server + +const ( + MediaTypeJSON = "application/json" + MediaTypeOctetStream = "application/octet-stream" +) diff --git a/server/get_header.go b/server/get_header.go index 204dcea1..2920992a 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -240,7 +240,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // decodeBid decodes a bid by SSZ or JSON, depending on the provided respContentType func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bid *builderSpec.VersionedSignedBuilderBid) error { switch respContentType { - case "application/octet-stream": + case MediaTypeOctetStream: if ethConsensusVersion != "" { // Do SSZ decoding switch ethConsensusVersion { @@ -266,7 +266,7 @@ func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bi } else { return types.ErrMissingEthConsensusVersion } - case "application/json": + case MediaTypeJSON: // Do JSON decoding return json.Unmarshal(respBytes, bid) } diff --git a/server/service.go b/server/service.go index 3f14e545..485c26f9 100644 --- a/server/service.go +++ b/server/service.go @@ -38,7 +38,6 @@ var ( errInvalidSlot = errors.New("invalid slot") errInvalidHash = errors.New("invalid hash") errInvalidPubkey = errors.New("invalid pubkey") - errUnknownAcceptValue = errors.New("unknown accept value") errNoSuccessfulRelayResponse = errors.New("no successful relay response") errServerAlreadyRunning = errors.New("server already running") ) @@ -357,57 +356,65 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "relays": strings.Join(types.RelayEntriesToStrings(result.relays), ", "), }).Info("best bid") - // Return the bid - for _, accept := range clientAccepts { - if accept.MediaType == MediaTypeOctetStream { - w.Header().Set("Content-Type", MediaTypeOctetStream) - w.WriteHeader(http.StatusOK) - - // Serialize the response - var sszData []byte - switch result.response.Version { - case spec.DataVersionBellatrix: - sszData, err = result.response.Bellatrix.MarshalSSZ() - case spec.DataVersionCapella: - sszData, err = result.response.Capella.MarshalSSZ() - case spec.DataVersionDeneb: - sszData, err = result.response.Deneb.MarshalSSZ() - case spec.DataVersionElectra: - sszData, err = result.response.Electra.MarshalSSZ() - case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: - err = errInvalidForkVersion - } - if err != nil { - m.log.WithError(err).Error("error serializing response as SSZ") - http.Error(w, "failed to serialize response", http.StatusInternalServerError) - return - } + // A function which responds in the JSON encoding + respondJSON := func() { + w.Header().Set("Content-Type", MediaTypeJSON) + w.WriteHeader(http.StatusOK) - // Write SSZ data - if _, err := w.Write(sszData); err != nil { - m.log.WithError(err).Error("error writing SSZ response") - http.Error(w, "failed to write response", http.StatusInternalServerError) - } + // Serialize and write the data + if err := json.NewEncoder(w).Encode(&result.response); err != nil { + m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") + http.Error(w, "", http.StatusInternalServerError) + } + } - // We're done here, return + // A function which responds in the SSZ encoding + respondSSZ := func() { + w.Header().Set("Content-Type", MediaTypeOctetStream) + w.WriteHeader(http.StatusOK) + + // Serialize the response + var sszData []byte + switch result.response.Version { + case spec.DataVersionBellatrix: + sszData, err = result.response.Bellatrix.MarshalSSZ() + case spec.DataVersionCapella: + sszData, err = result.response.Capella.MarshalSSZ() + case spec.DataVersionDeneb: + sszData, err = result.response.Deneb.MarshalSSZ() + case spec.DataVersionElectra: + sszData, err = result.response.Electra.MarshalSSZ() + case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: + err = errInvalidForkVersion + } + if err != nil { + m.log.WithError(err).Error("error serializing response as SSZ") + http.Error(w, "failed to serialize response", http.StatusInternalServerError) return - } else if accept.MediaType == MediaTypeJSON { - w.Header().Set("Content-Type", MediaTypeJSON) - w.WriteHeader(http.StatusOK) - - // Serialize and write the data - if err := json.NewEncoder(w).Encode(&result.response); err != nil { - m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") - http.Error(w, "", http.StatusInternalServerError) - } + } + + // Write SSZ data + if _, err := w.Write(sszData); err != nil { + m.log.WithError(err).Error("error writing SSZ response") + http.Error(w, "failed to write response", http.StatusInternalServerError) + } + } - // We're done here, return + // Return the bid. We iterate over the client's acceptable + // media types in order of highest to lowest quality. + for _, accept := range clientAccepts { + switch accept.MediaType { + case MediaTypeJSON: + respondJSON() + return + case MediaTypeOctetStream: + respondSSZ() return } } - // If this is reached, none of the client's accept values were valid - m.respondError(w, http.StatusNotAcceptable, errUnknownAcceptValue.Error()) + // If the accept value is unknown, respond with JSON + respondJSON() } // respondPayload responds to the proposer with the payload diff --git a/server/service_test.go b/server/service_test.go index 23725d43..3497e1f8 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -332,13 +332,73 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) bid := new(builderApiDeneb.SignedBuilderBid) err := bid.UnmarshalSSZ(rr.Body.Bytes()) require.NoError(t, err) require.EqualValues(t, *resp.Deneb, *bid) + }) + + t.Run("Okay response from relay deneb accepts with q values", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + + backend := newTestBackend(t, 1, time.Second) + resp := backend.relays[0].MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + backend.relays[0].GetHeaderResponse = resp + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + }) + + t.Run("Okay response from relay deneb no accept value", func(t *testing.T) { + // This should default to JSON + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Del("Accept") + + backend := newTestBackend(t, 1, time.Second) + resp := backend.relays[0].MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + backend.relays[0].GetHeaderResponse = resp + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + }) + + t.Run("Okay response from relay deneb client prefers json", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream;q=0.9,application/json;q=1.0") - rr.Body.Bytes() + backend := newTestBackend(t, 1, time.Second) + resp := backend.relays[0].MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + backend.relays[0].GetHeaderResponse = resp + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, "application/json", rr.Header().Get("Content-Type")) }) t.Run("Bad response from relays", func(t *testing.T) { From e7d51dec21cfd2b508e36731ef3801c097fee76d Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 20:30:46 -0600 Subject: [PATCH 24/52] Fix nits --- server/{content_types.go => media_types.go} | 0 server/service.go | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename server/{content_types.go => media_types.go} (100%) diff --git a/server/content_types.go b/server/media_types.go similarity index 100% rename from server/content_types.go rename to server/media_types.go diff --git a/server/service.go b/server/service.go index 485c26f9..d76b0ec4 100644 --- a/server/service.go +++ b/server/service.go @@ -308,7 +308,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) } slot := phase0.Slot(slotValue) - // + // Add relevant fields to the logger log := m.log.WithFields(logrus.Fields{ "method": "getHeader", "slot": slot, From 25462d69001c807f6afe4f0803854f34ba72c68d Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 6 Feb 2025 12:49:52 -0600 Subject: [PATCH 25/52] Add SupportsSSZ field to RelayEntry --- server/get_header.go | 8 +++++++- server/types/relay_entry.go | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 2920992a..5e7ff390 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -204,8 +204,14 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa mu.Lock() defer mu.Unlock() + // Create a copy of the relay instance with its encoding preference. If we request SSZ and the relay + // responds with JSON, we know that it does not support SSZ yet. This preference will be used in getPayload, + // because we must encode the blinded block in the request in such a way that the relay can decode it. + relayWithEncodingPreference := relay.Copy() + relayWithEncodingPreference.SupportsSSZ = respContentType == MediaTypeOctetStream + // Remember which relays delivered which bids (multiple relays might deliver the top bid) - relays[BlockHashHex(bidInfo.blockHash.String())] = append(relays[BlockHashHex(bidInfo.blockHash.String())], relay) + relays[BlockHashHex(bidInfo.blockHash.String())] = append(relays[BlockHashHex(bidInfo.blockHash.String())], relayWithEncodingPreference) // Compare the bid with already known top bid (if any) if !result.response.IsEmpty() { diff --git a/server/types/relay_entry.go b/server/types/relay_entry.go index 3ad7ac93..f2e37c11 100644 --- a/server/types/relay_entry.go +++ b/server/types/relay_entry.go @@ -10,8 +10,9 @@ import ( // RelayEntry represents a relay that mev-boost connects to. type RelayEntry struct { - PublicKey phase0.BLSPubKey - URL *url.URL + PublicKey phase0.BLSPubKey + URL *url.URL + SupportsSSZ bool } func (r *RelayEntry) String() string { @@ -72,3 +73,14 @@ func RelayEntriesToStrings(relays []RelayEntry) []string { } return ret } + +// Copy returns a deep copy of the relay entry. +func (r *RelayEntry) Copy() (ret RelayEntry) { + ret.PublicKey = r.PublicKey + if r.URL != nil { + urlCopy := *r.URL + ret.URL = &urlCopy + } + ret.SupportsSSZ = r.SupportsSSZ + return +} From 3f50e919b74a978b14bde60572db884ca2ac4c98 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 6 Feb 2025 13:36:35 -0600 Subject: [PATCH 26/52] Improve media type handling --- server/accept.go | 137 ++++++++++++++++++++++++++++++++++-------- server/accept_test.go | 50 +++++++++++++++ server/service.go | 108 ++++++++++++++++----------------- 3 files changed, 215 insertions(+), 80 deletions(-) create mode 100644 server/accept_test.go diff --git a/server/accept.go b/server/accept.go index 99e12f2c..f57c4ba0 100644 --- a/server/accept.go +++ b/server/accept.go @@ -1,40 +1,125 @@ package server import ( - "sort" "strconv" "strings" ) -// AcceptEntry represents a parsed Accept header entry with q-weight. -type AcceptEntry struct { - MediaType string - QValue float64 +type acceptEntry struct { + mediaRange string + q float64 + pos int // position in the header (lower=earlier) } -// ParseAcceptHeader parses and sorts the Accept header by q-values. -func ParseAcceptHeader(header string) []AcceptEntry { - rawAcceptValues := strings.Split(header, ",") - entries := make([]AcceptEntry, 0, len(rawAcceptValues)) - - for _, part := range rawAcceptValues { - mediaQ := strings.Split(strings.TrimSpace(part), ";") - mediaType := mediaQ[0] - qValue := 1.0 // Default q-value if not specified - - if len(mediaQ) > 1 && strings.HasPrefix(mediaQ[1], "q=") { - if q, err := strconv.ParseFloat(strings.TrimPrefix(mediaQ[1], "q="), 64); err == nil { - qValue = q +// ParseAcceptHeader takes an Accept header string and returns a slice of valid entries. +// It splits the header by commas and for each part, it splits by semicolon to find +// a possible "q" parameter. If the part is malformed (for example, a trailing semicolon +// with no parameter or a q value that cannot be parsed or is not in [0,1]), the entry is ignored. +func ParseAcceptHeader(header string) []acceptEntry { + var entries []acceptEntry + parts := strings.Split(header, ",") + for i, part := range parts { + part = strings.TrimSpace(part) + if part == "" { + continue + } + // Split on semicolon; first token is the media-range. + tokens := strings.Split(part, ";") + mediaRange := strings.TrimSpace(tokens[0]) + if mediaRange == "" { + continue + } + // Default quality is 1. + q := 1.0 + valid := true + // Process parameters. + for _, token := range tokens[1:] { + token = strings.TrimSpace(token) + // If a parameter is empty (for example, a trailing semicolon) then consider the entry malformed. + if token == "" { + valid = false + break + } + // Look for a "q" parameter. + if strings.HasPrefix(token, "q=") { + kv := strings.SplitN(token, "=", 2) + if len(kv) != 2 { + valid = false + break + } + value := strings.TrimSpace(kv[1]) + f, err := strconv.ParseFloat(value, 64) + if err != nil || f < 0 || f > 1 { + valid = false + break + } + q = f } + // Other parameters are ignored. } - - entries = append(entries, AcceptEntry{MediaType: mediaType, QValue: qValue}) + if !valid { + continue + } + entries = append(entries, acceptEntry{ + mediaRange: mediaRange, + q: q, + pos: i, + }) } - - // Sort by q-value (highest first) - sort.Slice(entries, func(i, j int) bool { - return entries[i].QValue > entries[j].QValue - }) - return entries } + +// SelectHighestQualityValueMediaType takes the parsed Accept header entries (from ParseAcceptHeader) +// and a slice of supported media types, then returns the one with the highest quality factor. +// A supported type is considered matching if it is an exact match or if the accept entry is "*/*". +// When quality factors are equal, the later (higher pos) accept entry “wins” – and if even that is tied, +// the order of supportedMediaTypes is used as a final tie‐breaker. +func SelectHighestQualityValueMediaType(entries []acceptEntry, supportedMediaTypes []string, defaultMediaType string) string { + type candidate struct { + mediaType string + q float64 + pos int // position of the matching accept entry + index int // the index in the supportedMediaTypes slice + } + var bestCandidate *candidate + // For each supported media type, find its best matching accept entry. + for idx, supp := range supportedMediaTypes { + bestQ := -1.0 + bestPos := -1 + found := false + for _, e := range entries { + if e.mediaRange == supp || e.mediaRange == "*/*" { + // If this entry has a higher q or, in case of equal q, a later position, + // then it is preferred. + if e.q > bestQ || (e.q == bestQ && e.pos > bestPos) { + bestQ = e.q + bestPos = e.pos + found = true + } + } + } + if found { + cand := candidate{ + mediaType: supp, + q: bestQ, + pos: bestPos, + index: idx, + } + if bestCandidate == nil { + bestCandidate = &cand + } else { + // Compare candidates: higher q wins; if equal, then later accept header position wins; + // if still equal, then the supportedMediaTypes order (lower index wins). + if cand.q > bestCandidate.q || + (cand.q == bestCandidate.q && cand.pos > bestCandidate.pos) || + (cand.q == bestCandidate.q && cand.pos == bestCandidate.pos && cand.index < bestCandidate.index) { + bestCandidate = &cand + } + } + } + } + if bestCandidate != nil { + return bestCandidate.mediaType + } + return defaultMediaType +} diff --git a/server/accept_test.go b/server/accept_test.go new file mode 100644 index 00000000..f3f5fead --- /dev/null +++ b/server/accept_test.go @@ -0,0 +1,50 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseAcceptHeader(t *testing.T) { + testCases := []struct { + header string + expected string + }{ + {"", MediaTypeJSON}, + {"*/*", MediaTypeJSON}, + {"application/json", MediaTypeJSON}, + {"application/octet-stream", MediaTypeOctetStream}, + {"application/invalid", MediaTypeJSON}, + {"application/invalid;q=1,application/octet-stream;q=0.1", MediaTypeOctetStream}, + {"application/octet-stream;q=0.5,application/json;q=1", MediaTypeJSON}, + {"application/octet-stream;q=1,application/json;q=0.1", MediaTypeOctetStream}, + {"application/octet-stream;q=1,application/json;q=0.9", MediaTypeOctetStream}, + {"application/octet-stream;q=1,*/*;q=0.9", MediaTypeOctetStream}, + {"application/octet-stream,application/json;q=0.1", MediaTypeOctetStream}, + {"application/octet-stream;,application/json;q=0.1", MediaTypeJSON}, // Malformed entry + {"application/octet-stream;q=2,application/json;q=0.1", MediaTypeJSON}, // Invalid q-value >1, fallback + {"application/octet-stream;q=invalid,application/json;q=0.1", MediaTypeJSON}, // Invalid q-value, ignored + {"application/octet-stream ; q=0.5 , application/json ; q=1", MediaTypeJSON}, + {"application/octet-stream ; q=1 , application/json ; q=0.1", MediaTypeOctetStream}, + {"application/octet-stream;q=1,application/json;q=0.1", MediaTypeOctetStream}, + { + // Default Chrome Accept header, JSON should be preferred + "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7", + MediaTypeJSON, + }, + // Order-dependent tests (last one wins if q-values are the same) + {"application/octet-stream;q=1,application/json;q=1", MediaTypeJSON}, + {"application/json;q=1,application/octet-stream;q=1", MediaTypeOctetStream}, + } + + supportedMediaTypes := []string{MediaTypeJSON, MediaTypeOctetStream} + + for _, tc := range testCases { + t.Run(tc.header, func(t *testing.T) { + parsed := ParseAcceptHeader(tc.header) + selected := SelectHighestQualityValueMediaType(parsed, supportedMediaTypes, MediaTypeJSON) + require.Equal(t, tc.expected, selected) + }) + } +} diff --git a/server/service.go b/server/service.go index d76b0ec4..cb0cbe7f 100644 --- a/server/service.go +++ b/server/service.go @@ -356,65 +356,65 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "relays": strings.Join(types.RelayEntriesToStrings(result.relays), ", "), }).Info("best bid") - // A function which responds in the JSON encoding - respondJSON := func() { - w.Header().Set("Content-Type", MediaTypeJSON) - w.WriteHeader(http.StatusOK) - - // Serialize and write the data - if err := json.NewEncoder(w).Encode(&result.response); err != nil { - m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") - http.Error(w, "", http.StatusInternalServerError) - } - } - - // A function which responds in the SSZ encoding - respondSSZ := func() { - w.Header().Set("Content-Type", MediaTypeOctetStream) - w.WriteHeader(http.StatusOK) - - // Serialize the response - var sszData []byte - switch result.response.Version { - case spec.DataVersionBellatrix: - sszData, err = result.response.Bellatrix.MarshalSSZ() - case spec.DataVersionCapella: - sszData, err = result.response.Capella.MarshalSSZ() - case spec.DataVersionDeneb: - sszData, err = result.response.Deneb.MarshalSSZ() - case spec.DataVersionElectra: - sszData, err = result.response.Electra.MarshalSSZ() - case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: - err = errInvalidForkVersion - } - if err != nil { - m.log.WithError(err).Error("error serializing response as SSZ") - http.Error(w, "failed to serialize response", http.StatusInternalServerError) - return - } + // Define map of supported handlers + supportedMediaTypeHandlers := map[string]func(){ + MediaTypeJSON: func() { + w.Header().Set("Content-Type", MediaTypeJSON) + w.WriteHeader(http.StatusOK) + + // Serialize and write the data + if err := json.NewEncoder(w).Encode(&result.response); err != nil { + m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") + http.Error(w, "", http.StatusInternalServerError) + } + }, + MediaTypeOctetStream: func() { + w.Header().Set("Content-Type", MediaTypeOctetStream) + w.WriteHeader(http.StatusOK) + + // Serialize the response + var sszData []byte + switch result.response.Version { + case spec.DataVersionBellatrix: + sszData, err = result.response.Bellatrix.MarshalSSZ() + case spec.DataVersionCapella: + sszData, err = result.response.Capella.MarshalSSZ() + case spec.DataVersionDeneb: + sszData, err = result.response.Deneb.MarshalSSZ() + case spec.DataVersionElectra: + sszData, err = result.response.Electra.MarshalSSZ() + case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: + err = errInvalidForkVersion + } + if err != nil { + m.log.WithError(err).Error("error serializing response as SSZ") + http.Error(w, "failed to serialize response", http.StatusInternalServerError) + return + } - // Write SSZ data - if _, err := w.Write(sszData); err != nil { - m.log.WithError(err).Error("error writing SSZ response") - http.Error(w, "failed to write response", http.StatusInternalServerError) - } + // Write SSZ data + if _, err := w.Write(sszData); err != nil { + m.log.WithError(err).Error("error writing SSZ response") + http.Error(w, "failed to write response", http.StatusInternalServerError) + } + }, } - // Return the bid. We iterate over the client's acceptable - // media types in order of highest to lowest quality. - for _, accept := range clientAccepts { - switch accept.MediaType { - case MediaTypeJSON: - respondJSON() - return - case MediaTypeOctetStream: - respondSSZ() - return - } + // Generate slice of supported media types + var supportedMediaTypes []string + for mediaType := range supportedMediaTypeHandlers { + supportedMediaTypes = append(supportedMediaTypes, mediaType) } - // If the accept value is unknown, respond with JSON - respondJSON() + // Given the client's acceptable media types, respond with the highest quality one + preferredContentType := SelectHighestQualityValueMediaType(clientAccepts, supportedMediaTypes, MediaTypeJSON) + if mediaTypeHandler, ok := supportedMediaTypeHandlers[preferredContentType]; ok { + mediaTypeHandler() + } else { + // This should never happen, but just in case. + message := fmt.Sprintf("unsupported media type: %s", preferredContentType) + m.respondError(w, http.StatusNotAcceptable, message) + } } // respondPayload responds to the proposer with the payload From 0cde21a3d97c87b15ae79a2f3fcc024ec62f25b0 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 6 Feb 2025 14:58:03 -0600 Subject: [PATCH 27/52] Return NotAcceptable if the only accept type is unsupported --- server/accept.go | 32 ++++++++++++++++---------------- server/accept_test.go | 9 ++++++--- server/get_header.go | 8 ++++++-- server/service.go | 8 ++++++-- server/service_test.go | 18 ++++++++++++++++++ 5 files changed, 52 insertions(+), 23 deletions(-) diff --git a/server/accept.go b/server/accept.go index f57c4ba0..8d6d15b4 100644 --- a/server/accept.go +++ b/server/accept.go @@ -6,9 +6,9 @@ import ( ) type acceptEntry struct { - mediaRange string - q float64 - pos int // position in the header (lower=earlier) + MediaType string + Quality float64 + pos int // position in the header (lower=earlier) } // ParseAcceptHeader takes an Accept header string and returns a slice of valid entries. @@ -25,12 +25,12 @@ func ParseAcceptHeader(header string) []acceptEntry { } // Split on semicolon; first token is the media-range. tokens := strings.Split(part, ";") - mediaRange := strings.TrimSpace(tokens[0]) - if mediaRange == "" { + mediaType := strings.TrimSpace(tokens[0]) + if mediaType == "" { continue } - // Default quality is 1. - q := 1.0 + // Default q is 1. + quality := 1.0 valid := true // Process parameters. for _, token := range tokens[1:] { @@ -53,7 +53,7 @@ func ParseAcceptHeader(header string) []acceptEntry { valid = false break } - q = f + quality = f } // Other parameters are ignored. } @@ -61,9 +61,9 @@ func ParseAcceptHeader(header string) []acceptEntry { continue } entries = append(entries, acceptEntry{ - mediaRange: mediaRange, - q: q, - pos: i, + MediaType: mediaType, + Quality: quality, + pos: i, }) } return entries @@ -74,7 +74,7 @@ func ParseAcceptHeader(header string) []acceptEntry { // A supported type is considered matching if it is an exact match or if the accept entry is "*/*". // When quality factors are equal, the later (higher pos) accept entry “wins” – and if even that is tied, // the order of supportedMediaTypes is used as a final tie‐breaker. -func SelectHighestQualityValueMediaType(entries []acceptEntry, supportedMediaTypes []string, defaultMediaType string) string { +func SelectHighestQualityValueMediaType(entries []acceptEntry, supportedMediaTypes []string) string { type candidate struct { mediaType string q float64 @@ -88,11 +88,11 @@ func SelectHighestQualityValueMediaType(entries []acceptEntry, supportedMediaTyp bestPos := -1 found := false for _, e := range entries { - if e.mediaRange == supp || e.mediaRange == "*/*" { + if e.MediaType == supp || e.MediaType == "*/*" { // If this entry has a higher q or, in case of equal q, a later position, // then it is preferred. - if e.q > bestQ || (e.q == bestQ && e.pos > bestPos) { - bestQ = e.q + if e.Quality > bestQ || (e.Quality == bestQ && e.pos > bestPos) { + bestQ = e.Quality bestPos = e.pos found = true } @@ -121,5 +121,5 @@ func SelectHighestQualityValueMediaType(entries []acceptEntry, supportedMediaTyp if bestCandidate != nil { return bestCandidate.mediaType } - return defaultMediaType + return "" } diff --git a/server/accept_test.go b/server/accept_test.go index f3f5fead..7884623d 100644 --- a/server/accept_test.go +++ b/server/accept_test.go @@ -6,16 +6,19 @@ import ( "github.com/stretchr/testify/require" ) +// TestParseAcceptHeader ensures that parsing/selection works properly. +// These tests come from the Lodestar client (thank you for sharing!) here: +// https://github.com/ChainSafe/lodestar/blob/9c66fac4a47446b225c874997dfc4d7b93a820b3/packages/api/test/unit/utils/headers.test.ts#L5-L42 func TestParseAcceptHeader(t *testing.T) { testCases := []struct { header string expected string }{ - {"", MediaTypeJSON}, + {"", ""}, {"*/*", MediaTypeJSON}, {"application/json", MediaTypeJSON}, {"application/octet-stream", MediaTypeOctetStream}, - {"application/invalid", MediaTypeJSON}, + {"application/invalid", ""}, {"application/invalid;q=1,application/octet-stream;q=0.1", MediaTypeOctetStream}, {"application/octet-stream;q=0.5,application/json;q=1", MediaTypeJSON}, {"application/octet-stream;q=1,application/json;q=0.1", MediaTypeOctetStream}, @@ -43,7 +46,7 @@ func TestParseAcceptHeader(t *testing.T) { for _, tc := range testCases { t.Run(tc.header, func(t *testing.T) { parsed := ParseAcceptHeader(tc.header) - selected := SelectHighestQualityValueMediaType(parsed, supportedMediaTypes, MediaTypeJSON) + selected := SelectHighestQualityValueMediaType(parsed, supportedMediaTypes) require.Equal(t, tc.expected, selected) }) } diff --git a/server/get_header.go b/server/get_header.go index 5e7ff390..23caf02f 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "mime" "net/http" "sync" "time" @@ -118,8 +119,11 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa return } - // Get the response's content type - respContentType := resp.Header.Get("Content-Type") + // Get the response's content type, default to JSON + respContentType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type")) + if err != nil { + respContentType = MediaTypeJSON + } log = log.WithField("respContentType", respContentType) // Decode bid diff --git a/server/service.go b/server/service.go index cb0cbe7f..ee975687 100644 --- a/server/service.go +++ b/server/service.go @@ -345,6 +345,10 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // How should we respond to the client clientAccepts := ParseAcceptHeader(req.Header.Get("Accept")) log.Debug("clientAccepts", clientAccepts) + if len(clientAccepts) == 0 { + log.Info("no client accepts, defaulting to JSON") + clientAccepts = []acceptEntry{{MediaType: MediaTypeJSON}} + } // Log result valueEth := weiBigIntToEthBigFloat(result.bidInfo.value.ToBig()) @@ -406,8 +410,8 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) supportedMediaTypes = append(supportedMediaTypes, mediaType) } - // Given the client's acceptable media types, respond with the highest quality one - preferredContentType := SelectHighestQualityValueMediaType(clientAccepts, supportedMediaTypes, MediaTypeJSON) + // Given the client's acceptable media types, respond with the highest q one + preferredContentType := SelectHighestQualityValueMediaType(clientAccepts, supportedMediaTypes) if mediaTypeHandler, ok := supportedMediaTypeHandlers[preferredContentType]; ok { mediaTypeHandler() } else { diff --git a/server/service_test.go b/server/service_test.go index 3497e1f8..a59ae71b 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -401,6 +401,24 @@ func TestGetHeader(t *testing.T) { require.Equal(t, "application/json", rr.Header().Get("Content-Type")) }) + t.Run("Okay response from relay deneb client accepts media type we do not support", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "plain/text") + + backend := newTestBackend(t, 1, time.Second) + resp := backend.relays[0].MakeGetHeaderResponse( + 12345, + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", + "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", + spec.DataVersionDeneb, + ) + backend.relays[0].GetHeaderResponse = resp + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusNotAcceptable, rr.Code, rr.Body.String()) + }) + t.Run("Bad response from relays", func(t *testing.T) { header := make(http.Header) header.Set("Accept", "application/json") From 4c9f49bd9253f545f159bc6c0927a48741370fe5 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 6 Feb 2025 15:01:00 -0600 Subject: [PATCH 28/52] Fix lint --- server/accept.go | 10 +++++----- server/service.go | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/accept.go b/server/accept.go index 8d6d15b4..206bb90e 100644 --- a/server/accept.go +++ b/server/accept.go @@ -5,7 +5,7 @@ import ( "strings" ) -type acceptEntry struct { +type AcceptEntry struct { MediaType string Quality float64 pos int // position in the header (lower=earlier) @@ -15,9 +15,9 @@ type acceptEntry struct { // It splits the header by commas and for each part, it splits by semicolon to find // a possible "q" parameter. If the part is malformed (for example, a trailing semicolon // with no parameter or a q value that cannot be parsed or is not in [0,1]), the entry is ignored. -func ParseAcceptHeader(header string) []acceptEntry { - var entries []acceptEntry +func ParseAcceptHeader(header string) []AcceptEntry { parts := strings.Split(header, ",") + entries := make([]AcceptEntry, 0, len(parts)) for i, part := range parts { part = strings.TrimSpace(part) if part == "" { @@ -60,7 +60,7 @@ func ParseAcceptHeader(header string) []acceptEntry { if !valid { continue } - entries = append(entries, acceptEntry{ + entries = append(entries, AcceptEntry{ MediaType: mediaType, Quality: quality, pos: i, @@ -74,7 +74,7 @@ func ParseAcceptHeader(header string) []acceptEntry { // A supported type is considered matching if it is an exact match or if the accept entry is "*/*". // When quality factors are equal, the later (higher pos) accept entry “wins” – and if even that is tied, // the order of supportedMediaTypes is used as a final tie‐breaker. -func SelectHighestQualityValueMediaType(entries []acceptEntry, supportedMediaTypes []string) string { +func SelectHighestQualityValueMediaType(entries []AcceptEntry, supportedMediaTypes []string) string { type candidate struct { mediaType string q float64 diff --git a/server/service.go b/server/service.go index ee975687..a0ee5963 100644 --- a/server/service.go +++ b/server/service.go @@ -347,7 +347,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) log.Debug("clientAccepts", clientAccepts) if len(clientAccepts) == 0 { log.Info("no client accepts, defaulting to JSON") - clientAccepts = []acceptEntry{{MediaType: MediaTypeJSON}} + clientAccepts = []AcceptEntry{{MediaType: MediaTypeJSON}} } // Log result @@ -405,7 +405,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) } // Generate slice of supported media types - var supportedMediaTypes []string + supportedMediaTypes := make([]string, 0, len(supportedMediaTypeHandlers)) for mediaType := range supportedMediaTypeHandlers { supportedMediaTypes = append(supportedMediaTypes, mediaType) } From 7c27374100dd761211a890ac2cc6b60c40bb82e4 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 6 Feb 2025 15:01:22 -0600 Subject: [PATCH 29/52] Remove now incorrect comment --- server/service.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/service.go b/server/service.go index a0ee5963..1738dcc1 100644 --- a/server/service.go +++ b/server/service.go @@ -415,7 +415,6 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) if mediaTypeHandler, ok := supportedMediaTypeHandlers[preferredContentType]; ok { mediaTypeHandler() } else { - // This should never happen, but just in case. message := fmt.Sprintf("unsupported media type: %s", preferredContentType) m.respondError(w, http.StatusNotAcceptable, message) } From 78463c4bea0cff67ac34c42da1f7029d5c79bab5 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 11 Feb 2025 14:39:10 -0600 Subject: [PATCH 30/52] Return JSON in getHeader under some circumstances --- server/accept.go | 9 +++++++++ server/service.go | 21 ++++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/server/accept.go b/server/accept.go index 206bb90e..15fc4012 100644 --- a/server/accept.go +++ b/server/accept.go @@ -123,3 +123,12 @@ func SelectHighestQualityValueMediaType(entries []AcceptEntry, supportedMediaTyp } return "" } + +func Accepts(entries []AcceptEntry, mediaType string) bool { + for _, entry := range entries { + if entry.MediaType == mediaType { + return true + } + } + return false +} diff --git a/server/service.go b/server/service.go index 1738dcc1..bf05a7f8 100644 --- a/server/service.go +++ b/server/service.go @@ -410,8 +410,27 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) supportedMediaTypes = append(supportedMediaTypes, mediaType) } - // Given the client's acceptable media types, respond with the highest q one + // Default to the client's highest quality acceptable media type preferredContentType := SelectHighestQualityValueMediaType(clientAccepts, supportedMediaTypes) + + // If every relay returned the bid in JSON, that means that none + // of them support SSZ and this would always require extra conversions. + // In this situation, if the client still accepts JSON, respond to the + // getHeader in JSON so the client sends the getPayload request in JSON. + allBidsWereJSON := true + for _, relay := range result.relays { + if relay.SupportsSSZ { + log.Debug("there is a relay that supports SSZ") + allBidsWereJSON = false + break + } + } + if preferredContentType != MediaTypeJSON && allBidsWereJSON && Accepts(clientAccepts, MediaTypeJSON) { + log.Debug("overriding the response content type to be JSON") + preferredContentType = MediaTypeJSON + } + + // Respond appropriately if mediaTypeHandler, ok := supportedMediaTypeHandlers[preferredContentType]; ok { mediaTypeHandler() } else { From 17810c2bdcd5e8f4274af8ec43871b11f8291aba Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 11 Feb 2025 14:46:34 -0600 Subject: [PATCH 31/52] Include ethConsensusVersion in getHeader response --- server/service.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/service.go b/server/service.go index bf05a7f8..87697e7d 100644 --- a/server/service.go +++ b/server/service.go @@ -373,19 +373,20 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) } }, MediaTypeOctetStream: func() { - w.Header().Set("Content-Type", MediaTypeOctetStream) - w.WriteHeader(http.StatusOK) - // Serialize the response var sszData []byte switch result.response.Version { case spec.DataVersionBellatrix: + w.Header().Set("Eth-Consensus-Version", "bellatrix") sszData, err = result.response.Bellatrix.MarshalSSZ() case spec.DataVersionCapella: + w.Header().Set("Eth-Consensus-Version", "capella") sszData, err = result.response.Capella.MarshalSSZ() case spec.DataVersionDeneb: + w.Header().Set("Eth-Consensus-Version", "deneb") sszData, err = result.response.Deneb.MarshalSSZ() case spec.DataVersionElectra: + w.Header().Set("Eth-Consensus-Version", "electra") sszData, err = result.response.Electra.MarshalSSZ() case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: err = errInvalidForkVersion @@ -396,6 +397,10 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) return } + // Write the header + w.Header().Set("Content-Type", MediaTypeOctetStream) + w.WriteHeader(http.StatusOK) + // Write SSZ data if _, err := w.Write(sszData); err != nil { m.log.WithError(err).Error("error writing SSZ response") From 461ceab02ff0159a1e96fe7fa1edd065501de77e Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 11 Feb 2025 14:47:05 -0600 Subject: [PATCH 32/52] Decode header with ethConsensusVersion from response --- server/get_header.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 23caf02f..76302305 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -52,10 +52,6 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa "msIntoSlot": msIntoSlot, }).Infof("getHeader request start - %d milliseconds into slot %d", msIntoSlot, slot) - // Get the optional version, used with SSZ decoding - ethConsensusVersion := header.Get("Eth-Consensus-Version") - log = log.WithField("ethConsensusVersion", ethConsensusVersion) - var ( mu sync.Mutex wg sync.WaitGroup @@ -126,6 +122,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } log = log.WithField("respContentType", respContentType) + // Get the optional version, used with SSZ decoding + ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + log = log.WithField("ethConsensusVersion", ethConsensusVersion) + // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) From a405b924c3ca7d1c1800f545bde6b3d84c854529 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 11 Feb 2025 15:19:45 -0600 Subject: [PATCH 33/52] Fix & improve tests --- server/mock/mock_relay.go | 37 +++++++++++--- server/service_test.go | 105 ++++++++++++++++++++++---------------- 2 files changed, 90 insertions(+), 52 deletions(-) diff --git a/server/mock/mock_relay.go b/server/mock/mock_relay.go index 3313d513..47ceda65 100644 --- a/server/mock/mock_relay.go +++ b/server/mock/mock_relay.go @@ -6,6 +6,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "sync" "testing" "time" @@ -69,6 +70,10 @@ type Relay struct { // Server section Server *httptest.Server ResponseDelay time.Duration + + // Force response encodings + ForceJSON bool + ForceSSZ bool } // NewRelay creates a mocked relay which implements the backend.BoostBackend interface @@ -277,7 +282,17 @@ func (m *Relay) defaultHandleGetHeader(w http.ResponseWriter, req *http.Request) response = m.GetHeaderResponse } - if req.Header.Get("Accept") == "application/octet-stream" { + respondJSON := func() { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + if err := json.NewEncoder(w).Encode(response); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + } + + respondSSZ := func() { + w.Header().Set("Eth-Consensus-Version", "deneb") w.Header().Set("Content-Type", "application/octet-stream") w.WriteHeader(http.StatusOK) sszData, err := response.Deneb.MarshalSSZ() @@ -290,13 +305,19 @@ func (m *Relay) defaultHandleGetHeader(w http.ResponseWriter, req *http.Request) http.Error(w, err.Error(), http.StatusInternalServerError) return } - } else { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusOK) - if err := json.NewEncoder(w).Encode(response); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } + } + + // We cannot use code in server, so this is a simplistic + // negotiation which should only be used in testing. + switch { + case m.ForceJSON: + respondJSON() + case m.ForceSSZ: + respondSSZ() + case strings.Contains(req.Header.Get("Accept"), "application/octet-stream"): + respondSSZ() + default: + respondJSON() } } diff --git a/server/service_test.go b/server/service_test.go index a59ae71b..301c20b0 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -302,14 +302,6 @@ func TestGetHeader(t *testing.T) { header.Set("Accept", "application/json") backend := newTestBackend(t, 1, time.Second) - resp := backend.relays[0].MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - backend.relays[0].GetHeaderResponse = resp rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) @@ -334,87 +326,112 @@ func TestGetHeader(t *testing.T) { require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + // Ensure the response was SSZ bid := new(builderApiDeneb.SignedBuilderBid) err := bid.UnmarshalSSZ(rr.Body.Bytes()) require.NoError(t, err) require.EqualValues(t, *resp.Deneb, *bid) }) - t.Run("Okay response from relay deneb accepts with q values", func(t *testing.T) { + t.Run("Relay returns SSZ, mev-boost returns SSZ", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + + backend := newTestBackend(t, 1, time.Second) + backend.relays[0].ForceSSZ = true + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + }) + + t.Run("One relay returns SSZ, another relay returns JSON, mev-boost returns SSZ", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + + backend := newTestBackend(t, 2, time.Second) + backend.relays[0].ForceSSZ = true + backend.relays[1].ForceJSON = true + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) + require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + }) + + t.Run("One relay returns JSON, mev-boost returns JSON", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + + backend := newTestBackend(t, 1, time.Second) + backend.relays[0].ForceJSON = true + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + }) + + t.Run("Two relays return JSON, mev-boost returns JSON", func(t *testing.T) { + header := make(http.Header) + header.Set("Eth-Consensus-Version", "deneb") + header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + + backend := newTestBackend(t, 2, time.Second) + backend.relays[0].ForceJSON = true + backend.relays[1].ForceJSON = true + rr := backend.request(t, http.MethodGet, path, header, nil) + require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) + require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) + require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) + require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + }) + + t.Run("Accepts both with Q values", func(t *testing.T) { header := make(http.Header) header.Set("Eth-Consensus-Version", "deneb") header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") backend := newTestBackend(t, 1, time.Second) - resp := backend.relays[0].MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - backend.relays[0].GetHeaderResponse = resp rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) }) - t.Run("Okay response from relay deneb no accept value", func(t *testing.T) { + t.Run("No accept value", func(t *testing.T) { // This should default to JSON header := make(http.Header) header.Set("Eth-Consensus-Version", "deneb") header.Del("Accept") backend := newTestBackend(t, 1, time.Second) - resp := backend.relays[0].MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - backend.relays[0].GetHeaderResponse = resp rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, "application/json", rr.Header().Get("Content-Type")) }) - t.Run("Okay response from relay deneb client prefers json", func(t *testing.T) { + t.Run("Accepts both but prefers JSON", func(t *testing.T) { header := make(http.Header) header.Set("Eth-Consensus-Version", "deneb") header.Set("Accept", "application/octet-stream;q=0.9,application/json;q=1.0") backend := newTestBackend(t, 1, time.Second) - resp := backend.relays[0].MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - backend.relays[0].GetHeaderResponse = resp rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, "application/json", rr.Header().Get("Content-Type")) }) - t.Run("Okay response from relay deneb client accepts media type we do not support", func(t *testing.T) { + t.Run("Only accepts unsupported media type", func(t *testing.T) { header := make(http.Header) header.Set("Eth-Consensus-Version", "deneb") header.Set("Accept", "plain/text") backend := newTestBackend(t, 1, time.Second) - resp := backend.relays[0].MakeGetHeaderResponse( - 12345, - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0xe28385e7bd68df656cd0042b74b69c3104b5356ed1f20eb69f1f925df47a3ab7", - "0x8a1d7b8dd64e0aafe7ea7b6c95065c9364cf99d38470c12ee807d55f7de1529ad29ce2c422e0b65e3d5a05c02caca249", - spec.DataVersionDeneb, - ) - backend.relays[0].GetHeaderResponse = resp rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusNotAcceptable, rr.Code, rr.Body.String()) }) From f607ce36058e9f135927b8afea9e7f378dfc0b59 Mon Sep 17 00:00:00 2001 From: Justin Traglia <95511699+jtraglia@users.noreply.github.com> Date: Thu, 13 Feb 2025 14:41:22 -0600 Subject: [PATCH 34/52] Update comment Co-authored-by: Chris Hager --- server/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/service.go b/server/service.go index 87697e7d..7ec2ccce 100644 --- a/server/service.go +++ b/server/service.go @@ -342,7 +342,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) m.bids[bidKey(slot, result.bidInfo.blockHash)] = result m.bidsLock.Unlock() - // How should we respond to the client + // Decide response content type (JSON by default) clientAccepts := ParseAcceptHeader(req.Header.Get("Accept")) log.Debug("clientAccepts", clientAccepts) if len(clientAccepts) == 0 { From 1ff636c0f2881f337d87e524806aa7e220088a30 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 13 Feb 2025 15:01:46 -0600 Subject: [PATCH 35/52] Do away with function map --- server/get_header.go | 50 +++++++++++++++++++++++++++++++++++ server/service.go | 63 ++++---------------------------------------- 2 files changed, 55 insertions(+), 58 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 76302305..fb959afb 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -282,3 +282,53 @@ func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bi } return types.ErrInvalidContentType } + +// respondGetHeaderJSON responds to the client in JSON +func (m *BoostService) respondGetHeaderJSON(w http.ResponseWriter, result *bidResp) { + w.Header().Set("Content-Type", MediaTypeJSON) + w.WriteHeader(http.StatusOK) + + // Serialize and write the data + if err := json.NewEncoder(w).Encode(&result.response); err != nil { + m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") + http.Error(w, "", http.StatusInternalServerError) + } +} + +// respondGetHeaderSSZ responds to the client in SSZ +func (m *BoostService) respondGetHeaderSSZ(w http.ResponseWriter, result *bidResp) { + // Serialize the response + var err error + var sszData []byte + switch result.response.Version { + case spec.DataVersionBellatrix: + w.Header().Set("Eth-Consensus-Version", "bellatrix") + sszData, err = result.response.Bellatrix.MarshalSSZ() + case spec.DataVersionCapella: + w.Header().Set("Eth-Consensus-Version", "capella") + sszData, err = result.response.Capella.MarshalSSZ() + case spec.DataVersionDeneb: + w.Header().Set("Eth-Consensus-Version", "deneb") + sszData, err = result.response.Deneb.MarshalSSZ() + case spec.DataVersionElectra: + w.Header().Set("Eth-Consensus-Version", "electra") + sszData, err = result.response.Electra.MarshalSSZ() + case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: + err = errInvalidForkVersion + } + if err != nil { + m.log.WithError(err).Error("error serializing response as SSZ") + http.Error(w, "failed to serialize response", http.StatusInternalServerError) + return + } + + // Write the header + w.Header().Set("Content-Type", MediaTypeOctetStream) + w.WriteHeader(http.StatusOK) + + // Write SSZ data + if _, err := w.Write(sszData); err != nil { + m.log.WithError(err).Error("error writing SSZ response") + http.Error(w, "failed to write response", http.StatusInternalServerError) + } +} diff --git a/server/service.go b/server/service.go index 1071d6e2..0d1640b5 100644 --- a/server/service.go +++ b/server/service.go @@ -20,7 +20,6 @@ import ( eth2ApiV1Capella "github.com/attestantio/go-eth2-client/api/v1/capella" eth2ApiV1Deneb "github.com/attestantio/go-eth2-client/api/v1/deneb" eth2ApiV1Electra "github.com/attestantio/go-eth2-client/api/v1/electra" - "github.com/attestantio/go-eth2-client/spec" "github.com/attestantio/go-eth2-client/spec/phase0" "github.com/flashbots/go-boost-utils/ssz" "github.com/flashbots/go-utils/httplogger" @@ -331,62 +330,8 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "relays": strings.Join(types.RelayEntriesToStrings(result.relays), ", "), }).Info("best bid") - // Define map of supported handlers - supportedMediaTypeHandlers := map[string]func(){ - MediaTypeJSON: func() { - w.Header().Set("Content-Type", MediaTypeJSON) - w.WriteHeader(http.StatusOK) - - // Serialize and write the data - if err := json.NewEncoder(w).Encode(&result.response); err != nil { - m.log.WithField("response", result.response).WithError(err).Error("could not write OK response") - http.Error(w, "", http.StatusInternalServerError) - } - }, - MediaTypeOctetStream: func() { - // Serialize the response - var sszData []byte - switch result.response.Version { - case spec.DataVersionBellatrix: - w.Header().Set("Eth-Consensus-Version", "bellatrix") - sszData, err = result.response.Bellatrix.MarshalSSZ() - case spec.DataVersionCapella: - w.Header().Set("Eth-Consensus-Version", "capella") - sszData, err = result.response.Capella.MarshalSSZ() - case spec.DataVersionDeneb: - w.Header().Set("Eth-Consensus-Version", "deneb") - sszData, err = result.response.Deneb.MarshalSSZ() - case spec.DataVersionElectra: - w.Header().Set("Eth-Consensus-Version", "electra") - sszData, err = result.response.Electra.MarshalSSZ() - case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: - err = errInvalidForkVersion - } - if err != nil { - m.log.WithError(err).Error("error serializing response as SSZ") - http.Error(w, "failed to serialize response", http.StatusInternalServerError) - return - } - - // Write the header - w.Header().Set("Content-Type", MediaTypeOctetStream) - w.WriteHeader(http.StatusOK) - - // Write SSZ data - if _, err := w.Write(sszData); err != nil { - m.log.WithError(err).Error("error writing SSZ response") - http.Error(w, "failed to write response", http.StatusInternalServerError) - } - }, - } - - // Generate slice of supported media types - supportedMediaTypes := make([]string, 0, len(supportedMediaTypeHandlers)) - for mediaType := range supportedMediaTypeHandlers { - supportedMediaTypes = append(supportedMediaTypes, mediaType) - } - // Default to the client's highest quality acceptable media type + supportedMediaTypes := []string{MediaTypeJSON, MediaTypeOctetStream} preferredContentType := SelectHighestQualityValueMediaType(clientAccepts, supportedMediaTypes) // If every relay returned the bid in JSON, that means that none @@ -407,8 +352,10 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) } // Respond appropriately - if mediaTypeHandler, ok := supportedMediaTypeHandlers[preferredContentType]; ok { - mediaTypeHandler() + if preferredContentType == MediaTypeJSON { + m.respondGetHeaderJSON(w, &result) + } else if preferredContentType == MediaTypeOctetStream { + m.respondGetHeaderSSZ(w, &result) } else { message := fmt.Sprintf("unsupported media type: %s", preferredContentType) m.respondError(w, http.StatusNotAcceptable, message) From fd18b827f24cc5323f15c7a409b9fbddce2238fc Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 13 Feb 2025 15:16:03 -0600 Subject: [PATCH 36/52] Pass header fields, not the whole header --- server/get_header.go | 22 +++++++++++++--------- server/service.go | 29 ++++++++++++++--------------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index fb959afb..b01901cf 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -24,7 +24,7 @@ import ( ) // getHeader requests a bid from each relay and returns the most profitable one -func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, header http.Header) (bidResp, error) { +func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, ua UserAgent, reqAccept, reqEthConsensusVersion string) (bidResp, error) { // Ensure arguments are valid if len(pubkey) != 98 { return bidResp{}, errInvalidPubkey @@ -43,6 +43,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa m.slotUIDLock.Unlock() log = log.WithField("slotUID", slotUID) + // Compute these once, instead of for each relay + userAgent := wrapUserAgent(ua) + startTime := fmt.Sprintf("%d", time.Now().UTC().UnixMilli()) + // Log how late into the slot the request starts slotStartTimestamp := m.genesisTime + uint64(slot)*config.SlotTimeSec msIntoSlot := uint64(time.Now().UTC().UnixMilli()) - slotStartTimestamp*1000 @@ -80,11 +84,11 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa return } - // Add headers from the request to this request. - // This includes Accept and Eth-Consensus-Version, if provided. - for key, values := range header { - req.Header[key] = values - } + // Add header fields to this request + req.Header.Set("User-Agent", userAgent) + req.Header.Set("Accept", reqAccept) + req.Header.Set("Eth-Consensus-Version", reqEthConsensusVersion) + req.Header.Set(HeaderStartTimeUnixMS, startTime) // Send the request log.Debug("requesting header") @@ -123,12 +127,12 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa log = log.WithField("respContentType", respContentType) // Get the optional version, used with SSZ decoding - ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") - log = log.WithField("ethConsensusVersion", ethConsensusVersion) + respEthConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + log = log.WithField("respEthConsensusVersion", respEthConsensusVersion) // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) - err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) + err = decodeBid(respBytes, respContentType, respEthConsensusVersion, bid) if err != nil { log.WithError(err).Warn("error decoding bid") return diff --git a/server/service.go b/server/service.go index 0d1640b5..4d3ae7f8 100644 --- a/server/service.go +++ b/server/service.go @@ -264,10 +264,12 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. // handleGetHeader requests bids from the relays func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) { var ( - vars = mux.Vars(req) - parentHashHex = vars["parent_hash"] - pubkey = vars["pubkey"] - ua = UserAgent(req.Header.Get("User-Agent")) + vars = mux.Vars(req) + parentHashHex = vars["parent_hash"] + pubkey = vars["pubkey"] + ua = UserAgent(req.Header.Get("User-Agent")) + reqAccept = req.Header.Get("Accept") + reqEthConsensusVersion = req.Header.Get("Eth-Consensus-Version") ) // Parse the slot @@ -280,21 +282,18 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // Add relevant fields to the logger log := m.log.WithFields(logrus.Fields{ - "method": "getHeader", - "slot": slot, - "parentHash": parentHashHex, - "pubkey": pubkey, - "ua": ua, + "method": "getHeader", + "slot": slot, + "parentHash": parentHashHex, + "pubkey": pubkey, + "ua": ua, + "reqAccept": reqAccept, + "reqEthConsensusVersion": reqEthConsensusVersion, }) log.Debug("handling request") - // Additional header fields - header := req.Header - header.Set("User-Agent", wrapUserAgent(ua)) - header.Set(HeaderStartTimeUnixMS, fmt.Sprintf("%d", time.Now().UTC().UnixMilli())) - // Query the relays for the header - result, err := m.getHeader(log, slot, pubkey, parentHashHex, header) + result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, reqAccept, reqEthConsensusVersion) if err != nil { m.respondError(w, http.StatusBadRequest, err.Error()) return From 0e38a228b6c98c52db06b4de1c2003fd03bd219f Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 13 Feb 2025 15:17:41 -0600 Subject: [PATCH 37/52] Add back HeaderKeySlotUID --- server/get_header.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/get_header.go b/server/get_header.go index b01901cf..7d0baaae 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -89,6 +89,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa req.Header.Set("Accept", reqAccept) req.Header.Set("Eth-Consensus-Version", reqEthConsensusVersion) req.Header.Set(HeaderStartTimeUnixMS, startTime) + req.Header.Set(HeaderKeySlotUID, slotUID.String()) // Send the request log.Debug("requesting header") From 8cd3b0e535cfea484f3fa8f10f89fb9b95eb5ad1 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Thu, 13 Feb 2025 15:33:20 -0600 Subject: [PATCH 38/52] Use timewasted/go-accept-headers --- go.mod | 1 + go.sum | 2 + server/accept.go | 134 ------------------------------------------ server/accept_test.go | 53 ----------------- server/service.go | 17 ++++-- 5 files changed, 14 insertions(+), 193 deletions(-) delete mode 100644 server/accept.go delete mode 100644 server/accept_test.go diff --git a/go.mod b/go.mod index 7ee99e62..25e524e6 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/prysmaticlabs/go-bitfield v0.0.0-20240618144021-706c95b2dd15 github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.10.0 + github.com/timewasted/go-accept-headers v0.0.0-20130320203746-c78f304b1b09 github.com/urfave/cli/v3 v3.0.0-beta1 ) diff --git a/go.sum b/go.sum index efcb6c5c..f9c9a855 100644 --- a/go.sum +++ b/go.sum @@ -129,6 +129,8 @@ github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOf github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= github.com/supranational/blst v0.3.13 h1:AYeSxdOMacwu7FBmpfloBz5pbFXDmJL33RuwnKtmTjk= github.com/supranational/blst v0.3.13/go.mod h1:jZJtfjgudtNl4en1tzwPIV3KjUnQUvG3/j+w+fVonLw= +github.com/timewasted/go-accept-headers v0.0.0-20130320203746-c78f304b1b09 h1:QVxbx5l/0pzciWYOynixQMtUhPYC3YKD6EcUlOsgGqw= +github.com/timewasted/go-accept-headers v0.0.0-20130320203746-c78f304b1b09/go.mod h1:Uy/Rnv5WKuOO+PuDhuYLEpUiiKIZtss3z519uk67aF0= github.com/tklauser/go-sysconf v0.3.12 h1:0QaGUFOdQaIVdPgfITYzaTegZvdCjmYO52cSFAEVmqU= github.com/tklauser/go-sysconf v0.3.12/go.mod h1:Ho14jnntGE1fpdOqQEEaiKRpvIavV0hSfmBq8nJbHYI= github.com/tklauser/numcpus v0.6.1 h1:ng9scYS7az0Bk4OZLvrNXNSAO2Pxr1XXRAPyjhIx+Fk= diff --git a/server/accept.go b/server/accept.go deleted file mode 100644 index 15fc4012..00000000 --- a/server/accept.go +++ /dev/null @@ -1,134 +0,0 @@ -package server - -import ( - "strconv" - "strings" -) - -type AcceptEntry struct { - MediaType string - Quality float64 - pos int // position in the header (lower=earlier) -} - -// ParseAcceptHeader takes an Accept header string and returns a slice of valid entries. -// It splits the header by commas and for each part, it splits by semicolon to find -// a possible "q" parameter. If the part is malformed (for example, a trailing semicolon -// with no parameter or a q value that cannot be parsed or is not in [0,1]), the entry is ignored. -func ParseAcceptHeader(header string) []AcceptEntry { - parts := strings.Split(header, ",") - entries := make([]AcceptEntry, 0, len(parts)) - for i, part := range parts { - part = strings.TrimSpace(part) - if part == "" { - continue - } - // Split on semicolon; first token is the media-range. - tokens := strings.Split(part, ";") - mediaType := strings.TrimSpace(tokens[0]) - if mediaType == "" { - continue - } - // Default q is 1. - quality := 1.0 - valid := true - // Process parameters. - for _, token := range tokens[1:] { - token = strings.TrimSpace(token) - // If a parameter is empty (for example, a trailing semicolon) then consider the entry malformed. - if token == "" { - valid = false - break - } - // Look for a "q" parameter. - if strings.HasPrefix(token, "q=") { - kv := strings.SplitN(token, "=", 2) - if len(kv) != 2 { - valid = false - break - } - value := strings.TrimSpace(kv[1]) - f, err := strconv.ParseFloat(value, 64) - if err != nil || f < 0 || f > 1 { - valid = false - break - } - quality = f - } - // Other parameters are ignored. - } - if !valid { - continue - } - entries = append(entries, AcceptEntry{ - MediaType: mediaType, - Quality: quality, - pos: i, - }) - } - return entries -} - -// SelectHighestQualityValueMediaType takes the parsed Accept header entries (from ParseAcceptHeader) -// and a slice of supported media types, then returns the one with the highest quality factor. -// A supported type is considered matching if it is an exact match or if the accept entry is "*/*". -// When quality factors are equal, the later (higher pos) accept entry “wins” – and if even that is tied, -// the order of supportedMediaTypes is used as a final tie‐breaker. -func SelectHighestQualityValueMediaType(entries []AcceptEntry, supportedMediaTypes []string) string { - type candidate struct { - mediaType string - q float64 - pos int // position of the matching accept entry - index int // the index in the supportedMediaTypes slice - } - var bestCandidate *candidate - // For each supported media type, find its best matching accept entry. - for idx, supp := range supportedMediaTypes { - bestQ := -1.0 - bestPos := -1 - found := false - for _, e := range entries { - if e.MediaType == supp || e.MediaType == "*/*" { - // If this entry has a higher q or, in case of equal q, a later position, - // then it is preferred. - if e.Quality > bestQ || (e.Quality == bestQ && e.pos > bestPos) { - bestQ = e.Quality - bestPos = e.pos - found = true - } - } - } - if found { - cand := candidate{ - mediaType: supp, - q: bestQ, - pos: bestPos, - index: idx, - } - if bestCandidate == nil { - bestCandidate = &cand - } else { - // Compare candidates: higher q wins; if equal, then later accept header position wins; - // if still equal, then the supportedMediaTypes order (lower index wins). - if cand.q > bestCandidate.q || - (cand.q == bestCandidate.q && cand.pos > bestCandidate.pos) || - (cand.q == bestCandidate.q && cand.pos == bestCandidate.pos && cand.index < bestCandidate.index) { - bestCandidate = &cand - } - } - } - } - if bestCandidate != nil { - return bestCandidate.mediaType - } - return "" -} - -func Accepts(entries []AcceptEntry, mediaType string) bool { - for _, entry := range entries { - if entry.MediaType == mediaType { - return true - } - } - return false -} diff --git a/server/accept_test.go b/server/accept_test.go deleted file mode 100644 index 7884623d..00000000 --- a/server/accept_test.go +++ /dev/null @@ -1,53 +0,0 @@ -package server - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -// TestParseAcceptHeader ensures that parsing/selection works properly. -// These tests come from the Lodestar client (thank you for sharing!) here: -// https://github.com/ChainSafe/lodestar/blob/9c66fac4a47446b225c874997dfc4d7b93a820b3/packages/api/test/unit/utils/headers.test.ts#L5-L42 -func TestParseAcceptHeader(t *testing.T) { - testCases := []struct { - header string - expected string - }{ - {"", ""}, - {"*/*", MediaTypeJSON}, - {"application/json", MediaTypeJSON}, - {"application/octet-stream", MediaTypeOctetStream}, - {"application/invalid", ""}, - {"application/invalid;q=1,application/octet-stream;q=0.1", MediaTypeOctetStream}, - {"application/octet-stream;q=0.5,application/json;q=1", MediaTypeJSON}, - {"application/octet-stream;q=1,application/json;q=0.1", MediaTypeOctetStream}, - {"application/octet-stream;q=1,application/json;q=0.9", MediaTypeOctetStream}, - {"application/octet-stream;q=1,*/*;q=0.9", MediaTypeOctetStream}, - {"application/octet-stream,application/json;q=0.1", MediaTypeOctetStream}, - {"application/octet-stream;,application/json;q=0.1", MediaTypeJSON}, // Malformed entry - {"application/octet-stream;q=2,application/json;q=0.1", MediaTypeJSON}, // Invalid q-value >1, fallback - {"application/octet-stream;q=invalid,application/json;q=0.1", MediaTypeJSON}, // Invalid q-value, ignored - {"application/octet-stream ; q=0.5 , application/json ; q=1", MediaTypeJSON}, - {"application/octet-stream ; q=1 , application/json ; q=0.1", MediaTypeOctetStream}, - {"application/octet-stream;q=1,application/json;q=0.1", MediaTypeOctetStream}, - { - // Default Chrome Accept header, JSON should be preferred - "text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,image/apng,*/*;q=0.8,application/signed-exchange;v=b3;q=0.7", - MediaTypeJSON, - }, - // Order-dependent tests (last one wins if q-values are the same) - {"application/octet-stream;q=1,application/json;q=1", MediaTypeJSON}, - {"application/json;q=1,application/octet-stream;q=1", MediaTypeOctetStream}, - } - - supportedMediaTypes := []string{MediaTypeJSON, MediaTypeOctetStream} - - for _, tc := range testCases { - t.Run(tc.header, func(t *testing.T) { - parsed := ParseAcceptHeader(tc.header) - selected := SelectHighestQualityValueMediaType(parsed, supportedMediaTypes) - require.Equal(t, tc.expected, selected) - }) - } -} diff --git a/server/service.go b/server/service.go index 4d3ae7f8..0696f14c 100644 --- a/server/service.go +++ b/server/service.go @@ -29,6 +29,7 @@ import ( "github.com/google/uuid" "github.com/gorilla/mux" "github.com/sirupsen/logrus" + goacceptheaders "github.com/timewasted/go-accept-headers" ) var ( @@ -312,11 +313,11 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) m.bidsLock.Unlock() // Decide response content type (JSON by default) - clientAccepts := ParseAcceptHeader(req.Header.Get("Accept")) + clientAccepts := goacceptheaders.Parse(req.Header.Get("Accept")) log.Debug("clientAccepts", clientAccepts) if len(clientAccepts) == 0 { log.Info("no client accepts, defaulting to JSON") - clientAccepts = []AcceptEntry{{MediaType: MediaTypeJSON}} + clientAccepts = goacceptheaders.AcceptSlice{{Type: MediaTypeJSON}} } // Log result @@ -329,9 +330,13 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "relays": strings.Join(types.RelayEntriesToStrings(result.relays), ", "), }).Info("best bid") - // Default to the client's highest quality acceptable media type - supportedMediaTypes := []string{MediaTypeJSON, MediaTypeOctetStream} - preferredContentType := SelectHighestQualityValueMediaType(clientAccepts, supportedMediaTypes) + // Get the client's highest quality acceptable media type. If the client did not + // specify an Accept value, the first media type (JSON) will be provided. + preferredContentType, err := clientAccepts.Negotiate(MediaTypeJSON, MediaTypeOctetStream) + if err != nil { + log.Warn("failed to negotiate preferred content-type", err) + preferredContentType = MediaTypeJSON + } // If every relay returned the bid in JSON, that means that none // of them support SSZ and this would always require extra conversions. @@ -345,7 +350,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) break } } - if preferredContentType != MediaTypeJSON && allBidsWereJSON && Accepts(clientAccepts, MediaTypeJSON) { + if preferredContentType != MediaTypeJSON && allBidsWereJSON && clientAccepts.Accepts(MediaTypeJSON) { log.Debug("overriding the response content type to be JSON") preferredContentType = MediaTypeJSON } From 5b6caba00796cb5b65d5cd1998df6cbe4fdf83f1 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 09:17:54 -0600 Subject: [PATCH 39/52] Clean up header fields --- server/get_header.go | 11 +++++------ server/service.go | 26 ++++++++++++-------------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 7d0baaae..f74245a8 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -24,7 +24,7 @@ import ( ) // getHeader requests a bid from each relay and returns the most profitable one -func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, ua UserAgent, reqAccept, reqEthConsensusVersion string) (bidResp, error) { +func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, ua UserAgent, accept string) (bidResp, error) { // Ensure arguments are valid if len(pubkey) != 98 { return bidResp{}, errInvalidPubkey @@ -86,8 +86,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // Add header fields to this request req.Header.Set("User-Agent", userAgent) - req.Header.Set("Accept", reqAccept) - req.Header.Set("Eth-Consensus-Version", reqEthConsensusVersion) + req.Header.Set("Accept", accept) req.Header.Set(HeaderStartTimeUnixMS, startTime) req.Header.Set(HeaderKeySlotUID, slotUID.String()) @@ -128,12 +127,12 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa log = log.WithField("respContentType", respContentType) // Get the optional version, used with SSZ decoding - respEthConsensusVersion := resp.Header.Get("Eth-Consensus-Version") - log = log.WithField("respEthConsensusVersion", respEthConsensusVersion) + ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + log = log.WithField("ethConsensusVersion", ethConsensusVersion) // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) - err = decodeBid(respBytes, respContentType, respEthConsensusVersion, bid) + err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) if err != nil { log.WithError(err).Warn("error decoding bid") return diff --git a/server/service.go b/server/service.go index 0696f14c..88dc4b72 100644 --- a/server/service.go +++ b/server/service.go @@ -265,12 +265,11 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. // handleGetHeader requests bids from the relays func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) { var ( - vars = mux.Vars(req) - parentHashHex = vars["parent_hash"] - pubkey = vars["pubkey"] - ua = UserAgent(req.Header.Get("User-Agent")) - reqAccept = req.Header.Get("Accept") - reqEthConsensusVersion = req.Header.Get("Eth-Consensus-Version") + vars = mux.Vars(req) + parentHashHex = vars["parent_hash"] + pubkey = vars["pubkey"] + ua = UserAgent(req.Header.Get("User-Agent")) + accept = req.Header.Get("Accept") ) // Parse the slot @@ -283,18 +282,17 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // Add relevant fields to the logger log := m.log.WithFields(logrus.Fields{ - "method": "getHeader", - "slot": slot, - "parentHash": parentHashHex, - "pubkey": pubkey, - "ua": ua, - "reqAccept": reqAccept, - "reqEthConsensusVersion": reqEthConsensusVersion, + "method": "getHeader", + "slot": slot, + "parentHash": parentHashHex, + "pubkey": pubkey, + "ua": ua, + "accept": accept, }) log.Debug("handling request") // Query the relays for the header - result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, reqAccept, reqEthConsensusVersion) + result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, accept) if err != nil { m.respondError(w, http.StatusBadRequest, err.Error()) return From 3fbeafd57d903886596a778974b1791d0b50762a Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 09:28:44 -0600 Subject: [PATCH 40/52] Fix some nits & add more debug prints --- server/get_header.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index f74245a8..0649d14f 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -127,12 +127,12 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa log = log.WithField("respContentType", respContentType) // Get the optional version, used with SSZ decoding - ethConsensusVersion := resp.Header.Get("Eth-Consensus-Version") - log = log.WithField("ethConsensusVersion", ethConsensusVersion) + respEthConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + log = log.WithField("respEthConsensusVersion", respEthConsensusVersion) // Decode bid bid := new(builderSpec.VersionedSignedBuilderBid) - err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) + err = decodeBid(respBytes, respContentType, respEthConsensusVersion, bid) if err != nil { log.WithError(err).Warn("error decoding bid") return @@ -140,6 +140,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // Skip if bid is empty if bid.IsEmpty() { + log.Debug("skipping empty bid") return } @@ -226,12 +227,14 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa valueDiff := bidInfo.value.Cmp(result.bidInfo.value) if valueDiff == -1 { // The current bid is less profitable than already known one + log.Debug("ignoring less profitable bid") return } else if valueDiff == 0 { // The current bid is equally profitable as already known one // Use hash as tiebreaker previousBidBlockHash := result.bidInfo.blockHash if bidInfo.blockHash.String() >= previousBidBlockHash.String() { + log.Debug("equally profitable bid lost tiebreaker") return } } From dc7aaaa68ba94f32a9d083344e3344dbb24ce47c Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 09:45:07 -0600 Subject: [PATCH 41/52] Address review feedback --- server/get_header.go | 4 +-- server/service.go | 51 ++++++++++++++++++++----------------- server/types/relay_entry.go | 2 +- 3 files changed, 30 insertions(+), 27 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 0649d14f..d04cbc28 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -24,7 +24,7 @@ import ( ) // getHeader requests a bid from each relay and returns the most profitable one -func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, ua UserAgent, accept string) (bidResp, error) { +func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, parentHashHex string, ua UserAgent, proposerAcceptContentTypes string) (bidResp, error) { // Ensure arguments are valid if len(pubkey) != 98 { return bidResp{}, errInvalidPubkey @@ -86,7 +86,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // Add header fields to this request req.Header.Set("User-Agent", userAgent) - req.Header.Set("Accept", accept) + req.Header.Set("Accept", proposerAcceptContentTypes) req.Header.Set(HeaderStartTimeUnixMS, startTime) req.Header.Set(HeaderKeySlotUID, slotUID.String()) diff --git a/server/service.go b/server/service.go index 88dc4b72..da7cde8e 100644 --- a/server/service.go +++ b/server/service.go @@ -265,11 +265,11 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. // handleGetHeader requests bids from the relays func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) { var ( - vars = mux.Vars(req) - parentHashHex = vars["parent_hash"] - pubkey = vars["pubkey"] - ua = UserAgent(req.Header.Get("User-Agent")) - accept = req.Header.Get("Accept") + vars = mux.Vars(req) + parentHashHex = vars["parent_hash"] + pubkey = vars["pubkey"] + ua = UserAgent(req.Header.Get("User-Agent")) + proposerAcceptContentTypes = req.Header.Get("Accept") ) // Parse the slot @@ -282,17 +282,17 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // Add relevant fields to the logger log := m.log.WithFields(logrus.Fields{ - "method": "getHeader", - "slot": slot, - "parentHash": parentHashHex, - "pubkey": pubkey, - "ua": ua, - "accept": accept, + "method": "getHeader", + "slot": slot, + "parentHash": parentHashHex, + "pubkey": pubkey, + "ua": ua, + "proposerAcceptContentTypes": proposerAcceptContentTypes, }) log.Debug("handling request") // Query the relays for the header - result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, accept) + result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, proposerAcceptContentTypes) if err != nil { m.respondError(w, http.StatusBadRequest, err.Error()) return @@ -311,11 +311,11 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) m.bidsLock.Unlock() // Decide response content type (JSON by default) - clientAccepts := goacceptheaders.Parse(req.Header.Get("Accept")) - log.Debug("clientAccepts", clientAccepts) - if len(clientAccepts) == 0 { - log.Info("no client accepts, defaulting to JSON") - clientAccepts = goacceptheaders.AcceptSlice{{Type: MediaTypeJSON}} + proposerAccepts := goacceptheaders.Parse(proposerAcceptContentTypes) + log.Debug("proposerAcceptContentTypes", proposerAccepts) + if len(proposerAccepts) == 0 { + log.Info("no proposerAccepts, defaulting to JSON") + proposerAccepts = goacceptheaders.AcceptSlice{{Type: MediaTypeJSON}} } // Log result @@ -330,10 +330,10 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // Get the client's highest quality acceptable media type. If the client did not // specify an Accept value, the first media type (JSON) will be provided. - preferredContentType, err := clientAccepts.Negotiate(MediaTypeJSON, MediaTypeOctetStream) + proposerPreferredContentType, err := proposerAccepts.Negotiate(MediaTypeJSON, MediaTypeOctetStream) if err != nil { log.Warn("failed to negotiate preferred content-type", err) - preferredContentType = MediaTypeJSON + proposerPreferredContentType = MediaTypeJSON } // If every relay returned the bid in JSON, that means that none @@ -348,18 +348,21 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) break } } - if preferredContentType != MediaTypeJSON && allBidsWereJSON && clientAccepts.Accepts(MediaTypeJSON) { + if proposerPreferredContentType != MediaTypeJSON && allBidsWereJSON && proposerAccepts.Accepts(MediaTypeJSON) { log.Debug("overriding the response content type to be JSON") - preferredContentType = MediaTypeJSON + proposerPreferredContentType = MediaTypeJSON } // Respond appropriately - if preferredContentType == MediaTypeJSON { + if proposerPreferredContentType == MediaTypeJSON { + log.Debug("responding with JSON") m.respondGetHeaderJSON(w, &result) - } else if preferredContentType == MediaTypeOctetStream { + } else if proposerPreferredContentType == MediaTypeOctetStream { + log.Debug("responding with SSZ") m.respondGetHeaderSSZ(w, &result) } else { - message := fmt.Sprintf("unsupported media type: %s", preferredContentType) + message := fmt.Sprintf("unsupported media type: %s", proposerPreferredContentType) + log.Error(message) m.respondError(w, http.StatusNotAcceptable, message) } } diff --git a/server/types/relay_entry.go b/server/types/relay_entry.go index f2e37c11..ec97f09f 100644 --- a/server/types/relay_entry.go +++ b/server/types/relay_entry.go @@ -77,10 +77,10 @@ func RelayEntriesToStrings(relays []RelayEntry) []string { // Copy returns a deep copy of the relay entry. func (r *RelayEntry) Copy() (ret RelayEntry) { ret.PublicKey = r.PublicKey + ret.SupportsSSZ = r.SupportsSSZ if r.URL != nil { urlCopy := *r.URL ret.URL = &urlCopy } - ret.SupportsSSZ = r.SupportsSSZ return } From dc70b85305715b97de0f39f798b605bff51d1626 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 09:48:39 -0600 Subject: [PATCH 42/52] Replace "client" with "proposer" in comments --- server/get_header.go | 4 ++-- server/service.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index d04cbc28..4dc1bc2a 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -290,7 +290,7 @@ func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bi return types.ErrInvalidContentType } -// respondGetHeaderJSON responds to the client in JSON +// respondGetHeaderJSON responds to the proposer in JSON func (m *BoostService) respondGetHeaderJSON(w http.ResponseWriter, result *bidResp) { w.Header().Set("Content-Type", MediaTypeJSON) w.WriteHeader(http.StatusOK) @@ -302,7 +302,7 @@ func (m *BoostService) respondGetHeaderJSON(w http.ResponseWriter, result *bidRe } } -// respondGetHeaderSSZ responds to the client in SSZ +// respondGetHeaderSSZ responds to the proposer in SSZ func (m *BoostService) respondGetHeaderSSZ(w http.ResponseWriter, result *bidResp) { // Serialize the response var err error diff --git a/server/service.go b/server/service.go index da7cde8e..f4b078f6 100644 --- a/server/service.go +++ b/server/service.go @@ -328,7 +328,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "relays": strings.Join(types.RelayEntriesToStrings(result.relays), ", "), }).Info("best bid") - // Get the client's highest quality acceptable media type. If the client did not + // Get the proposer's highest quality acceptable media type. If the proposer did not // specify an Accept value, the first media type (JSON) will be provided. proposerPreferredContentType, err := proposerAccepts.Negotiate(MediaTypeJSON, MediaTypeOctetStream) if err != nil { @@ -338,8 +338,8 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // If every relay returned the bid in JSON, that means that none // of them support SSZ and this would always require extra conversions. - // In this situation, if the client still accepts JSON, respond to the - // getHeader in JSON so the client sends the getPayload request in JSON. + // In this situation, if the proposer still accepts JSON, respond to the + // getHeader in JSON so the proposer sends the getPayload request in JSON. allBidsWereJSON := true for _, relay := range result.relays { if relay.SupportsSSZ { From d3123bf728e5a0883927b37c4ac84d3cc49eab6b Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 09:50:35 -0600 Subject: [PATCH 43/52] Remove unnecessary debug print --- server/service.go | 1 - 1 file changed, 1 deletion(-) diff --git a/server/service.go b/server/service.go index f4b078f6..ba11380b 100644 --- a/server/service.go +++ b/server/service.go @@ -312,7 +312,6 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // Decide response content type (JSON by default) proposerAccepts := goacceptheaders.Parse(proposerAcceptContentTypes) - log.Debug("proposerAcceptContentTypes", proposerAccepts) if len(proposerAccepts) == 0 { log.Info("no proposerAccepts, defaulting to JSON") proposerAccepts = goacceptheaders.AcceptSlice{{Type: MediaTypeJSON}} From 744273a51d7438d52c1a579bd529b393417a6f5f Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 10:05:23 -0600 Subject: [PATCH 44/52] Rename some variables & fix a nit --- server/service.go | 47 +++++++++++++++++++++++------------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/server/service.go b/server/service.go index ba11380b..aa7c3b21 100644 --- a/server/service.go +++ b/server/service.go @@ -265,11 +265,12 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. // handleGetHeader requests bids from the relays func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) { var ( - vars = mux.Vars(req) - parentHashHex = vars["parent_hash"] - pubkey = vars["pubkey"] - ua = UserAgent(req.Header.Get("User-Agent")) - proposerAcceptContentTypes = req.Header.Get("Accept") + vars = mux.Vars(req) + parentHashHex = vars["parent_hash"] + pubkey = vars["pubkey"] + ua = UserAgent(req.Header.Get("User-Agent")) + rawProposerAcceptContentTypes = req.Header.Get("Accept") + parsedProposerAcceptContentTypes = goacceptheaders.Parse(rawProposerAcceptContentTypes) ) // Parse the slot @@ -282,17 +283,17 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) // Add relevant fields to the logger log := m.log.WithFields(logrus.Fields{ - "method": "getHeader", - "slot": slot, - "parentHash": parentHashHex, - "pubkey": pubkey, - "ua": ua, - "proposerAcceptContentTypes": proposerAcceptContentTypes, + "method": "getHeader", + "slot": slot, + "parentHash": parentHashHex, + "pubkey": pubkey, + "ua": ua, + "rawProposerAcceptContentTypes": rawProposerAcceptContentTypes, }) log.Debug("handling request") // Query the relays for the header - result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, proposerAcceptContentTypes) + result, err := m.getHeader(log, slot, pubkey, parentHashHex, ua, rawProposerAcceptContentTypes) if err != nil { m.respondError(w, http.StatusBadRequest, err.Error()) return @@ -310,13 +311,6 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) m.bids[bidKey(slot, result.bidInfo.blockHash)] = result m.bidsLock.Unlock() - // Decide response content type (JSON by default) - proposerAccepts := goacceptheaders.Parse(proposerAcceptContentTypes) - if len(proposerAccepts) == 0 { - log.Info("no proposerAccepts, defaulting to JSON") - proposerAccepts = goacceptheaders.AcceptSlice{{Type: MediaTypeJSON}} - } - // Log result valueEth := weiBigIntToEthBigFloat(result.bidInfo.value.ToBig()) log.WithFields(logrus.Fields{ @@ -327,11 +321,16 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) "relays": strings.Join(types.RelayEntriesToStrings(result.relays), ", "), }).Info("best bid") - // Get the proposer's highest quality acceptable media type. If the proposer did not - // specify an Accept value, the first media type (JSON) will be provided. - proposerPreferredContentType, err := proposerAccepts.Negotiate(MediaTypeJSON, MediaTypeOctetStream) + // Default to JSON if the proposer provides nothing + if len(parsedProposerAcceptContentTypes) == 0 { + log.Info("no proposer accepts, defaulting to JSON") + parsedProposerAcceptContentTypes = goacceptheaders.AcceptSlice{{Type: MediaTypeJSON}} + } + + // Get the proposer's highest quality acceptable media type + proposerPreferredContentType, err := parsedProposerAcceptContentTypes.Negotiate(MediaTypeJSON, MediaTypeOctetStream) if err != nil { - log.Warn("failed to negotiate preferred content-type", err) + log.WithError(err).Warn("failed to negotiate preferred content-type") proposerPreferredContentType = MediaTypeJSON } @@ -347,7 +346,7 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) break } } - if proposerPreferredContentType != MediaTypeJSON && allBidsWereJSON && proposerAccepts.Accepts(MediaTypeJSON) { + if proposerPreferredContentType != MediaTypeJSON && allBidsWereJSON && parsedProposerAcceptContentTypes.Accepts(MediaTypeJSON) { log.Debug("overriding the response content type to be JSON") proposerPreferredContentType = MediaTypeJSON } From de439861d52fa2166f941ba431093e77859a820f Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Fri, 14 Feb 2025 10:14:42 -0600 Subject: [PATCH 45/52] Add another debug print --- server/get_header.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/get_header.go b/server/get_header.go index 4dc1bc2a..b4299b6e 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -122,6 +122,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa // Get the response's content type, default to JSON respContentType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type")) if err != nil { + log.WithError(err).Warn("error parsing response content type") respContentType = MediaTypeJSON } log = log.WithField("respContentType", respContentType) From 3c852739ee0b8cd32ba2de274079facad57949e9 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 08:37:30 -0600 Subject: [PATCH 46/52] Use more header constants --- server/get_header.go | 22 ++++----- server/service.go | 8 ++-- server/service_test.go | 100 ++++++++++++++++++++--------------------- server/utils.go | 10 +++-- 4 files changed, 72 insertions(+), 68 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index b4299b6e..56e7c207 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -85,10 +85,10 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } // Add header fields to this request - req.Header.Set("User-Agent", userAgent) - req.Header.Set("Accept", proposerAcceptContentTypes) - req.Header.Set(HeaderStartTimeUnixMS, startTime) + req.Header.Set(HeaderAccept, proposerAcceptContentTypes) req.Header.Set(HeaderKeySlotUID, slotUID.String()) + req.Header.Set(HeaderStartTimeUnixMS, startTime) + req.Header.Set(HeaderUserAgent, userAgent) // Send the request log.Debug("requesting header") @@ -120,7 +120,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa } // Get the response's content type, default to JSON - respContentType, _, err := mime.ParseMediaType(resp.Header.Get("Content-Type")) + respContentType, _, err := mime.ParseMediaType(resp.Header.Get(HeaderContentType)) if err != nil { log.WithError(err).Warn("error parsing response content type") respContentType = MediaTypeJSON @@ -128,7 +128,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, slot phase0.Slot, pubkey, pa log = log.WithField("respContentType", respContentType) // Get the optional version, used with SSZ decoding - respEthConsensusVersion := resp.Header.Get("Eth-Consensus-Version") + respEthConsensusVersion := resp.Header.Get(HeaderEthConsensusVersion) log = log.WithField("respEthConsensusVersion", respEthConsensusVersion) // Decode bid @@ -293,7 +293,7 @@ func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bi // respondGetHeaderJSON responds to the proposer in JSON func (m *BoostService) respondGetHeaderJSON(w http.ResponseWriter, result *bidResp) { - w.Header().Set("Content-Type", MediaTypeJSON) + w.Header().Set(HeaderContentType, MediaTypeJSON) w.WriteHeader(http.StatusOK) // Serialize and write the data @@ -310,16 +310,16 @@ func (m *BoostService) respondGetHeaderSSZ(w http.ResponseWriter, result *bidRes var sszData []byte switch result.response.Version { case spec.DataVersionBellatrix: - w.Header().Set("Eth-Consensus-Version", "bellatrix") + w.Header().Set(HeaderEthConsensusVersion, "bellatrix") sszData, err = result.response.Bellatrix.MarshalSSZ() case spec.DataVersionCapella: - w.Header().Set("Eth-Consensus-Version", "capella") + w.Header().Set(HeaderEthConsensusVersion, "capella") sszData, err = result.response.Capella.MarshalSSZ() case spec.DataVersionDeneb: - w.Header().Set("Eth-Consensus-Version", "deneb") + w.Header().Set(HeaderEthConsensusVersion, "deneb") sszData, err = result.response.Deneb.MarshalSSZ() case spec.DataVersionElectra: - w.Header().Set("Eth-Consensus-Version", "electra") + w.Header().Set(HeaderEthConsensusVersion, "electra") sszData, err = result.response.Electra.MarshalSSZ() case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: err = errInvalidForkVersion @@ -331,7 +331,7 @@ func (m *BoostService) respondGetHeaderSSZ(w http.ResponseWriter, result *bidRes } // Write the header - w.Header().Set("Content-Type", MediaTypeOctetStream) + w.Header().Set(HeaderContentType, MediaTypeOctetStream) w.WriteHeader(http.StatusOK) // Write SSZ data diff --git a/server/service.go b/server/service.go index 737987a6..d965357e 100644 --- a/server/service.go +++ b/server/service.go @@ -133,7 +133,7 @@ func NewBoostService(opts BoostServiceOpts) (*BoostService, error) { } func (m *BoostService) respondError(w http.ResponseWriter, code int, message string) { - w.Header().Set("Content-Type", "application/json") + w.Header().Set(HeaderContentType, MediaTypeJSON) w.WriteHeader(code) resp := httpErrorResp{code, message} if err := json.NewEncoder(w).Encode(resp); err != nil { @@ -143,7 +143,7 @@ func (m *BoostService) respondError(w http.ResponseWriter, code int, message str } func (m *BoostService) respondOK(w http.ResponseWriter, response any) { - w.Header().Set("Content-Type", "application/json") + w.Header().Set(HeaderContentType, MediaTypeJSON) w.WriteHeader(http.StatusOK) if err := json.NewEncoder(w).Encode(response); err != nil { m.log.WithField("response", response).WithError(err).Error("could not write OK response") @@ -261,8 +261,8 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) vars = mux.Vars(req) parentHashHex = vars["parent_hash"] pubkey = vars["pubkey"] - ua = UserAgent(req.Header.Get("User-Agent")) - rawProposerAcceptContentTypes = req.Header.Get("Accept") + ua = UserAgent(req.Header.Get(HeaderUserAgent)) + rawProposerAcceptContentTypes = req.Header.Get(HeaderAccept) parsedProposerAcceptContentTypes = goacceptheaders.Parse(rawProposerAcceptContentTypes) ) diff --git a/server/service_test.go b/server/service_test.go index c84a3f91..e2a828ea 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -172,7 +172,7 @@ func TestWebserverMaxHeaderSize(t *testing.T) { func TestStatus(t *testing.T) { t.Run("At least one relay is available", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) time.Sleep(time.Millisecond * 20) @@ -186,7 +186,7 @@ func TestStatus(t *testing.T) { t.Run("No relays available", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) backend.relays[0].Server.Close() // makes the relay unavailable @@ -216,7 +216,7 @@ func TestRegisterValidator(t *testing.T) { t.Run("Normal function", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodPost, path, header, payload) @@ -226,7 +226,7 @@ func TestRegisterValidator(t *testing.T) { t.Run("Relay error response", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 2, time.Second) backend.relays[0].ResponseDelay = 5 * time.Millisecond @@ -260,7 +260,7 @@ func TestRegisterValidator(t *testing.T) { t.Run("mev-boost relay timeout works with slow relay", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, 150*time.Millisecond) // 10ms max rr := backend.request(t, http.MethodPost, path, header, payload) @@ -288,7 +288,7 @@ func TestGetHeader(t *testing.T) { t.Run("Okay response from relay", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodGet, path, header, nil) @@ -298,7 +298,7 @@ func TestGetHeader(t *testing.T) { t.Run("Okay response from relay deneb", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodGet, path, header, nil) @@ -308,8 +308,8 @@ func TestGetHeader(t *testing.T) { t.Run("Okay response from relay deneb in ssz", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, MediaTypeOctetStream) backend := newTestBackend(t, 1, time.Second) resp := backend.relays[0].MakeGetHeaderResponse( @@ -323,7 +323,7 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) // Ensure the response was SSZ bid := new(builderApiDeneb.SignedBuilderBid) @@ -334,21 +334,21 @@ func TestGetHeader(t *testing.T) { t.Run("Relay returns SSZ, mev-boost returns SSZ", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") backend := newTestBackend(t, 1, time.Second) backend.relays[0].ForceSSZ = true rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) }) t.Run("One relay returns SSZ, another relay returns JSON, mev-boost returns SSZ", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") backend := newTestBackend(t, 2, time.Second) backend.relays[0].ForceSSZ = true @@ -357,26 +357,26 @@ func TestGetHeader(t *testing.T) { require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) - require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) }) t.Run("One relay returns JSON, mev-boost returns JSON", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") backend := newTestBackend(t, 1, time.Second) backend.relays[0].ForceJSON = true rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) }) t.Run("Two relays return JSON, mev-boost returns JSON", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") backend := newTestBackend(t, 2, time.Second) backend.relays[0].ForceJSON = true @@ -385,50 +385,50 @@ func TestGetHeader(t *testing.T) { require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) - require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) }) t.Run("Accepts both with Q values", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream;q=1.0,application/json;q=0.9") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, "application/octet-stream", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) }) t.Run("No accept value", func(t *testing.T) { // This should default to JSON header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Del("Accept") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Del(HeaderAccept) backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) }) t.Run("Accepts both but prefers JSON", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "application/octet-stream;q=0.9,application/json;q=1.0") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "application/octet-stream;q=0.9,application/json;q=1.0") backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, "application/json", rr.Header().Get("Content-Type")) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) }) t.Run("Only accepts unsupported media type", func(t *testing.T) { header := make(http.Header) - header.Set("Eth-Consensus-Version", "deneb") - header.Set("Accept", "plain/text") + header.Set(HeaderEthConsensusVersion, "deneb") + header.Set(HeaderAccept, "plain/text") backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodGet, path, header, nil) @@ -437,7 +437,7 @@ func TestGetHeader(t *testing.T) { t.Run("Bad response from relays", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 2, time.Second) resp := backend.relays[0].MakeGetHeaderResponse( @@ -466,7 +466,7 @@ func TestGetHeader(t *testing.T) { t.Run("Invalid relay public key", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) @@ -491,7 +491,7 @@ func TestGetHeader(t *testing.T) { t.Run("Invalid relay signature", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) @@ -515,7 +515,7 @@ func TestGetHeader(t *testing.T) { t.Run("Invalid slot number", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) // Number larger than uint64 creates parsing error slot := fmt.Sprintf("%d0", uint64(math.MaxUint64)) @@ -530,7 +530,7 @@ func TestGetHeader(t *testing.T) { t.Run("Invalid pubkey length", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) invalidPubkeyPath := fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", 1, hash.String(), "0x1") @@ -543,7 +543,7 @@ func TestGetHeader(t *testing.T) { t.Run("Invalid hash length", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) invalidSlotPath := fmt.Sprintf("/eth/v1/builder/header/%d/%s/%s", 1, "0x1", pubkey.String()) @@ -556,7 +556,7 @@ func TestGetHeader(t *testing.T) { t.Run("Invalid parent hash", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) @@ -576,7 +576,7 @@ func TestGetHeaderBids(t *testing.T) { t.Run("Use header with highest value", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) // Create backend and register 3 relays. backend := newTestBackend(t, 3, time.Second) @@ -629,7 +629,7 @@ func TestGetHeaderBids(t *testing.T) { t.Run("Use header with lowest blockhash if same value", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) // Create backend and register 3 relays. backend := newTestBackend(t, 3, time.Second) @@ -683,7 +683,7 @@ func TestGetHeaderBids(t *testing.T) { t.Run("Respect minimum bid cutoff", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) // Create backend and register relay. backend := newTestBackend(t, 1, time.Second) @@ -709,7 +709,7 @@ func TestGetHeaderBids(t *testing.T) { t.Run("Allow bids which meet minimum bid cutoff", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) // Create backend and register relay. backend := newTestBackend(t, 1, time.Second) @@ -777,7 +777,7 @@ func TestGetPayload(t *testing.T) { t.Run("Okay response from relay", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) rr := backend.request(t, http.MethodPost, path, header, payload) @@ -792,7 +792,7 @@ func TestGetPayload(t *testing.T) { t.Run("Bad response from relays", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 2, time.Second) resp := &builderApi.VersionedSubmitBlindedBlockResponse{ @@ -824,7 +824,7 @@ func TestGetPayload(t *testing.T) { t.Run("Retries on error from relay", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, 2*time.Second) @@ -846,7 +846,7 @@ func TestGetPayload(t *testing.T) { t.Run("Error after max retries are reached", func(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) backend := newTestBackend(t, 1, time.Second) @@ -1016,7 +1016,7 @@ func denebExecutionPayloadAndBlobsBundle(header *deneb.ExecutionPayloadHeader, k func TestGetPayloadForks(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) //nolint: forcetypeassert,thelper tests := []struct { @@ -1085,7 +1085,7 @@ func TestGetPayloadForks(t *testing.T) { func TestGetPayloadToAllRelays(t *testing.T) { header := make(http.Header) - header.Set("Accept", "application/json") + header.Set(HeaderAccept, MediaTypeJSON) // Load the signed blinded beacon block used for getPayload jsonFile, err := os.Open("../testdata/signed-blinded-beacon-block-deneb.json") diff --git a/server/utils.go b/server/utils.go index 70ca747c..5580b4d5 100644 --- a/server/utils.go +++ b/server/utils.go @@ -27,9 +27,13 @@ import ( ) const ( - HeaderKeySlotUID = "X-MEVBoost-SlotID" - HeaderKeyVersion = "X-MEVBoost-Version" - HeaderStartTimeUnixMS = "X-MEVBoost-StartTimeUnixMS" + HeaderAccept = "Accept" + HeaderContentType = "Content-Type" + HeaderEthConsensusVersion = "Eth-Consensus-Version" + HeaderKeySlotUID = "X-MEVBoost-SlotID" + HeaderKeyVersion = "X-MEVBoost-Version" + HeaderStartTimeUnixMS = "X-MEVBoost-StartTimeUnixMS" + HeaderUserAgent = "User-Agent" ) var ( From 6f8b5381c6708adce79671ae63409c8a86772cf9 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 08:38:52 -0600 Subject: [PATCH 47/52] Rename media_type.go to constants.go & move header constants --- server/constants.go | 16 ++++++++++++++++ server/media_types.go | 6 ------ server/utils.go | 10 ---------- 3 files changed, 16 insertions(+), 16 deletions(-) create mode 100644 server/constants.go delete mode 100644 server/media_types.go diff --git a/server/constants.go b/server/constants.go new file mode 100644 index 00000000..23ce1938 --- /dev/null +++ b/server/constants.go @@ -0,0 +1,16 @@ +package server + +const ( + HeaderAccept = "Accept" + HeaderContentType = "Content-Type" + HeaderEthConsensusVersion = "Eth-Consensus-Version" + HeaderKeySlotUID = "X-MEVBoost-SlotID" + HeaderKeyVersion = "X-MEVBoost-Version" + HeaderStartTimeUnixMS = "X-MEVBoost-StartTimeUnixMS" + HeaderUserAgent = "User-Agent" +) + +const ( + MediaTypeJSON = "application/json" + MediaTypeOctetStream = "application/octet-stream" +) diff --git a/server/media_types.go b/server/media_types.go deleted file mode 100644 index be414971..00000000 --- a/server/media_types.go +++ /dev/null @@ -1,6 +0,0 @@ -package server - -const ( - MediaTypeJSON = "application/json" - MediaTypeOctetStream = "application/octet-stream" -) diff --git a/server/utils.go b/server/utils.go index 5580b4d5..a4c546ce 100644 --- a/server/utils.go +++ b/server/utils.go @@ -26,16 +26,6 @@ import ( "github.com/sirupsen/logrus" ) -const ( - HeaderAccept = "Accept" - HeaderContentType = "Content-Type" - HeaderEthConsensusVersion = "Eth-Consensus-Version" - HeaderKeySlotUID = "X-MEVBoost-SlotID" - HeaderKeyVersion = "X-MEVBoost-Version" - HeaderStartTimeUnixMS = "X-MEVBoost-StartTimeUnixMS" - HeaderUserAgent = "User-Agent" -) - var ( errHTTPErrorResponse = errors.New("HTTP error response") errInvalidForkVersion = errors.New("invalid fork version") From bdf4c6aed02a6b5fb49292ce83c1a044fe4ffa0e Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 08:41:23 -0600 Subject: [PATCH 48/52] Fix nits --- server/service.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/server/service.go b/server/service.go index d965357e..a1c907da 100644 --- a/server/service.go +++ b/server/service.go @@ -228,12 +228,12 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. log.Debug("handling request") // Get the user agent - ua := UserAgent(req.Header.Get("User-Agent")) + ua := UserAgent(req.Header.Get(HeaderUserAgent)) log = log.WithFields(logrus.Fields{"ua": ua}) // Additional header fields header := req.Header - header.Set("User-Agent", wrapUserAgent(ua)) + header.Set(HeaderUserAgent, wrapUserAgent(ua)) header.Set(HeaderStartTimeUnixMS, fmt.Sprintf("%d", time.Now().UTC().UnixMilli())) // Read the validator registrations @@ -258,10 +258,11 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. // handleGetHeader requests bids from the relays func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) { var ( - vars = mux.Vars(req) - parentHashHex = vars["parent_hash"] - pubkey = vars["pubkey"] - ua = UserAgent(req.Header.Get(HeaderUserAgent)) + vars = mux.Vars(req) + parentHashHex = vars["parent_hash"] + pubkey = vars["pubkey"] + ua = UserAgent(req.Header.Get(HeaderUserAgent)) + rawProposerAcceptContentTypes = req.Header.Get(HeaderAccept) parsedProposerAcceptContentTypes = goacceptheaders.Parse(rawProposerAcceptContentTypes) ) @@ -384,7 +385,7 @@ func (m *BoostService) handleGetPayload(w http.ResponseWriter, req *http.Request } // Read user agent for logging - userAgent := UserAgent(req.Header.Get("User-Agent")) + userAgent := UserAgent(req.Header.Get(HeaderUserAgent)) // New forks need to be added at the front of this array. // The ordering of the array conveys precedence of the decoders. From 93a4755d2a5c2c957ec1d24b56a239f405d45aa6 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 08:53:39 -0600 Subject: [PATCH 49/52] Add EthConsensusVersion constants --- server/constants.go | 7 +++++-- server/get_header.go | 16 ++++++++-------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/server/constants.go b/server/constants.go index 23ce1938..c49dd637 100644 --- a/server/constants.go +++ b/server/constants.go @@ -8,9 +8,12 @@ const ( HeaderKeyVersion = "X-MEVBoost-Version" HeaderStartTimeUnixMS = "X-MEVBoost-StartTimeUnixMS" HeaderUserAgent = "User-Agent" -) -const ( MediaTypeJSON = "application/json" MediaTypeOctetStream = "application/octet-stream" + + EthConsensusVersionBellatrix = "bellatrix" + EthConsensusVersionCapella = "capella" + EthConsensusVersionDeneb = "deneb" + EthConsensusVersionElectra = "electra" ) diff --git a/server/get_header.go b/server/get_header.go index 56e7c207..a5cba4e8 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -262,19 +262,19 @@ func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bi if ethConsensusVersion != "" { // Do SSZ decoding switch ethConsensusVersion { - case "bellatrix": + case EthConsensusVersionBellatrix: bid.Version = spec.DataVersionBellatrix bid.Bellatrix = new(builderApiBellatrix.SignedBuilderBid) return bid.Bellatrix.UnmarshalSSZ(respBytes) - case "capella": + case EthConsensusVersionCapella: bid.Version = spec.DataVersionCapella bid.Capella = new(builderApiCapella.SignedBuilderBid) return bid.Capella.UnmarshalSSZ(respBytes) - case "deneb": + case EthConsensusVersionDeneb: bid.Version = spec.DataVersionDeneb bid.Deneb = new(builderApiDeneb.SignedBuilderBid) return bid.Deneb.UnmarshalSSZ(respBytes) - case "electra": + case EthConsensusVersionElectra: bid.Version = spec.DataVersionElectra bid.Electra = new(builderApiElectra.SignedBuilderBid) return bid.Electra.UnmarshalSSZ(respBytes) @@ -310,16 +310,16 @@ func (m *BoostService) respondGetHeaderSSZ(w http.ResponseWriter, result *bidRes var sszData []byte switch result.response.Version { case spec.DataVersionBellatrix: - w.Header().Set(HeaderEthConsensusVersion, "bellatrix") + w.Header().Set(HeaderEthConsensusVersion, EthConsensusVersionBellatrix) sszData, err = result.response.Bellatrix.MarshalSSZ() case spec.DataVersionCapella: - w.Header().Set(HeaderEthConsensusVersion, "capella") + w.Header().Set(HeaderEthConsensusVersion, EthConsensusVersionCapella) sszData, err = result.response.Capella.MarshalSSZ() case spec.DataVersionDeneb: - w.Header().Set(HeaderEthConsensusVersion, "deneb") + w.Header().Set(HeaderEthConsensusVersion, EthConsensusVersionDeneb) sszData, err = result.response.Deneb.MarshalSSZ() case spec.DataVersionElectra: - w.Header().Set(HeaderEthConsensusVersion, "electra") + w.Header().Set(HeaderEthConsensusVersion, EthConsensusVersionElectra) sszData, err = result.response.Electra.MarshalSSZ() case spec.DataVersionUnknown, spec.DataVersionPhase0, spec.DataVersionAltair: err = errInvalidForkVersion From 14442d14a33fccda42c37c78b7e6b20cd3f84be4 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 08:55:20 -0600 Subject: [PATCH 50/52] Fix linter --- server/service_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/service_test.go b/server/service_test.go index e2a828ea..032cce18 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -370,7 +370,7 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) //nolint:testifylint }) t.Run("Two relays return JSON, mev-boost returns JSON", func(t *testing.T) { @@ -385,7 +385,7 @@ func TestGetHeader(t *testing.T) { require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) - require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) //nolint:testifylint }) t.Run("Accepts both with Q values", func(t *testing.T) { @@ -410,7 +410,7 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) //nolint:testifylint }) t.Run("Accepts both but prefers JSON", func(t *testing.T) { @@ -422,7 +422,7 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) + require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) //nolint:testifylint }) t.Run("Only accepts unsupported media type", func(t *testing.T) { From 1fcd5ff678da89b26f6206e2c4be9420ced835e9 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 09:04:30 -0600 Subject: [PATCH 51/52] Remove allBidsWereJSON logic --- server/service.go | 17 ----------------- server/service_test.go | 8 ++++---- 2 files changed, 4 insertions(+), 21 deletions(-) diff --git a/server/service.go b/server/service.go index a1c907da..5c24f0e9 100644 --- a/server/service.go +++ b/server/service.go @@ -328,23 +328,6 @@ func (m *BoostService) handleGetHeader(w http.ResponseWriter, req *http.Request) proposerPreferredContentType = MediaTypeJSON } - // If every relay returned the bid in JSON, that means that none - // of them support SSZ and this would always require extra conversions. - // In this situation, if the proposer still accepts JSON, respond to the - // getHeader in JSON so the proposer sends the getPayload request in JSON. - allBidsWereJSON := true - for _, relay := range result.relays { - if relay.SupportsSSZ { - log.Debug("there is a relay that supports SSZ") - allBidsWereJSON = false - break - } - } - if proposerPreferredContentType != MediaTypeJSON && allBidsWereJSON && parsedProposerAcceptContentTypes.Accepts(MediaTypeJSON) { - log.Debug("overriding the response content type to be JSON") - proposerPreferredContentType = MediaTypeJSON - } - // Respond appropriately if proposerPreferredContentType == MediaTypeJSON { log.Debug("responding with JSON") diff --git a/server/service_test.go b/server/service_test.go index 032cce18..e5d330d3 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -360,7 +360,7 @@ func TestGetHeader(t *testing.T) { require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) }) - t.Run("One relay returns JSON, mev-boost returns JSON", func(t *testing.T) { + t.Run("One relay returns JSON, mev-boost returns preferred SSZ", func(t *testing.T) { header := make(http.Header) header.Set(HeaderEthConsensusVersion, "deneb") header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") @@ -370,10 +370,10 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) //nolint:testifylint + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) //nolint:testifylint }) - t.Run("Two relays return JSON, mev-boost returns JSON", func(t *testing.T) { + t.Run("Two relays return JSON, mev-boost returns preferred SSZ", func(t *testing.T) { header := make(http.Header) header.Set(HeaderEthConsensusVersion, "deneb") header.Set(HeaderAccept, "application/octet-stream;q=1.0,application/json;q=0.9") @@ -385,7 +385,7 @@ func TestGetHeader(t *testing.T) { require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) - require.Equal(t, MediaTypeJSON, rr.Header().Get(HeaderContentType)) //nolint:testifylint + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) //nolint:testifylint }) t.Run("Accepts both with Q values", func(t *testing.T) { From a745a43055005a15584c24e68b42c38f2a988d65 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Mon, 17 Feb 2025 09:07:57 -0600 Subject: [PATCH 52/52] Fix lint --- server/service_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/service_test.go b/server/service_test.go index e5d330d3..db2e6cf3 100644 --- a/server/service_test.go +++ b/server/service_test.go @@ -370,7 +370,7 @@ func TestGetHeader(t *testing.T) { rr := backend.request(t, http.MethodGet, path, header, nil) require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) - require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) //nolint:testifylint + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) }) t.Run("Two relays return JSON, mev-boost returns preferred SSZ", func(t *testing.T) { @@ -385,7 +385,7 @@ func TestGetHeader(t *testing.T) { require.Equal(t, http.StatusOK, rr.Code, rr.Body.String()) require.Equal(t, 1, backend.relays[0].GetRequestCount(path)) require.Equal(t, 1, backend.relays[1].GetRequestCount(path)) - require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) //nolint:testifylint + require.Equal(t, MediaTypeOctetStream, rr.Header().Get(HeaderContentType)) }) t.Run("Accepts both with Q values", func(t *testing.T) {