From 4a1b7c39b3f4297ccf9d79e4a87e657b92da5e72 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 4 Feb 2025 19:37:48 -0600 Subject: [PATCH 1/8] Do not call relay monitor --- server/service.go | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/server/service.go b/server/service.go index 0df3b42b..33ca75c6 100644 --- a/server/service.go +++ b/server/service.go @@ -209,22 +209,6 @@ func (m *BoostService) startBidCacheCleanupTask() { } } -func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(payload []builderApiV1.SignedValidatorRegistration) { - log := m.log.WithField("method", "sendValidatorRegistrationsToRelayMonitors").WithField("numRegistrations", len(payload)) - for _, relayMonitor := range m.relayMonitors { - go func(relayMonitor *url.URL) { - url := types.GetURI(relayMonitor, params.PathRegisterValidator) - log = log.WithField("url", url) - _, err := SendHTTPRequest(context.Background(), m.httpClientRegVal, http.MethodPost, url, "", nil, payload, nil) - if err != nil { - log.WithError(err).Warn("error calling registerValidator on relay monitor") - return - } - log.Debug("sent validator registrations to relay monitor") - }(relayMonitor) - } -} - func (m *BoostService) handleRoot(w http.ResponseWriter, _ *http.Request) { m.respondOK(w, nilResponse) } @@ -277,8 +261,6 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. }(relay) } - go m.sendValidatorRegistrationsToRelayMonitors(payload) - for i := 0; i < len(m.relays); i++ { respErr := <-relayRespCh if respErr == nil { From b5ce901d2ad3dce06d9f38320268c23125b4a097 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 4 Feb 2025 20:56:37 -0600 Subject: [PATCH 2/8] Forward validator registrations without decoding --- server/get_header.go | 2 +- server/get_payload.go | 2 +- server/service.go | 71 +++++++++++++++++++++----------- server/types/relay_entry.go | 6 +-- server/types/relay_entry_test.go | 2 +- server/utils.go | 4 ++ 6 files changed, 58 insertions(+), 29 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 4bd9fb5c..81b2021f 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -73,7 +73,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, ua UserAgent, slot phase0.Sl // 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) + code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url.String(), ua, headers, nil, bid) if err != nil { log.WithError(err).Warn("error making request to relay") return diff --git a/server/get_payload.go b/server/get_payload.go index e8ea9f23..24eda311 100644 --- a/server/get_payload.go +++ b/server/get_payload.go @@ -103,7 +103,7 @@ func processPayload[P Payload](m *BoostService, log *logrus.Entry, ua UserAgent, log.Debug("calling getPayload") responsePayload := new(builderApi.VersionedSubmitBlindedBlockResponse) - _, err := SendHTTPRequestWithRetries(requestCtx, m.httpClientGetPayload, http.MethodPost, url, ua, headers, blindedBlock, responsePayload, m.requestMaxRetries, log) + _, err := SendHTTPRequestWithRetries(requestCtx, m.httpClientGetPayload, http.MethodPost, url.String(), ua, headers, blindedBlock, responsePayload, m.requestMaxRetries, log) if err != nil { if errors.Is(requestCtx.Err(), context.Canceled) { // This is expected if the payload has already been received by another relay diff --git a/server/service.go b/server/service.go index 33ca75c6..53623000 100644 --- a/server/service.go +++ b/server/service.go @@ -16,7 +16,6 @@ import ( "time" builderApi "github.com/attestantio/go-builder-client/api" - builderApiV1 "github.com/attestantio/go-builder-client/api/v1" eth2ApiV1Bellatrix "github.com/attestantio/go-eth2-client/api/v1/bellatrix" eth2ApiV1Capella "github.com/attestantio/go-eth2-client/api/v1/capella" eth2ApiV1Deneb "github.com/attestantio/go-eth2-client/api/v1/deneb" @@ -224,51 +223,77 @@ func (m *BoostService) handleStatus(w http.ResponseWriter, _ *http.Request) { } } -// handleRegisterValidator returns StatusOK if at least one relay returns StatusOK, else StatusBadGateway +// handleRegisterValidator returns StatusOK if at least one relay returns StatusOK, else StatusBadGateway. +// This forwards the message from the node to relays with minimal overhead. The registrations will maintain their +// original encoding (SSZ or JSON) from the node. func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http.Request) { log := m.log.WithField("method", "registerValidator") - log.Debug("registerValidator") - - payload := []builderApiV1.SignedValidatorRegistration{} - if err := DecodeJSON(req.Body, &payload); err != nil { - m.respondError(w, http.StatusBadRequest, err.Error()) - return - } + log.Debug("handling request") + // Get the user agent ua := UserAgent(req.Header.Get("User-Agent")) - log = log.WithFields(logrus.Fields{ - "numRegistrations": len(payload), - "ua": ua, - }) + log = log.WithFields(logrus.Fields{"ua": ua}) - // Add request headers + // Additional header fields headers := map[string]string{ + "User-Agent": wrapUserAgent(ua), HeaderStartTimeUnixMS: fmt.Sprintf("%d", time.Now().UTC().UnixMilli()), } - relayRespCh := make(chan error, len(m.relays)) + // Read the body bytes + bodyBytes, err := io.ReadAll(req.Body) + if err != nil { + m.respondError(w, http.StatusInternalServerError, err.Error()) + return + } + req.Body.Close() + // Forward request to each relay + respErrCh := make(chan error, len(m.relays)) for _, relay := range m.relays { go func(relay types.RelayEntry) { - url := relay.GetURI(params.PathRegisterValidator) - log := log.WithField("url", url) + // Build the new request + relayReq := req.Clone(req.Context()) + relayReq.URL = relay.GetURI(params.PathRegisterValidator) + relayReq.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + for key, value := range headers { + relayReq.Header.Set(key, value) + } - _, err := SendHTTPRequest(context.Background(), m.httpClientRegVal, http.MethodPost, url, ua, headers, payload, nil) + // Create a new logger with this request URL + log := log.WithField("url", relayReq.URL) + + // Send the request + resp, err := m.httpClientGetHeader.Do(relayReq) if err != nil { log.WithError(err).Warn("error calling registerValidator on relay") + respErrCh <- err + return + } + resp.Body.Close() + + // Check if response is successful + if resp.StatusCode == http.StatusOK { + respErrCh <- nil + } else { + respErrCh <- fmt.Errorf("%w: %d", errHTTPErrorResponse, resp.StatusCode) } - relayRespCh <- err }(relay) } - for i := 0; i < len(m.relays); i++ { - respErr := <-relayRespCh + // Return OK if any relay responds OK + for range m.relays { + respErr := <-respErrCh if respErr == nil { - m.respondOK(w, nilResponse) + w.WriteHeader(http.StatusOK) + // Goroutines are independent, so if there are a lot of configured + // relays and the first one responds OK, this will continue to send + // validator registrations to the other relays. return } } + // None of the relays responded OK m.respondError(w, http.StatusBadGateway, errNoSuccessfulRelayResponse.Error()) } @@ -432,7 +457,7 @@ func (m *BoostService) CheckRelays() int { log := m.log.WithField("url", url) log.Debug("checking relay status") - code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url, "", nil, nil, nil) + code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url.String(), "", nil, nil, nil) if err != nil { log.WithError(err).Error("relay status error - request failed") return diff --git a/server/types/relay_entry.go b/server/types/relay_entry.go index 3ad7ac93..b82e4f5c 100644 --- a/server/types/relay_entry.go +++ b/server/types/relay_entry.go @@ -19,15 +19,15 @@ func (r *RelayEntry) String() string { } // GetURI returns the full request URI with scheme, host, path and args. -func GetURI(url *url.URL, path string) string { +func GetURI(url *url.URL, path string) *url.URL { u2 := *url u2.User = nil u2.Path = path - return u2.String() + return &u2 } // GetURI returns the full request URI with scheme, host, path and args for the relay. -func (r *RelayEntry) GetURI(path string) string { +func (r *RelayEntry) GetURI(path string) *url.URL { return GetURI(r.URL, path) } diff --git a/server/types/relay_entry_test.go b/server/types/relay_entry_test.go index b3b601fd..9ae29221 100644 --- a/server/types/relay_entry_test.go +++ b/server/types/relay_entry_test.go @@ -102,7 +102,7 @@ func TestParseRelaysURLs(t *testing.T) { // Now perform content assertions. if tt.expectedErr == nil { - require.Equal(t, tt.expectedURI, relayEntry.GetURI(tt.path)) + require.Equal(t, tt.expectedURI, relayEntry.GetURI(tt.path).String()) require.Equal(t, tt.expectedPublicKey, relayEntry.PublicKey.String()) require.Equal(t, tt.expectedURL, relayEntry.String()) } 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 32f0b2aa387181ca3223dc4f322bf0dd13ebd7d7 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 07:35:15 -0600 Subject: [PATCH 3/8] Add back sendValidatorRegistrationsToRelayMonitors --- server/service.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/server/service.go b/server/service.go index 53623000..6e15f495 100644 --- a/server/service.go +++ b/server/service.go @@ -208,6 +208,22 @@ func (m *BoostService) startBidCacheCleanupTask() { } } +func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(payloadBytes []byte) { + log := m.log.WithField("method", "sendValidatorRegistrationsToRelayMonitors").WithField("registrationsLen", len(payloadBytes)) + for _, relayMonitor := range m.relayMonitors { + go func(relayMonitor *url.URL) { + url := types.GetURI(relayMonitor, params.PathRegisterValidator) + log = log.WithField("url", url) + _, err := SendHTTPRequest(context.Background(), m.httpClientRegVal, http.MethodPost, url.String(), "", nil, payloadBytes, nil) + if err != nil { + log.WithError(err).Warn("error calling registerValidator on relay monitor") + return + } + log.Debug("sent validator registrations to relay monitor") + }(relayMonitor) + } +} + func (m *BoostService) handleRoot(w http.ResponseWriter, _ *http.Request) { m.respondOK(w, nilResponse) } @@ -281,6 +297,8 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. }(relay) } + go m.sendValidatorRegistrationsToRelayMonitors(bodyBytes) + // Return OK if any relay responds OK for range m.relays { respErr := <-respErrCh From 3de02f575e675f22d2bfc1f16942ff6ff3fbaa30 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 07:37:40 -0600 Subject: [PATCH 4/8] Rename payloadBytes/bodyBytes -> regBytes --- server/service.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/server/service.go b/server/service.go index 6e15f495..230884f6 100644 --- a/server/service.go +++ b/server/service.go @@ -208,13 +208,13 @@ func (m *BoostService) startBidCacheCleanupTask() { } } -func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(payloadBytes []byte) { - log := m.log.WithField("method", "sendValidatorRegistrationsToRelayMonitors").WithField("registrationsLen", len(payloadBytes)) +func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(regBytes []byte) { + log := m.log.WithField("method", "sendValidatorRegistrationsToRelayMonitors").WithField("registrationsLen", len(regBytes)) for _, relayMonitor := range m.relayMonitors { go func(relayMonitor *url.URL) { url := types.GetURI(relayMonitor, params.PathRegisterValidator) log = log.WithField("url", url) - _, err := SendHTTPRequest(context.Background(), m.httpClientRegVal, http.MethodPost, url.String(), "", nil, payloadBytes, nil) + _, err := SendHTTPRequest(context.Background(), m.httpClientRegVal, http.MethodPost, url.String(), "", nil, regBytes, nil) if err != nil { log.WithError(err).Warn("error calling registerValidator on relay monitor") return @@ -256,8 +256,8 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. HeaderStartTimeUnixMS: fmt.Sprintf("%d", time.Now().UTC().UnixMilli()), } - // Read the body bytes - bodyBytes, err := io.ReadAll(req.Body) + // Read the validator registrations + regBytes, err := io.ReadAll(req.Body) if err != nil { m.respondError(w, http.StatusInternalServerError, err.Error()) return @@ -271,7 +271,7 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. // Build the new request relayReq := req.Clone(req.Context()) relayReq.URL = relay.GetURI(params.PathRegisterValidator) - relayReq.Body = io.NopCloser(bytes.NewReader(bodyBytes)) + relayReq.Body = io.NopCloser(bytes.NewReader(regBytes)) for key, value := range headers { relayReq.Header.Set(key, value) } @@ -297,7 +297,8 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. }(relay) } - go m.sendValidatorRegistrationsToRelayMonitors(bodyBytes) + // Send the registrations to relay monitors, if configured + go m.sendValidatorRegistrationsToRelayMonitors(regBytes) // Return OK if any relay responds OK for range m.relays { From a6487bebb0693d3d5edd3b031f834752fb13d060 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 08:24:57 -0600 Subject: [PATCH 5/8] Move functionality to new register_validator.go file --- server/register_validator.go | 99 ++++++++++++++++++++++++++++++++++++ server/service.go | 81 +++++------------------------ 2 files changed, 112 insertions(+), 68 deletions(-) create mode 100644 server/register_validator.go diff --git a/server/register_validator.go b/server/register_validator.go new file mode 100644 index 00000000..32215901 --- /dev/null +++ b/server/register_validator.go @@ -0,0 +1,99 @@ +package server + +import ( + "bytes" + "context" + "fmt" + "net/http" + "net/url" + + "github.com/flashbots/mev-boost/server/params" + "github.com/flashbots/mev-boost/server/types" + "github.com/sirupsen/logrus" +) + +func (m *BoostService) registerValidator(log *logrus.Entry, regBytes []byte, header http.Header) error { + respErrCh := make(chan error, len(m.relays)) + + // Forward request to each relay + for _, relay := range m.relays { + go func(relay types.RelayEntry) { + // Get the URL for this relay + requestURL := relay.GetURI(params.PathRegisterValidator) + log := log.WithField("url", requestURL) + + // Build the new request + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL.String(), bytes.NewReader(regBytes)) + 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 request + resp, err := m.httpClientRegVal.Do(req) + if err != nil { + log.WithError(err).Warn("error calling registerValidator on relay") + respErrCh <- err + return + } + resp.Body.Close() + + // Check if response is successful + if resp.StatusCode == http.StatusOK { + respErrCh <- nil + } else { + respErrCh <- fmt.Errorf("%w: %d", errHTTPErrorResponse, resp.StatusCode) + } + }(relay) + } + + // Return OK if any relay responds OK + for range m.relays { + respErr := <-respErrCh + if respErr == nil { + // Goroutines are independent, so if there are a lot of configured + // relays and the first one responds OK, this will continue to send + // validator registrations to the other relays. + return nil + } + } + + // None of the relays responded OK + return errNoSuccessfulRelayResponse +} + +func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(log *logrus.Entry, regBytes []byte, header http.Header) { + // Forward request to each relay monitor + for _, relayMonitor := range m.relayMonitors { + go func(relayMonitor *url.URL) { + // Get the URL for this relay monitor + requestURL := types.GetURI(relayMonitor, params.PathRegisterValidator) + log := log.WithField("url", requestURL) + + // Build the new request + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL.String(), bytes.NewReader(regBytes)) + 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 request + resp, err := m.httpClientRegVal.Do(req) + if err != nil { + log.WithError(err).Warn("error calling registerValidator on relay monitor") + return + } + resp.Body.Close() + }(relayMonitor) + } +} diff --git a/server/service.go b/server/service.go index 230884f6..5b7ab094 100644 --- a/server/service.go +++ b/server/service.go @@ -208,22 +208,6 @@ func (m *BoostService) startBidCacheCleanupTask() { } } -func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(regBytes []byte) { - log := m.log.WithField("method", "sendValidatorRegistrationsToRelayMonitors").WithField("registrationsLen", len(regBytes)) - for _, relayMonitor := range m.relayMonitors { - go func(relayMonitor *url.URL) { - url := types.GetURI(relayMonitor, params.PathRegisterValidator) - log = log.WithField("url", url) - _, err := SendHTTPRequest(context.Background(), m.httpClientRegVal, http.MethodPost, url.String(), "", nil, regBytes, nil) - if err != nil { - log.WithError(err).Warn("error calling registerValidator on relay monitor") - return - } - log.Debug("sent validator registrations to relay monitor") - }(relayMonitor) - } -} - func (m *BoostService) handleRoot(w http.ResponseWriter, _ *http.Request) { m.respondOK(w, nilResponse) } @@ -251,10 +235,9 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. log = log.WithFields(logrus.Fields{"ua": ua}) // Additional header fields - headers := map[string]string{ - "User-Agent": wrapUserAgent(ua), - HeaderStartTimeUnixMS: fmt.Sprintf("%d", time.Now().UTC().UnixMilli()), - } + header := req.Header + header.Set("User-Agent", wrapUserAgent(ua)) + header.Set(HeaderStartTimeUnixMS, fmt.Sprintf("%d", time.Now().UTC().UnixMilli())) // Read the validator registrations regBytes, err := io.ReadAll(req.Body) @@ -264,56 +247,18 @@ func (m *BoostService) handleRegisterValidator(w http.ResponseWriter, req *http. } req.Body.Close() - // Forward request to each relay - respErrCh := make(chan error, len(m.relays)) - for _, relay := range m.relays { - go func(relay types.RelayEntry) { - // Build the new request - relayReq := req.Clone(req.Context()) - relayReq.URL = relay.GetURI(params.PathRegisterValidator) - relayReq.Body = io.NopCloser(bytes.NewReader(regBytes)) - for key, value := range headers { - relayReq.Header.Set(key, value) - } - - // Create a new logger with this request URL - log := log.WithField("url", relayReq.URL) - - // Send the request - resp, err := m.httpClientGetHeader.Do(relayReq) - if err != nil { - log.WithError(err).Warn("error calling registerValidator on relay") - respErrCh <- err - return - } - resp.Body.Close() - - // Check if response is successful - if resp.StatusCode == http.StatusOK { - respErrCh <- nil - } else { - respErrCh <- fmt.Errorf("%w: %d", errHTTPErrorResponse, resp.StatusCode) - } - }(relay) - } - // Send the registrations to relay monitors, if configured - go m.sendValidatorRegistrationsToRelayMonitors(regBytes) - - // Return OK if any relay responds OK - for range m.relays { - respErr := <-respErrCh - if respErr == nil { - w.WriteHeader(http.StatusOK) - // Goroutines are independent, so if there are a lot of configured - // relays and the first one responds OK, this will continue to send - // validator registrations to the other relays. - return - } - } + go m.sendValidatorRegistrationsToRelayMonitors(log, regBytes, header) - // None of the relays responded OK - m.respondError(w, http.StatusBadGateway, errNoSuccessfulRelayResponse.Error()) + // Send the registrations to each relay + err = m.registerValidator(log, regBytes, header) + if err == nil { + // One of the relays responded OK + m.respondOK(w, nilResponse) + } else { + // None of the relays responded OK + m.respondError(w, http.StatusBadGateway, err.Error()) + } } // handleGetHeader requests bids from the relays From 8a52e7731d5d70330ed6e126229620471fd36acb Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 08:27:49 -0600 Subject: [PATCH 6/8] Revert change to GetURI --- server/get_header.go | 2 +- server/get_payload.go | 2 +- server/register_validator.go | 4 ++-- server/service.go | 2 +- server/types/relay_entry.go | 6 +++--- server/types/relay_entry_test.go | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/server/get_header.go b/server/get_header.go index 81b2021f..4bd9fb5c 100644 --- a/server/get_header.go +++ b/server/get_header.go @@ -73,7 +73,7 @@ func (m *BoostService) getHeader(log *logrus.Entry, ua UserAgent, slot phase0.Sl // Send the get bid request to the relay bid := new(builderSpec.VersionedSignedBuilderBid) - code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url.String(), ua, headers, nil, bid) + code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url, ua, headers, nil, bid) if err != nil { log.WithError(err).Warn("error making request to relay") return diff --git a/server/get_payload.go b/server/get_payload.go index 24eda311..e8ea9f23 100644 --- a/server/get_payload.go +++ b/server/get_payload.go @@ -103,7 +103,7 @@ func processPayload[P Payload](m *BoostService, log *logrus.Entry, ua UserAgent, log.Debug("calling getPayload") responsePayload := new(builderApi.VersionedSubmitBlindedBlockResponse) - _, err := SendHTTPRequestWithRetries(requestCtx, m.httpClientGetPayload, http.MethodPost, url.String(), ua, headers, blindedBlock, responsePayload, m.requestMaxRetries, log) + _, err := SendHTTPRequestWithRetries(requestCtx, m.httpClientGetPayload, http.MethodPost, url, ua, headers, blindedBlock, responsePayload, m.requestMaxRetries, log) if err != nil { if errors.Is(requestCtx.Err(), context.Canceled) { // This is expected if the payload has already been received by another relay diff --git a/server/register_validator.go b/server/register_validator.go index 32215901..68b6b1db 100644 --- a/server/register_validator.go +++ b/server/register_validator.go @@ -23,7 +23,7 @@ func (m *BoostService) registerValidator(log *logrus.Entry, regBytes []byte, hea log := log.WithField("url", requestURL) // Build the new request - req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL.String(), bytes.NewReader(regBytes)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL, bytes.NewReader(regBytes)) if err != nil { log.WithError(err).Warn("error creating new request") return @@ -76,7 +76,7 @@ func (m *BoostService) sendValidatorRegistrationsToRelayMonitors(log *logrus.Ent log := log.WithField("url", requestURL) // Build the new request - req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL.String(), bytes.NewReader(regBytes)) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL, bytes.NewReader(regBytes)) if err != nil { log.WithError(err).Warn("error creating new request") return diff --git a/server/service.go b/server/service.go index 5b7ab094..93c3c19a 100644 --- a/server/service.go +++ b/server/service.go @@ -421,7 +421,7 @@ func (m *BoostService) CheckRelays() int { log := m.log.WithField("url", url) log.Debug("checking relay status") - code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url.String(), "", nil, nil, nil) + code, err := SendHTTPRequest(context.Background(), m.httpClientGetHeader, http.MethodGet, url, "", nil, nil, nil) if err != nil { log.WithError(err).Error("relay status error - request failed") return diff --git a/server/types/relay_entry.go b/server/types/relay_entry.go index b82e4f5c..3ad7ac93 100644 --- a/server/types/relay_entry.go +++ b/server/types/relay_entry.go @@ -19,15 +19,15 @@ func (r *RelayEntry) String() string { } // GetURI returns the full request URI with scheme, host, path and args. -func GetURI(url *url.URL, path string) *url.URL { +func GetURI(url *url.URL, path string) string { u2 := *url u2.User = nil u2.Path = path - return &u2 + return u2.String() } // GetURI returns the full request URI with scheme, host, path and args for the relay. -func (r *RelayEntry) GetURI(path string) *url.URL { +func (r *RelayEntry) GetURI(path string) string { return GetURI(r.URL, path) } diff --git a/server/types/relay_entry_test.go b/server/types/relay_entry_test.go index 9ae29221..b3b601fd 100644 --- a/server/types/relay_entry_test.go +++ b/server/types/relay_entry_test.go @@ -102,7 +102,7 @@ func TestParseRelaysURLs(t *testing.T) { // Now perform content assertions. if tt.expectedErr == nil { - require.Equal(t, tt.expectedURI, relayEntry.GetURI(tt.path).String()) + require.Equal(t, tt.expectedURI, relayEntry.GetURI(tt.path)) require.Equal(t, tt.expectedPublicKey, relayEntry.PublicKey.String()) require.Equal(t, tt.expectedURL, relayEntry.String()) } From 31bd723579d502cd38f5cfa1ae84f2bccfa33299 Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Wed, 5 Feb 2025 08:37:47 -0600 Subject: [PATCH 7/8] Fix mistake --- server/register_validator.go | 1 + 1 file changed, 1 insertion(+) diff --git a/server/register_validator.go b/server/register_validator.go index 68b6b1db..6a570bd8 100644 --- a/server/register_validator.go +++ b/server/register_validator.go @@ -26,6 +26,7 @@ func (m *BoostService) registerValidator(log *logrus.Entry, regBytes []byte, hea req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, requestURL, bytes.NewReader(regBytes)) if err != nil { log.WithError(err).Warn("error creating new request") + respErrCh <- err return } From 08d3702a66bd08d7eb8683830533812be411f94a Mon Sep 17 00:00:00 2001 From: Justin Traglia Date: Tue, 11 Feb 2025 20:35:34 -0600 Subject: [PATCH 8/8] Add some basic validator registration tests --- server/mock/mock_relay.go | 21 ++- server/register_validator_test.go | 298 ++++++++++++++++++++++++++++++ 2 files changed, 313 insertions(+), 6 deletions(-) create mode 100644 server/register_validator_test.go diff --git a/server/mock/mock_relay.go b/server/mock/mock_relay.go index a93462fc..1e812807 100644 --- a/server/mock/mock_relay.go +++ b/server/mock/mock_relay.go @@ -158,12 +158,21 @@ func (m *Relay) handleRegisterValidator(w http.ResponseWriter, req *http.Request // defaultHandleRegisterValidator returns the default handler for handleRegisterValidator func (m *Relay) defaultHandleRegisterValidator(w http.ResponseWriter, req *http.Request) { - payload := []builderApiV1.SignedValidatorRegistration{} - decoder := json.NewDecoder(req.Body) - decoder.DisallowUnknownFields() - if err := decoder.Decode(&payload); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - return + reqContentType := req.Header.Get("Content-Type") + if reqContentType == "" || reqContentType == "application/json" { + var payload []builderApiV1.SignedValidatorRegistration + decoder := json.NewDecoder(req.Body) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&payload); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + } else if reqContentType == "application/octet-stream" { + // TODO(jtraglia): Handle this when a SignedValidatorRegistrationList type exists. + // See: https://github.com/attestantio/go-builder-client/pull/38 + _ = reqContentType + } else { + panic("invalid content type: " + reqContentType) } w.Header().Set("Content-Type", "application/json") diff --git a/server/register_validator_test.go b/server/register_validator_test.go new file mode 100644 index 00000000..5ba6accf --- /dev/null +++ b/server/register_validator_test.go @@ -0,0 +1,298 @@ +// register_validator_test.go +package server + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + builderApiV1 "github.com/attestantio/go-builder-client/api/v1" + "github.com/flashbots/mev-boost/server/mock" + "github.com/flashbots/mev-boost/server/params" + "github.com/flashbots/mev-boost/server/types" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/require" +) + +// TestHandleRegisterValidator_EmptyList verifies that a valid registration returns status ok +func TestHandleRegisterValidator_EmptyList(t *testing.T) { + relay := mock.NewRelay(t) + defer relay.Server.Close() + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("[]") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusOK, rr.Code, "expected status ok") + + count := relay.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, count) +} + +// TestHandleRegisterValidator_NotEmptyList verifies that a non-empty list returns status ok +func TestHandleRegisterValidator_NotEmptyList(t *testing.T) { + relay := mock.NewRelay(t) + defer relay.Server.Close() + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + validatorRegistrations := []builderApiV1.SignedValidatorRegistration{ + { + Message: &builderApiV1.ValidatorRegistration{ + Timestamp: time.Unix(1, 0), + }, + }, + { + Message: &builderApiV1.ValidatorRegistration{ + Timestamp: time.Unix(2, 0), + }, + }, + } + + encodedValidatorRegistrations, err := json.Marshal(validatorRegistrations) + require.NoError(t, err) + + reqBody := bytes.NewBuffer(encodedValidatorRegistrations) + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusOK, rr.Code, "expected status ok") + + count := relay.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, count) +} + +// TestHandleRegisterValidator_InvalidJSON verifies that an invalid registration returns bad gateway +func TestHandleRegisterValidator_InvalidJSON(t *testing.T) { + relay := mock.NewRelay(t) + defer relay.Server.Close() + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("invalid json") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusBadGateway, rr.Code) + + count := relay.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, count) +} + +// TestHandleRegisterValidator_ValidSSZ verifies that a valid registration returns status ok +func TestHandleRegisterValidator_ValidSSZ(t *testing.T) { + relay := mock.NewRelay(t) + defer relay.Server.Close() + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + validatorRegistrations := []builderApiV1.SignedValidatorRegistration{ + { + Message: &builderApiV1.ValidatorRegistration{ + Timestamp: time.Unix(1, 0), + }, + }, + { + Message: &builderApiV1.ValidatorRegistration{ + Timestamp: time.Unix(2, 0), + }, + }, + } + + // TODO(jtraglia): Use SSZ here when a SignedValidatorRegistrationList type exists. + // See: https://github.com/attestantio/go-builder-client/pull/38 + encodedValidatorRegistrations, err := json.Marshal(validatorRegistrations) + require.NoError(t, err) + + reqBody := bytes.NewBuffer(encodedValidatorRegistrations) + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/octet-stream") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + + count := relay.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, count) +} + +// TestHandleRegisterValidator_InvalidSSZ verifies that an invalid registration returns bad gateway +func TestHandleRegisterValidator_InvalidSSZ(t *testing.T) { + relay := mock.NewRelay(t) + defer relay.Server.Close() + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("invalid ssz") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/octet-stream") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + // TODO(jtraglia): Enable this when a SignedValidatorRegistrationList type exists. + // See: https://github.com/attestantio/go-builder-client/pull/38 + // require.Equal(t, http.StatusBadGateway, rr.Code) + + count := relay.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, count) +} + +// TestHandleRegisterValidator_MultipleRelaysOneSuccess verifies that if one relay succeeds the response is ok +func TestHandleRegisterValidator_MultipleRelaysOneSuccess(t *testing.T) { + badRelay := mock.NewRelay(t) + defer badRelay.Server.Close() + badRelay.OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "simulated failure", http.StatusInternalServerError) + }) + + relaySuccess := mock.NewRelay(t) + defer relaySuccess.Server.Close() + + m := &BoostService{ + relays: []types.RelayEntry{badRelay.RelayEntry, relaySuccess.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("[]") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + + countBadRelay := badRelay.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, countBadRelay) + countSuccess := relaySuccess.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, countSuccess) +} + +// TestHandleRegisterValidator_AllFail verifies that if all relays fail the response is bad gateway +func TestHandleRegisterValidator_AllFail(t *testing.T) { + badRelay1 := mock.NewRelay(t) + defer badRelay1.Server.Close() + badRelay1.OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "simulated failure 1", http.StatusInternalServerError) + }) + + badRelay2 := mock.NewRelay(t) + defer badRelay2.Server.Close() + badRelay2.OverrideHandleRegisterValidator(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "simulated failure 2", http.StatusInternalServerError) + }) + + m := &BoostService{ + relays: []types.RelayEntry{badRelay1.RelayEntry, badRelay2.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("[]") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusBadGateway, rr.Code) + + countBadRelay1 := badRelay1.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, countBadRelay1) + countBadRelay2 := badRelay2.GetRequestCount(params.PathRegisterValidator) + require.Equal(t, 1, countBadRelay2) +} + +// TestHandleRegisterValidator_RelayNetworkError verifies that a network error results in bad gateway +func TestHandleRegisterValidator_RelayNetworkError(t *testing.T) { + relay := mock.NewRelay(t) + relay.Server.Close() // simulate network error + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("[]") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusBadGateway, rr.Code) +} + +// TestHandleRegisterValidator_HeaderPropagation verifies that headers from the request are forwarded +func TestHandleRegisterValidator_HeaderPropagation(t *testing.T) { + relay := mock.NewRelay(t) + defer relay.Server.Close() + + headerChan := make(chan http.Header, 1) + relay.OverrideHandleRegisterValidator(func(w http.ResponseWriter, req *http.Request) { + headerChan <- req.Header + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + }) + + m := &BoostService{ + relays: []types.RelayEntry{relay.RelayEntry}, + httpClientRegVal: *http.DefaultClient, + log: logrus.NewEntry(logrus.New()), + } + + reqBody := bytes.NewBufferString("[]") + req := httptest.NewRequest(http.MethodPost, "https://example.com"+params.PathRegisterValidator, reqBody) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-Custom-Header", "custom-value") + + rr := httptest.NewRecorder() + m.handleRegisterValidator(rr, req) + + require.Equal(t, http.StatusOK, rr.Code) + + select { + case capturedHeader := <-headerChan: + require.Equal(t, "custom-value", capturedHeader.Get("X-Custom-Header")) + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for header capture") + } +}