diff --git a/cl/beacon/beaconhttp/api.go b/cl/beacon/beaconhttp/api.go index 87167e4d69f..303ec987ac2 100644 --- a/cl/beacon/beaconhttp/api.go +++ b/cl/beacon/beaconhttp/api.go @@ -27,6 +27,7 @@ import ( "strconv" "strings" + "github.com/erigontech/erigon/cl/beacon/synced_data" "github.com/erigontech/erigon/cl/phase1/forkchoice/fork_graph" "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/common/ssz" @@ -47,13 +48,23 @@ var ErrorCantFindBeaconState = errors.New("Could not find beacon state") var ErrorSszNotSupported = errors.New("This endpoint does not support SSZ response") func WrapEndpointError(err error) *EndpointError { - e := &EndpointError{} - if errors.As(err, e) { - return e + // Handlers build these with NewEndpointError, so the pointer form is the one that carries a + // deliberate code; matching only the value form would silently turn it into a 500. + var byPointer *EndpointError + if errors.As(err, &byPointer) { + return byPointer + } + byValue := EndpointError{} + if errors.As(err, &byValue) { + return &byValue } if errors.Is(err, fork_graph.ErrStateNotFound) { return NewEndpointError(http.StatusNotFound, ErrorCantFindBeaconState) } + // A node without a head state is transiently unavailable, not faulty. + if errors.Is(err, synced_data.ErrNotSynced) { + return NewEndpointError(http.StatusServiceUnavailable, err) + } return NewEndpointError(http.StatusInternalServerError, err) } diff --git a/cl/beacon/beaconhttp/api_error_test.go b/cl/beacon/beaconhttp/api_error_test.go new file mode 100644 index 00000000000..b36c29a7422 --- /dev/null +++ b/cl/beacon/beaconhttp/api_error_test.go @@ -0,0 +1,47 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package beaconhttp + +import ( + "errors" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/cl/beacon/synced_data" + "github.com/erigontech/erigon/cl/phase1/forkchoice/fork_graph" +) + +func TestWrapEndpointErrorStatusCodes(t *testing.T) { + for _, tc := range []struct { + name string + err error + code int + }{ + {"not synced", synced_data.ErrNotSynced, http.StatusServiceUnavailable}, + {"not synced, wrapped", fmt.Errorf("attester duties: %w", synced_data.ErrNotSynced), http.StatusServiceUnavailable}, + {"state not found", fork_graph.ErrStateNotFound, http.StatusNotFound}, + {"anything else", errors.New("boom"), http.StatusInternalServerError}, + {"explicit code is preserved", NewEndpointError(http.StatusBadRequest, errors.New("bad")), http.StatusBadRequest}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.code, WrapEndpointError(tc.err).Code) + }) + } +} diff --git a/cl/beacon/handler/handler.go b/cl/beacon/handler/handler.go index d0d04dd42f3..1d082e109cb 100644 --- a/cl/beacon/handler/handler.go +++ b/cl/beacon/handler/handler.go @@ -119,7 +119,7 @@ type ApiHandler struct { elClientVersion atomic.Pointer[engine_types.ClientVersionV1] // Cached execution client version for default graffiti. elClientVersionFetching atomic.Bool // Guards a single in-flight background elClientVersion fetch. syncMessagePool sync_contribution_pool.SyncContributionPool - committeeSub *committee_subscription.CommitteeSubscribeMgmt + committeeSub committee_subscription.CommitteeSubscribe attestationProducer attestation_producer.AttestationDataProducer slotWaitedForAttestationProduction *lru.Cache[uint64, struct{}] aggregatePool aggregation.AggregationPool @@ -177,7 +177,7 @@ func NewApiHandler( attestationProducer attestation_producer.AttestationDataProducer, engine execution_client.ExecutionEngine, syncMessagePool sync_contribution_pool.SyncContributionPool, - committeeSub *committee_subscription.CommitteeSubscribeMgmt, + committeeSub committee_subscription.CommitteeSubscribe, aggregatePool aggregation.AggregationPool, syncCommitteeMessagesService services.SyncCommitteeMessagesService, syncContributionAndProofs services.SyncContributionService, diff --git a/cl/beacon/handler/subscription.go b/cl/beacon/handler/subscription.go index cc7802d9509..eb8ef5a3490 100644 --- a/cl/beacon/handler/subscription.go +++ b/cl/beacon/handler/subscription.go @@ -26,6 +26,7 @@ import ( "time" "github.com/erigontech/erigon/cl/beacon/beaconhttp" + "github.com/erigontech/erigon/cl/beacon/synced_data" "github.com/erigontech/erigon/cl/cltypes" "github.com/erigontech/erigon/cl/gossip" "github.com/erigontech/erigon/cl/phase1/core/state" @@ -67,12 +68,6 @@ func (a *ApiHandler) PostEthV1ValidatorSyncCommitteeSubscriptions(w http.Respons syncnets = append(syncnets, uint64(i)) } } else { - // headState, cn := a.syncedData.HeadState() - // defer cn() - // if headState == nil { - // http.Error(w, "head state not available", http.StatusServiceUnavailable) - // return - // } if err := a.syncedData.ViewHeadState(func(headState *state.CachingBeaconState) error { syncnets, err = subnets.ComputeSubnetsForSyncCommittee(headState, subRequest.ValidatorIndex) if err != nil { @@ -80,10 +75,9 @@ func (a *ApiHandler) PostEthV1ValidatorSyncCommitteeSubscriptions(w http.Respons } return nil }); err != nil { - beaconhttp.NewEndpointError(http.StatusInternalServerError, err).WriteTo(w) + beaconhttp.WrapEndpointError(err).WriteTo(w) return } - //cn() } // subscribe to subnets @@ -118,8 +112,10 @@ func (a *ApiHandler) PostEthV1ValidatorBeaconCommitteeSubscription(w http.Respon } for _, sub := range req { if err := a.committeeSub.AddAttestationSubscription(context.Background(), sub); err != nil { - log.Error("failed to add attestation subscription", "err", err) - beaconhttp.NewEndpointError(http.StatusInternalServerError, err).WriteTo(w) + if !errors.Is(err, synced_data.ErrNotSynced) { + log.Error("failed to add attestation subscription", "err", err) + } + beaconhttp.WrapEndpointError(err).WriteTo(w) return } } diff --git a/cl/beacon/handler/subscription_test.go b/cl/beacon/handler/subscription_test.go new file mode 100644 index 00000000000..42cbef9a640 --- /dev/null +++ b/cl/beacon/handler/subscription_test.go @@ -0,0 +1,128 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package handler + +import ( + "bytes" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/erigontech/erigon/cl/beacon/synced_data" + sync_mock_services "github.com/erigontech/erigon/cl/beacon/synced_data/mock_services" + "github.com/erigontech/erigon/cl/clparams" + "github.com/erigontech/erigon/cl/cltypes" + "github.com/erigontech/erigon/cl/validator/committee_subscription/mock_services" + "github.com/erigontech/erigon/common/log/v3" +) + +func postBeaconCommitteeSubscription(t *testing.T, handler *ApiHandler) int { + t.Helper() + + server := httptest.NewServer(handler.mux) + defer server.Close() + + body, err := json.Marshal([]*cltypes.BeaconCommitteeSubscription{{ + ValidatorIndex: 1, + CommitteeIndex: 0, + CommitteesAtSlot: 1, + Slot: 1, + IsAggregator: false, + }}) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, + server.URL+"/eth/v1/validator/beacon_committee_subscriptions", bytes.NewBuffer(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + + resp, err := server.Client().Do(req) + require.NoError(t, err) + defer resp.Body.Close() + return resp.StatusCode +} + +// The beacon-APIs spec declares 503 CurrentlySyncing for this endpoint, so a node without a head +// state must not report the condition as an internal error. +func TestBeaconCommitteeSubscriptionIsUnavailableWhileSyncing(t *testing.T) { + ctrl := gomock.NewController(t) + _, _, _, _, _, handler, _, _, _, _ := setupTestingHandler(t, clparams.Phase0Version, log.Root(), false) + + committeeSub := mock_services.NewMockCommitteeSubscribe(ctrl) + committeeSub.EXPECT().AddAttestationSubscription(gomock.Any(), gomock.Any()). + Return(synced_data.ErrNotSynced).AnyTimes() + handler.committeeSub = committeeSub + + require.Equal(t, http.StatusServiceUnavailable, postBeaconCommitteeSubscription(t, handler)) +} + +func TestBeaconCommitteeSubscriptionReportsOtherFailuresAsInternalError(t *testing.T) { + ctrl := gomock.NewController(t) + _, _, _, _, _, handler, _, _, _, _ := setupTestingHandler(t, clparams.Phase0Version, log.Root(), false) + + committeeSub := mock_services.NewMockCommitteeSubscribe(ctrl) + committeeSub.EXPECT().AddAttestationSubscription(gomock.Any(), gomock.Any()). + Return(errors.New("subnet computation blew up")).AnyTimes() + handler.committeeSub = committeeSub + + require.Equal(t, http.StatusInternalServerError, postBeaconCommitteeSubscription(t, handler)) +} + +func TestBeaconCommitteeSubscriptionSucceedsWhenSynced(t *testing.T) { + ctrl := gomock.NewController(t) + _, _, _, _, _, handler, _, _, _, _ := setupTestingHandler(t, clparams.Phase0Version, log.Root(), false) + + committeeSub := mock_services.NewMockCommitteeSubscribe(ctrl) + committeeSub.EXPECT().AddAttestationSubscription(gomock.Any(), gomock.Any()). + Return(nil).AnyTimes() + handler.committeeSub = committeeSub + + require.Equal(t, http.StatusOK, postBeaconCommitteeSubscription(t, handler)) +} + +func TestSyncCommitteeSubscriptionIsUnavailableWhileSyncing(t *testing.T) { + _, _, _, _, _, handler, _, syncedDataMgr, _, _ := setupTestingHandler(t, clparams.Phase0Version, log.Root(), false) + syncedDataMgr.(*sync_mock_services.MockSyncedData).EXPECT().ViewHeadState(gomock.Any()). + Return(synced_data.ErrNotSynced).AnyTimes() + + server := httptest.NewServer(handler.mux) + defer server.Close() + + // Far enough ahead that the subscription has not expired, without overflowing the slot clock. + body, err := json.Marshal([]ValidatorSyncCommitteeSubscriptionsRequest{{ + ValidatorIndex: 1, + SyncCommitteeIndicies: []string{"0"}, + UntilEpoch: 1_000_000_000, + }}) + require.NoError(t, err) + + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, + server.URL+"/eth/v1/validator/sync_committee_subscriptions", bytes.NewBuffer(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/json") + + resp, err := server.Client().Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusServiceUnavailable, resp.StatusCode) +} diff --git a/cl/validator/committee_subscription/committee_subscription.go b/cl/validator/committee_subscription/committee_subscription.go index b7ec4dbd88a..5b1a8049fd4 100644 --- a/cl/validator/committee_subscription/committee_subscription.go +++ b/cl/validator/committee_subscription/committee_subscription.go @@ -94,7 +94,7 @@ func (c *CommitteeSubscribeMgmt) AddAttestationSubscription(ctx context.Context, ) if c.syncedData.Syncing() { - return errors.New("head state not available") + return synced_data.ErrNotSynced } log.Trace("Add attestation subscription", "slot", slot, "committeeIndex", cIndex, "isAggregator", p.IsAggregator, "validatorIndex", p.ValidatorIndex) diff --git a/cl/validator/committee_subscription/committee_subscription_test.go b/cl/validator/committee_subscription/committee_subscription_test.go new file mode 100644 index 00000000000..3eb44ff1bb4 --- /dev/null +++ b/cl/validator/committee_subscription/committee_subscription_test.go @@ -0,0 +1,46 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package committee_subscription + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/cl/beacon/synced_data" + "github.com/erigontech/erigon/cl/clparams" + "github.com/erigontech/erigon/cl/cltypes" +) + +// The REST layer turns this into 503 CurrentlySyncing by matching the sentinel, so a bare error +// here would be reported as an internal failure instead. +func TestAddAttestationSubscriptionReportsNotSyncedWhileSyncing(t *testing.T) { + cfg := clparams.MainnetBeaconConfig + c := &CommitteeSubscribeMgmt{ + beaconConfig: &cfg, + syncedData: synced_data.NewSyncedDataManager(&cfg, true), + } + + err := c.AddAttestationSubscription(t.Context(), &cltypes.BeaconCommitteeSubscription{ + ValidatorIndex: 1, + CommitteeIndex: 0, + CommitteesAtSlot: 1, + Slot: 1, + }) + + require.ErrorIs(t, err, synced_data.ErrNotSynced) +}