diff --git a/beacon-chain/rpc/eth/beacon/handlers.go b/beacon-chain/rpc/eth/beacon/handlers.go index fc12a5e6cd..349ada614c 100644 --- a/beacon-chain/rpc/eth/beacon/handlers.go +++ b/beacon-chain/rpc/eth/beacon/handlers.go @@ -1220,7 +1220,7 @@ func (s *Server) GetStateFork(w http.ResponseWriter, r *http.Request) { fork := st.Fork() isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status"+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -1331,7 +1331,7 @@ func (s *Server) GetCommittees(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } @@ -1512,7 +1512,7 @@ func (s *Server) GetFinalityCheckpoints(w http.ResponseWriter, r *http.Request) } isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -1686,7 +1686,7 @@ func (s *Server) GetPendingConsolidations(w http.ResponseWriter, r *http.Request } else { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -1742,7 +1742,7 @@ func (s *Server) GetPendingDeposits(w http.ResponseWriter, r *http.Request) { } else { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -1798,7 +1798,7 @@ func (s *Server) GetPendingPartialWithdrawals(w http.ResponseWriter, r *http.Req } else { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -1851,7 +1851,7 @@ func (s *Server) GetProposerLookahead(w http.ResponseWriter, r *http.Request) { } else { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() diff --git a/beacon-chain/rpc/eth/beacon/handlers_state.go b/beacon-chain/rpc/eth/beacon/handlers_state.go index 93238f1139..fec9fea1d8 100644 --- a/beacon-chain/rpc/eth/beacon/handlers_state.go +++ b/beacon-chain/rpc/eth/beacon/handlers_state.go @@ -56,7 +56,7 @@ func (s *Server) GetStateRoot(w http.ResponseWriter, r *http.Request) { } isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -125,7 +125,7 @@ func (s *Server) GetRandao(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } @@ -227,7 +227,7 @@ func (s *Server) GetSyncCommittees(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } diff --git a/beacon-chain/rpc/eth/beacon/handlers_validator.go b/beacon-chain/rpc/eth/beacon/handlers_validator.go index 6ca3ecc973..25a40b4892 100644 --- a/beacon-chain/rpc/eth/beacon/handlers_validator.go +++ b/beacon-chain/rpc/eth/beacon/handlers_validator.go @@ -44,7 +44,7 @@ func (s *Server) GetValidators(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -222,7 +222,7 @@ func (s *Server) GetValidator(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -258,7 +258,7 @@ func (s *Server) GetValidatorBalances(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() @@ -419,7 +419,7 @@ func (s *Server) getValidatorIdentitiesJSON( ) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateId), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() diff --git a/beacon-chain/rpc/eth/debug/handlers.go b/beacon-chain/rpc/eth/debug/handlers.go index 639cc5af94..7a28a2da98 100644 --- a/beacon-chain/rpc/eth/debug/handlers.go +++ b/beacon-chain/rpc/eth/debug/handlers.go @@ -46,7 +46,7 @@ func (s *Server) getBeaconStateV2(ctx context.Context, w http.ResponseWriter, id isOptimistic, err := helpers.IsOptimistic(ctx, id, s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - httputil.HandleError(w, "Could not check if state is optimistic: "+err.Error(), http.StatusInternalServerError) + helpers.HandleIsOptimisticError(w, err) return } blockRoot, err := st.LatestBlockHeader().HashTreeRoot() diff --git a/beacon-chain/rpc/eth/helpers/BUILD.bazel b/beacon-chain/rpc/eth/helpers/BUILD.bazel index ea727b703e..763472b6c4 100644 --- a/beacon-chain/rpc/eth/helpers/BUILD.bazel +++ b/beacon-chain/rpc/eth/helpers/BUILD.bazel @@ -12,6 +12,7 @@ go_library( deps = [ "//beacon-chain/blockchain:go_default_library", "//beacon-chain/db:go_default_library", + "//beacon-chain/rpc/eth/shared:go_default_library", "//beacon-chain/rpc/lookup:go_default_library", "//beacon-chain/state:go_default_library", "//beacon-chain/state/stategen:go_default_library", @@ -21,6 +22,7 @@ go_library( "//consensus-types/primitives:go_default_library", "//consensus-types/validator:go_default_library", "//encoding/bytesutil:go_default_library", + "//network/httputil:go_default_library", "//time/slots:go_default_library", "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library", "@com_github_pkg_errors//:go_default_library", @@ -40,6 +42,7 @@ go_test( "//beacon-chain/blockchain/testing:go_default_library", "//beacon-chain/db/testing:go_default_library", "//beacon-chain/forkchoice/doubly-linked-tree:go_default_library", + "//beacon-chain/rpc/lookup:go_default_library", "//beacon-chain/rpc/testutil:go_default_library", "//beacon-chain/state:go_default_library", "//beacon-chain/state/state-native:go_default_library", @@ -57,5 +60,6 @@ go_test( "//testing/require:go_default_library", "//testing/util:go_default_library", "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library", + "@com_github_pkg_errors//:go_default_library", ], ) diff --git a/beacon-chain/rpc/eth/helpers/error_handling.go b/beacon-chain/rpc/eth/helpers/error_handling.go index 84222ac6d8..cd6b9ff46f 100644 --- a/beacon-chain/rpc/eth/helpers/error_handling.go +++ b/beacon-chain/rpc/eth/helpers/error_handling.go @@ -2,11 +2,14 @@ package helpers import ( "errors" + "net/http" + "github.com/OffchainLabs/prysm/v6/beacon-chain/rpc/eth/shared" "github.com/OffchainLabs/prysm/v6/beacon-chain/rpc/lookup" "github.com/OffchainLabs/prysm/v6/beacon-chain/state/stategen" "github.com/OffchainLabs/prysm/v6/consensus-types/blocks" "github.com/OffchainLabs/prysm/v6/consensus-types/interfaces" + "github.com/OffchainLabs/prysm/v6/network/httputil" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" ) @@ -28,6 +31,22 @@ func PrepareStateFetchGRPCError(err error) error { return status.Errorf(codes.Internal, "Invalid state ID: %v", err) } +// HandleIsOptimisticError handles errors from IsOptimistic function calls and writes appropriate HTTP responses. +func HandleIsOptimisticError(w http.ResponseWriter, err error) { + var fetchErr *lookup.FetchStateError + if errors.As(err, &fetchErr) { + shared.WriteStateFetchError(w, err) + return + } + + var blockRootsNotFoundErr *lookup.BlockRootsNotFoundError + if errors.As(err, &blockRootsNotFoundErr) { + httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusNotFound) + return + } + httputil.HandleError(w, "Could not check optimistic status: "+err.Error(), http.StatusInternalServerError) +} + // IndexedVerificationFailure represents a collection of verification failures. type IndexedVerificationFailure struct { Failures []*SingleIndexedVerificationFailure `json:"failures"` diff --git a/beacon-chain/rpc/eth/helpers/sync.go b/beacon-chain/rpc/eth/helpers/sync.go index 864d11dfe1..c54bd89c7a 100644 --- a/beacon-chain/rpc/eth/helpers/sync.go +++ b/beacon-chain/rpc/eth/helpers/sync.go @@ -56,7 +56,8 @@ func IsOptimistic( if bytesutil.IsHex(stateId) { id, err := hexutil.Decode(stateIdString) if err != nil { - return false, err + e := lookup.NewStateIdParseError(err) + return false, &e } return isStateRootOptimistic(ctx, id, optimisticModeFetcher, stateFetcher, chainInfo, database) } else if len(stateId) == 32 { @@ -127,7 +128,7 @@ func isStateRootOptimistic( ) (bool, error) { st, err := stateFetcher.State(ctx, stateId) if err != nil { - return true, errors.Wrap(err, "could not fetch state") + return true, lookup.NewFetchStateError(err) } if st.Slot() == chainInfo.HeadSlot() { return optimisticModeFetcher.IsOptimistic(ctx) @@ -137,7 +138,7 @@ func isStateRootOptimistic( return true, errors.Wrapf(err, "could not get block roots for slot %d", st.Slot()) } if !has { - return true, errors.New("no block roots returned from the database") + return true, lookup.NewBlockRootsNotFoundError() } for _, r := range roots { b, err := database.Block(ctx, r) diff --git a/beacon-chain/rpc/eth/helpers/sync_test.go b/beacon-chain/rpc/eth/helpers/sync_test.go index ca007c0c53..d9799b0a05 100644 --- a/beacon-chain/rpc/eth/helpers/sync_test.go +++ b/beacon-chain/rpc/eth/helpers/sync_test.go @@ -1,12 +1,15 @@ package helpers import ( + "net/http" + "net/http/httptest" "strconv" "testing" chainmock "github.com/OffchainLabs/prysm/v6/beacon-chain/blockchain/testing" dbtest "github.com/OffchainLabs/prysm/v6/beacon-chain/db/testing" doublylinkedtree "github.com/OffchainLabs/prysm/v6/beacon-chain/forkchoice/doubly-linked-tree" + "github.com/OffchainLabs/prysm/v6/beacon-chain/rpc/lookup" "github.com/OffchainLabs/prysm/v6/beacon-chain/rpc/testutil" "github.com/OffchainLabs/prysm/v6/beacon-chain/state" state_native "github.com/OffchainLabs/prysm/v6/beacon-chain/state/state-native" @@ -21,6 +24,7 @@ import ( "github.com/OffchainLabs/prysm/v6/testing/require" "github.com/OffchainLabs/prysm/v6/testing/util" "github.com/ethereum/go-ethereum/common/hexutil" + "github.com/pkg/errors" ) func TestIsOptimistic(t *testing.T) { @@ -226,7 +230,67 @@ func TestIsOptimistic(t *testing.T) { require.NoError(t, err) assert.Equal(t, true, o) }) + t.Run("State not found", func(t *testing.T) { + b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlock()) + require.NoError(t, err) + b.SetStateRoot(bytesutil.PadTo([]byte("root"), 32)) + db := dbtest.SetupDB(t) + require.NoError(t, db.SaveBlock(ctx, b)) + chainSt, err := util.NewBeaconState() + require.NoError(t, err) + require.NoError(t, chainSt.SetSlot(fieldparams.SlotsPerEpoch)) + bRoot, err := b.Block().HashTreeRoot() + require.NoError(t, err) + cs := &chainmock.ChainService{State: chainSt, OptimisticRoots: map[[32]byte]bool{bRoot: true}} + mf := &testutil.MockStater{ + CustomError: lookup.NewFetchStateError(nil), + } + _, err = IsOptimistic(ctx, []byte(hexutil.Encode(bytesutil.PadTo([]byte("root"), 32))), cs, mf, cs, db) + var fetchErr *lookup.FetchStateError + require.Equal(t, true, errors.As(err, &fetchErr)) + }) + + t.Run("stateId invalid", func(t *testing.T) { + b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlock()) + require.NoError(t, err) + b.SetStateRoot(bytesutil.PadTo([]byte("root"), 32)) + db := dbtest.SetupDB(t) + require.NoError(t, db.SaveBlock(ctx, b)) + chainSt, err := util.NewBeaconState() + require.NoError(t, err) + require.NoError(t, chainSt.SetSlot(fieldparams.SlotsPerEpoch)) + bRoot, err := b.Block().HashTreeRoot() + require.NoError(t, err) + cs := &chainmock.ChainService{State: chainSt, OptimisticRoots: map[[32]byte]bool{bRoot: true}} + mf := &testutil.MockStater{ + CustomError: lookup.NewFetchStateError(nil), + } + _, err = IsOptimistic(ctx, []byte("0xabc"), cs, mf, cs, db) + var fetchErr *lookup.FetchStateError + require.Equal(t, false, errors.As(err, &fetchErr)) + }) + t.Run("block roots not found", func(t *testing.T) { + b, err := blocks.NewSignedBeaconBlock(util.NewBeaconBlock()) + require.NoError(t, err) + b.SetStateRoot(bytesutil.PadTo([]byte("root"), 32)) + db := dbtest.SetupDB(t) + require.NoError(t, db.SaveBlock(ctx, b)) + chainSt, err := util.NewBeaconState() + require.NoError(t, err) + require.NoError(t, chainSt.SetSlot(fieldparams.SlotsPerEpoch)) + bRoot, err := b.Block().HashTreeRoot() + require.NoError(t, err) + cs := &chainmock.ChainService{State: chainSt, OptimisticRoots: map[[32]byte]bool{bRoot: true}} + st, err := util.NewBeaconState() + require.NoError(t, st.SetSlot(primitives.Slot(fieldparams.SlotsPerEpoch+1))) + require.NoError(t, err) + mf := &testutil.MockStater{BeaconState: st} + _, err = IsOptimistic(ctx, []byte(hexutil.Encode(bytesutil.PadTo([]byte("root"), 32))), cs, mf, cs, db) + var blockRootsNotFoundErr *lookup.BlockRootsNotFoundError + require.Equal(t, true, errors.As(err, &blockRootsNotFoundErr)) + }) }) + t.Run("slot", func(t *testing.T) { t.Run("head is not optimistic", func(t *testing.T) { cs := &chainmock.ChainService{Optimistic: false} @@ -319,6 +383,36 @@ func TestIsOptimistic(t *testing.T) { }) } +func TestHandleIsOptimisticError(t *testing.T) { + t.Run("fetch-state error handled as 404", func(t *testing.T) { + rr := httptest.NewRecorder() + notFoundErr := lookup.StateNotFoundError{} + fetchErr := lookup.NewFetchStateError(¬FoundErr) + HandleIsOptimisticError(rr, fetchErr) + + require.Equal(t, http.StatusNotFound, rr.Code) + require.StringContains(t, notFoundErr.Error(), rr.Body.String()) + }) + t.Run("no block roots error handled as 404", func(t *testing.T) { + rr := httptest.NewRecorder() + blockRootsErr := lookup.NewBlockRootsNotFoundError() + HandleIsOptimisticError(rr, blockRootsErr) + + require.Equal(t, http.StatusNotFound, rr.Code) + require.StringContains(t, blockRootsErr.Error(), rr.Body.String()) + }) + + t.Run("generic error handled as 500", func(t *testing.T) { + rr := httptest.NewRecorder() + genericErr := errors.New("boom") + + HandleIsOptimisticError(rr, genericErr) + + require.Equal(t, http.StatusInternalServerError, rr.Code) + require.StringContains(t, "Could not check optimistic status: boom", rr.Body.String()) + }) +} + // prepareForkchoiceState prepares a beacon state with the given data to mock // insert into forkchoice func prepareForkchoiceState( diff --git a/beacon-chain/rpc/lookup/blocker.go b/beacon-chain/rpc/lookup/blocker.go index fd5cef9bca..1a77d0d93b 100644 --- a/beacon-chain/rpc/lookup/blocker.go +++ b/beacon-chain/rpc/lookup/blocker.go @@ -23,6 +23,20 @@ import ( "github.com/pkg/errors" ) +type BlockRootsNotFoundError struct { + message string +} + +func NewBlockRootsNotFoundError() *BlockRootsNotFoundError { + return &BlockRootsNotFoundError{ + message: "no block roots returned from the database", + } +} + +func (e BlockRootsNotFoundError) Error() string { + return e.message +} + // BlockIdParseError represents an error scenario where a block ID could not be parsed. type BlockIdParseError struct { message string diff --git a/beacon-chain/rpc/lookup/stater.go b/beacon-chain/rpc/lookup/stater.go index 81d12c0b1b..abecc7a8d0 100644 --- a/beacon-chain/rpc/lookup/stater.go +++ b/beacon-chain/rpc/lookup/stater.go @@ -21,6 +21,27 @@ import ( "github.com/pkg/errors" ) +type FetchStateError struct { + message string + cause error +} + +func NewFetchStateError(cause error) *FetchStateError { + return &FetchStateError{ + message: "could not fetch state", + cause: cause, + } +} + +func (e *FetchStateError) Error() string { + if e.cause != nil { + return e.message + ": " + e.cause.Error() + } + return e.message +} + +func (e *FetchStateError) Unwrap() error { return e.cause } + // StateIdParseError represents an error scenario where a state ID could not be parsed. type StateIdParseError struct { message string diff --git a/beacon-chain/rpc/prysm/beacon/validator_count.go b/beacon-chain/rpc/prysm/beacon/validator_count.go index 545ad06bbd..7f0f71a0f3 100644 --- a/beacon-chain/rpc/prysm/beacon/validator_count.go +++ b/beacon-chain/rpc/prysm/beacon/validator_count.go @@ -56,11 +56,7 @@ func (s *Server) GetValidatorCount(w http.ResponseWriter, r *http.Request) { isOptimistic, err := helpers.IsOptimistic(ctx, []byte(stateID), s.OptimisticModeFetcher, s.Stater, s.ChainInfoFetcher, s.BeaconDB) if err != nil { - errJson := &httputil.DefaultJsonError{ - Message: fmt.Sprintf("could not check if slot's block is optimistic: %v", err), - Code: http.StatusInternalServerError, - } - httputil.WriteError(w, errJson) + helpers.HandleIsOptimisticError(w, err) return } diff --git a/beacon-chain/rpc/testutil/mock_stater.go b/beacon-chain/rpc/testutil/mock_stater.go index b0db3ae222..bd0949ba69 100644 --- a/beacon-chain/rpc/testutil/mock_stater.go +++ b/beacon-chain/rpc/testutil/mock_stater.go @@ -15,10 +15,14 @@ type MockStater struct { BeaconStateRoot []byte StatesBySlot map[primitives.Slot]state.BeaconState StatesByRoot map[[32]byte]state.BeaconState + CustomError error } // State -- func (m *MockStater) State(ctx context.Context, id []byte) (state.BeaconState, error) { + if m.CustomError != nil { + return nil, m.CustomError + } if m.StateProviderFunc != nil { return m.StateProviderFunc(ctx, id) } diff --git a/changelog/muzry_update_not_found_status.md b/changelog/muzry_update_not_found_status.md new file mode 100644 index 0000000000..73b414add8 --- /dev/null +++ b/changelog/muzry_update_not_found_status.md @@ -0,0 +1,2 @@ +### Fixed +- Fixed endpoint response to return 404 or 400 after isOptimistic check \ No newline at end of file