-
Notifications
You must be signed in to change notification settings - Fork 225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add getHeader SSZ support #734
base: develop
Are you sure you want to change the base?
Changes from 25 commits
8ed5f68
498d8fc
a0cc8a0
3005de8
3afd060
f65e83d
2073a41
b4b9088
b1cda56
097c625
db3f598
6545540
735bcbd
881cff0
8d1073f
5342231
db84842
1e5ff29
357212a
8e5b644
5014440
6171a13
a860bf2
5761355
e7d51de
25462d6
3f50e91
0cde21a
4c9f49b
7c27374
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,6 +30,7 @@ linters: | |
- goconst | ||
- gosec | ||
- ireturn | ||
- maintidx | ||
- noctx | ||
- tagliatelle | ||
- perfsprint | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package server | ||
|
||
import ( | ||
"sort" | ||
"strconv" | ||
"strings" | ||
) | ||
|
||
// 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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,19 @@ package server | |
|
||
import ( | ||
"context" | ||
"encoding/json" | ||
"fmt" | ||
"io" | ||
"net/http" | ||
"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" | ||
"github.com/flashbots/mev-boost/config" | ||
"github.com/flashbots/mev-boost/server/types" | ||
|
@@ -16,7 +23,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,11 +51,9 @@ 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()), | ||
} | ||
// Get the optional version, used with SSZ decoding | ||
ethConsensusVersion := header.Get("Eth-Consensus-Version") | ||
log = log.WithField("ethConsensusVersion", ethConsensusVersion) | ||
|
||
var ( | ||
mu sync.Mutex | ||
|
@@ -71,18 +76,60 @@ 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) | ||
|
||
// 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) | ||
// Make a new request | ||
req, err := http.NewRequestWithContext(context.Background(), http.MethodGet, url, nil) | ||
if err != nil { | ||
log.WithError(err).Warn("error making request to relay") | ||
log.WithError(err).Warn("error creating new request") | ||
return | ||
} | ||
if code == http.StatusNoContent { | ||
|
||
// Add headers from the request to this request. | ||
// This includes Accept and Eth-Consensus-Version, if provided. | ||
for key, values := range header { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my only worry here would be that relays don't properly handle q-weighted Accept headers, which is fine as long as they default to JSON in that case, if they throw an error it would be problematic though |
||
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 | ||
} | ||
defer resp.Body.Close() | ||
|
||
// 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 resp body content | ||
respBytes, err := io.ReadAll(resp.Body) | ||
if err != nil { | ||
log.WithError(err).Warn("error reading response body") | ||
return | ||
} | ||
|
||
// Get the response's content type | ||
respContentType := resp.Header.Get("Content-Type") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do all relays properly set this header, otherwise might be best to assume JSON if header is missing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure. Defaulting to JSON sounds like a good idea to me. Will make this change. |
||
log = log.WithField("respContentType", respContentType) | ||
|
||
// Decode bid | ||
bid := new(builderSpec.VersionedSignedBuilderBid) | ||
err = decodeBid(respBytes, respContentType, ethConsensusVersion, bid) | ||
if err != nil { | ||
log.WithError(err).Warn("error decoding bid") | ||
return | ||
} | ||
|
||
// Skip if bid is empty | ||
if bid.IsEmpty() { | ||
return | ||
|
@@ -189,3 +236,39 @@ 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 or JSON, depending on the provided respContentType | ||
func decodeBid(respBytes []byte, respContentType, ethConsensusVersion string, bid *builderSpec.VersionedSignedBuilderBid) error { | ||
switch respContentType { | ||
case MediaTypeOctetStream: | ||
if ethConsensusVersion != "" { | ||
// Do SSZ decoding | ||
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 | ||
} | ||
} else { | ||
return types.ErrMissingEthConsensusVersion | ||
} | ||
case MediaTypeJSON: | ||
// Do JSON decoding | ||
return json.Unmarshal(respBytes, bid) | ||
} | ||
return types.ErrInvalidContentType | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package server | ||
|
||
const ( | ||
MediaTypeJSON = "application/json" | ||
MediaTypeOctetStream = "application/octet-stream" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you need some test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've incorporated these into this PR. But with two changes. If there is no accept header or the accept header isn't in the supported media types, we will default to JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do the same although would return 406 if client only accepts media types we don't support, it's handled here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm yes. I like that idea. Will do the same.