From ba6dff3adb56ebf9fe46f308ae59cd7e4183f7fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rados=C5=82aw=20Kapka?= Date: Fri, 12 Apr 2024 19:24:30 +0900 Subject: [PATCH] Return syncing status when node is optimistic (#13875) --- beacon-chain/rpc/eth/node/handlers.go | 11 ++++++++--- beacon-chain/rpc/eth/node/handlers_test.go | 21 ++++++++++++++------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/beacon-chain/rpc/eth/node/handlers.go b/beacon-chain/rpc/eth/node/handlers.go index 6143fe435899..7ccae43ed4ad 100644 --- a/beacon-chain/rpc/eth/node/handlers.go +++ b/beacon-chain/rpc/eth/node/handlers.go @@ -108,7 +108,7 @@ func (*Server) GetVersion(w http.ResponseWriter, r *http.Request) { // GetHealth returns node health status in http status codes. Useful for load balancers. func (s *Server) GetHealth(w http.ResponseWriter, r *http.Request) { - _, span := trace.StartSpan(r.Context(), "node.GetHealth") + ctx, span := trace.StartSpan(r.Context(), "node.GetHealth") defer span.End() rawSyncingStatus, syncingStatus, ok := shared.UintFromQuery(w, r, "syncing_status", false) @@ -119,10 +119,14 @@ func (s *Server) GetHealth(w http.ResponseWriter, r *http.Request) { return } - if s.SyncChecker.Synced() { + optimistic, err := s.OptimisticModeFetcher.IsOptimistic(ctx) + if err != nil { + httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + } + if s.SyncChecker.Synced() && !optimistic { return } - if s.SyncChecker.Syncing() || s.SyncChecker.Initialized() { + if s.SyncChecker.Syncing() || optimistic { if rawSyncingStatus != "" { w.WriteHeader(intSyncingStatus) } else { @@ -130,5 +134,6 @@ func (s *Server) GetHealth(w http.ResponseWriter, r *http.Request) { } return } + w.WriteHeader(http.StatusServiceUnavailable) } diff --git a/beacon-chain/rpc/eth/node/handlers_test.go b/beacon-chain/rpc/eth/node/handlers_test.go index ce53b8d5ecd4..4b5d9bf9f396 100644 --- a/beacon-chain/rpc/eth/node/handlers_test.go +++ b/beacon-chain/rpc/eth/node/handlers_test.go @@ -91,8 +91,10 @@ func TestGetVersion(t *testing.T) { func TestGetHealth(t *testing.T) { checker := &syncmock.Sync{} + optimisticFetcher := &mock.ChainService{Optimistic: false} s := &Server{ - SyncChecker: checker, + SyncChecker: checker, + OptimisticModeFetcher: optimisticFetcher, } request := httptest.NewRequest(http.MethodGet, "http://example.com/eth/v1/node/health", nil) @@ -101,25 +103,30 @@ func TestGetHealth(t *testing.T) { s.GetHealth(writer, request) assert.Equal(t, http.StatusServiceUnavailable, writer.Code) - checker.IsInitialized = true - request = httptest.NewRequest(http.MethodGet, "http://example.com/eth/v1/node/health", nil) + checker.IsSyncing = true + checker.IsSynced = false + request = httptest.NewRequest(http.MethodGet, fmt.Sprintf("http://example.com/eth/v1/node/health?syncing_status=%d", http.StatusPaymentRequired), nil) writer = httptest.NewRecorder() writer.Body = &bytes.Buffer{} s.GetHealth(writer, request) - assert.Equal(t, http.StatusPartialContent, writer.Code) + assert.Equal(t, http.StatusPaymentRequired, writer.Code) - request = httptest.NewRequest(http.MethodGet, fmt.Sprintf("http://example.com/eth/v1/node/health?syncing_status=%d", http.StatusPaymentRequired), nil) + checker.IsSyncing = false + checker.IsSynced = true + request = httptest.NewRequest(http.MethodGet, "http://example.com/eth/v1/node/health", nil) writer = httptest.NewRecorder() writer.Body = &bytes.Buffer{} s.GetHealth(writer, request) - assert.Equal(t, http.StatusPaymentRequired, writer.Code) + assert.Equal(t, http.StatusOK, writer.Code) + checker.IsSyncing = false checker.IsSynced = true + optimisticFetcher.Optimistic = true request = httptest.NewRequest(http.MethodGet, "http://example.com/eth/v1/node/health", nil) writer = httptest.NewRecorder() writer.Body = &bytes.Buffer{} s.GetHealth(writer, request) - assert.Equal(t, http.StatusOK, writer.Code) + assert.Equal(t, http.StatusPartialContent, writer.Code) } func TestGetIdentity(t *testing.T) {