Return correct status codes from beacon endpoints

beacon-chain/rpc/beaconv1/BUILD.bazel

@@ -28,6 +28,7 @@ go_library(
         "//beacon-chain/p2p:go_default_library",
         "//beacon-chain/rpc/statefetcher:go_default_library",
         "//beacon-chain/state/interface:go_default_library",
+        "//beacon-chain/state/stateV0:go_default_library",
         "//beacon-chain/state/stategen:go_default_library",
         "//proto/migration:go_default_library",
         "//shared/bytesutil:go_default_library",
@@ -71,8 +72,8 @@ go_test(
         "//beacon-chain/operations/voluntaryexits:go_default_library",
         "//beacon-chain/p2p/testing:go_default_library",
         "//beacon-chain/rpc/statefetcher:go_default_library",
+        "//beacon-chain/rpc/testutil:go_default_library",
         "//beacon-chain/state/interface:go_default_library",
-        "//beacon-chain/state/stategen:go_default_library",
         "//proto/beacon/p2p/v1:go_default_library",
         "//proto/migration:go_default_library",
         "//shared/bls:go_default_library",
@@ -82,7 +83,6 @@ go_test(
         "//shared/testutil:go_default_library",
         "//shared/testutil/assert:go_default_library",
         "//shared/testutil/require:go_default_library",
-        "@com_github_ethereum_go_ethereum//common/hexutil:go_default_library",
         "@com_github_grpc_ecosystem_grpc_gateway_v2//runtime:go_default_library",
         "@com_github_prysmaticlabs_eth2_types//:go_default_library",
         "@com_github_prysmaticlabs_ethereumapis//eth/v1:go_default_library",

beacon-chain/rpc/beaconv1/blocks.go

@@ -20,12 +20,32 @@ import (
 	"google.golang.org/protobuf/types/known/emptypb"
 )
 
+// blockIdParseError represents an error scenario where a block ID could not be parsed.
+type blockIdParseError struct {
+	message string
+}
+
+// newBlockIdParseError creates a new error instance.
+func newBlockIdParseError(reason error) blockIdParseError {
+	return blockIdParseError{
+		message: fmt.Sprintf("could not parse block ID: %v", reason),
+	}
+}
+
+// Error returns the underlying error message.
+func (e *blockIdParseError) Error() string {
+	return e.message
+}
+
 // GetBlockHeader retrieves block header for given block id.
 func (bs *Server) GetBlockHeader(ctx context.Context, req *ethpb.BlockRequest) (*ethpb.BlockHeaderResponse, error) {
 	ctx, span := trace.StartSpan(ctx, "beaconv1.GetBlockHeader")
 	defer span.End()
 
 	rBlk, err := bs.blockFromBlockID(ctx, req.BlockId)
+	if invalidBlockIdErr, ok := err.(*blockIdParseError); ok {
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid block ID: %v", invalidBlockIdErr)
+	}
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not get block from block ID: %v", err)
 	}
@@ -141,13 +161,13 @@ func (bs *Server) SubmitBlock(ctx context.Context, req *ethpb.BeaconBlockContain
 	blk := req.Message
 	rBlock, err := migration.V1ToV1Alpha1Block(&ethpb.SignedBeaconBlock{Block: blk, Signature: req.Signature})
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not convert block to v1")
+		return nil, status.Errorf(codes.InvalidArgument, "Could not convert block to v1 block")
 	}
 	v1alpha1Block := interfaces.WrappedPhase0SignedBeaconBlock(rBlock)
 
 	root, err := blk.HashTreeRoot()
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not tree hash block: %v", err)
+		return nil, status.Errorf(codes.InvalidArgument, "Could not tree hash block: %v", err)
 	}
 
 	// Do not block proposal critical path with debug logging or block feed updates.
@@ -172,12 +192,15 @@ func (bs *Server) SubmitBlock(ctx context.Context, req *ethpb.BeaconBlockContain
 	return &emptypb.Empty{}, nil
 }
 
-// GetBlock retrieves block details for given block id.
+// GetBlock retrieves block details for given block ID.
 func (bs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb.BlockResponse, error) {
 	ctx, span := trace.StartSpan(ctx, "beaconv1.GetBlock")
 	defer span.End()
 
 	rBlk, err := bs.blockFromBlockID(ctx, req.BlockId)
+	if invalidBlockIdErr, ok := err.(*blockIdParseError); ok {
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid block ID: %v", invalidBlockIdErr)
+	}
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not get block from block ID: %v", err)
 	}
@@ -191,7 +214,7 @@ func (bs *Server) GetBlock(ctx context.Context, req *ethpb.BlockRequest) (*ethpb
 
 	v1Block, err := migration.V1Alpha1ToV1Block(blk)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not convert block to v1")
+		return nil, status.Errorf(codes.Internal, "Could not convert block to v1 block")
 	}
 
 	return &ethpb.BlockResponse{
@@ -248,7 +271,7 @@ func (bs *Server) GetBlockRoot(ctx context.Context, req *ethpb.BlockRequest) (*e
 		} else {
 			slot, err := strconv.ParseUint(string(req.BlockId), 10, 64)
 			if err != nil {
-				return nil, status.Errorf(codes.Internal, "could not decode block id: %v", err)
+				return nil, status.Errorf(codes.InvalidArgument, "Could not parse block ID: %v", err)
 			}
 			hasRoots, roots, err := bs.BeaconDB.BlockRootsBySlot(ctx, types.Slot(slot))
 			if err != nil {
@@ -288,6 +311,9 @@ func (bs *Server) ListBlockAttestations(ctx context.Context, req *ethpb.BlockReq
 	defer span.End()
 
 	rBlk, err := bs.blockFromBlockID(ctx, req.BlockId)
+	if invalidBlockIdErr, ok := err.(*blockIdParseError); ok {
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid block ID: %v", invalidBlockIdErr)
+	}
 	if err != nil {
 		return nil, status.Errorf(codes.Internal, "Could not get block from block ID: %v", err)
 	}
@@ -302,7 +328,7 @@ func (bs *Server) ListBlockAttestations(ctx context.Context, req *ethpb.BlockReq
 
 	v1Block, err := migration.V1Alpha1ToV1Block(blk)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Could not convert block to v1")
+		return nil, status.Errorf(codes.Internal, "Could not convert block to v1 block")
 	}
 	return &ethpb.BlockAttestationsResponse{
 		Data: v1Block.Block.Body.Attestations,
@@ -339,7 +365,8 @@ func (bs *Server) blockFromBlockID(ctx context.Context, blockId []byte) (interfa
 		} else {
 			slot, err := strconv.ParseUint(string(blockId), 10, 64)
 			if err != nil {
-				return nil, errors.Wrap(err, "could not decode block id")
+				e := newBlockIdParseError(err)
+				return nil, &e
 			}
 			_, blks, err := bs.BeaconDB.BlocksBySlot(ctx, types.Slot(slot))
 			if err != nil {

beacon-chain/rpc/beaconv1/pool.go

@@ -118,7 +118,7 @@ func (bs *Server) SubmitAttestations(ctx context.Context, req *ethpb.SubmitAttes
 		if err != nil {
 			return nil, status.Errorf(codes.Internal, "Could not prepare attestation failure information: %v", err)
 		}
-		return nil, status.Errorf(codes.Internal, "One or more attestations failed validation")
+		return nil, status.Errorf(codes.InvalidArgument, "One or more attestations failed validation")
 	}
 
 	return &emptypb.Empty{}, nil
@@ -160,7 +160,7 @@ func (bs *Server) SubmitAttesterSlashing(ctx context.Context, req *ethpb.Atteste
 	alphaSlashing := migration.V1AttSlashingToV1Alpha1(req)
 	err = blocks.VerifyAttesterSlashing(ctx, headState, alphaSlashing)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Invalid attester slashing: %v", err)
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid attester slashing: %v", err)
 	}
 
 	err = bs.SlashingsPool.InsertAttesterSlashing(ctx, headState, alphaSlashing)
@@ -212,7 +212,7 @@ func (bs *Server) SubmitProposerSlashing(ctx context.Context, req *ethpb.Propose
 	alphaSlashing := migration.V1ProposerSlashingToV1Alpha1(req)
 	err = blocks.VerifyProposerSlashing(headState, alphaSlashing)
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Invalid proposer slashing: %v", err)
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid proposer slashing: %v", err)
 	}
 
 	err = bs.SlashingsPool.InsertProposerSlashing(ctx, headState, alphaSlashing)
@@ -269,7 +269,7 @@ func (bs *Server) SubmitVoluntaryExit(ctx context.Context, req *ethpb.SignedVolu
 	alphaExit := migration.V1ExitToV1Alpha1(req)
 	err = blocks.VerifyExitAndSignature(validator, headState.Slot(), headState.Fork(), alphaExit, headState.GenesisValidatorRoot())
 	if err != nil {
-		return nil, status.Errorf(codes.Internal, "Invalid voluntary exit: %v", err)
+		return nil, status.Errorf(codes.InvalidArgument, "Invalid voluntary exit: %v", err)
 	}
 
 	bs.VoluntaryExitsPool.InsertVoluntaryExit(ctx, headState, alphaExit)

beacon-chain/rpc/beaconv1/server.go

@@ -29,5 +29,5 @@ type Server struct {
 	SlashingsPool      slashings.PoolManager
 	VoluntaryExitsPool voluntaryexits.PoolManager
 	StateGenService    stategen.StateManager
-	StateFetcher       statefetcher.StateProvider
+	StateFetcher       statefetcher.Fetcher
 }

beacon-chain/rpc/beaconv1/state.go

@@ -3,17 +3,11 @@ package beaconv1
 import (
 	"bytes"
 	"context"
-	"fmt"
-	"strconv"
-	"strings"
 
-	"github.com/pkg/errors"
-	types "github.com/prysmaticlabs/eth2-types"
 	ethpb "github.com/prysmaticlabs/ethereumapis/eth/v1"
 	eth "github.com/prysmaticlabs/ethereumapis/eth/v1alpha1"
-	"github.com/prysmaticlabs/prysm/beacon-chain/core/helpers"
+	"github.com/prysmaticlabs/prysm/beacon-chain/rpc/statefetcher"
 	iface "github.com/prysmaticlabs/prysm/beacon-chain/state/interface"
-	"github.com/prysmaticlabs/prysm/shared/bytesutil"
 	"github.com/prysmaticlabs/prysm/shared/params"
 	"go.opencensus.io/trace"
 	"google.golang.org/grpc/codes"
@@ -59,8 +53,13 @@ func (bs *Server) GetStateRoot(ctx context.Context, req *ethpb.StateRequest) (*e
 		err  error
 	)
 
-	root, err = bs.stateRoot(ctx, req.StateId)
+	root, err = bs.StateFetcher.StateRoot(ctx, req.StateId)
 	if err != nil {
+		if rootNotFoundErr, ok := err.(*statefetcher.StateRootNotFoundError); ok {
+			return nil, status.Errorf(codes.NotFound, "State root not found: %v", rootNotFoundErr)
+		} else if parseErr, ok := err.(*statefetcher.StateIdParseError); ok {
+			return nil, status.Errorf(codes.InvalidArgument, "Invalid state ID: %v", parseErr)
+		}
 		return nil, status.Errorf(codes.Internal, "Could not get state root: %v", err)
 	}
 
@@ -83,6 +82,11 @@ func (bs *Server) GetStateFork(ctx context.Context, req *ethpb.StateRequest) (*e
 
 	state, err = bs.StateFetcher.State(ctx, req.StateId)
 	if err != nil {
+		if stateNotFoundErr, ok := err.(*statefetcher.StateNotFoundError); ok {
+			return nil, status.Errorf(codes.NotFound, "State not found: %v", stateNotFoundErr)
+		} else if parseErr, ok := err.(*statefetcher.StateIdParseError); ok {
+			return nil, status.Errorf(codes.InvalidArgument, "Invalid state ID: %v", parseErr)
+		}
 		return nil, status.Errorf(codes.Internal, "Could not get state: %v", err)
 	}
 
@@ -109,6 +113,11 @@ func (bs *Server) GetFinalityCheckpoints(ctx context.Context, req *ethpb.StateRe
 
 	state, err = bs.StateFetcher.State(ctx, req.StateId)
 	if err != nil {
+		if stateNotFoundErr, ok := err.(*statefetcher.StateNotFoundError); ok {
+			return nil, status.Errorf(codes.NotFound, "State not found: %v", stateNotFoundErr)
+		} else if parseErr, ok := err.(*statefetcher.StateIdParseError); ok {
+			return nil, status.Errorf(codes.InvalidArgument, "Invalid state ID: %v", parseErr)
+		}
 		return nil, status.Errorf(codes.Internal, "Could not get state: %v", err)
 	}
 
@@ -121,126 +130,6 @@ func (bs *Server) GetFinalityCheckpoints(ctx context.Context, req *ethpb.StateRe
 	}, nil
 }
 
-func (bs *Server) stateRoot(ctx context.Context, stateId []byte) ([]byte, error) {
-	var (
-		root []byte
-		err  error
-	)
-
-	stateIdString := strings.ToLower(string(stateId))
-	switch stateIdString {
-	case "head":
-		root, err = bs.headStateRoot(ctx)
-	case "genesis":
-		root, err = bs.genesisStateRoot(ctx)
-	case "finalized":
-		root, err = bs.finalizedStateRoot(ctx)
-	case "justified":
-		root, err = bs.justifiedStateRoot(ctx)
-	default:
-		if len(stateId) == 32 {
-			root, err = bs.stateRootByHex(ctx, stateId)
-		} else {
-			slotNumber, parseErr := strconv.ParseUint(stateIdString, 10, 64)
-			if parseErr != nil {
-				// ID format does not match any valid options.
-				return nil, errors.New("invalid state ID: " + stateIdString)
-			}
-			root, err = bs.stateRootBySlot(ctx, types.Slot(slotNumber))
-		}
-	}
-
-	return root, err
-}
-
-func (bs *Server) headStateRoot(ctx context.Context) ([]byte, error) {
-	b, err := bs.ChainInfoFetcher.HeadBlock(ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get head block")
-	}
-	if err := helpers.VerifyNilBeaconBlock(b); err != nil {
-		return nil, err
-	}
-	return b.Block().StateRoot(), nil
-}
-
-func (bs *Server) genesisStateRoot(ctx context.Context) ([]byte, error) {
-	b, err := bs.BeaconDB.GenesisBlock(ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get genesis block")
-	}
-	if err := helpers.VerifyNilBeaconBlock(b); err != nil {
-		return nil, err
-	}
-	return b.Block().StateRoot(), nil
-}
-
-func (bs *Server) finalizedStateRoot(ctx context.Context) ([]byte, error) {
-	cp, err := bs.BeaconDB.FinalizedCheckpoint(ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get finalized checkpoint")
-	}
-	b, err := bs.BeaconDB.Block(ctx, bytesutil.ToBytes32(cp.Root))
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get finalized block")
-	}
-	if err := helpers.VerifyNilBeaconBlock(b); err != nil {
-		return nil, err
-	}
-	return b.Block().StateRoot(), nil
-}
-
-func (bs *Server) justifiedStateRoot(ctx context.Context) ([]byte, error) {
-	cp, err := bs.BeaconDB.JustifiedCheckpoint(ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get justified checkpoint")
-	}
-	b, err := bs.BeaconDB.Block(ctx, bytesutil.ToBytes32(cp.Root))
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get justified block")
-	}
-	if err := helpers.VerifyNilBeaconBlock(b); err != nil {
-		return nil, err
-	}
-	return b.Block().StateRoot(), nil
-}
-
-func (bs *Server) stateRootByHex(ctx context.Context, stateId []byte) ([]byte, error) {
-	var stateRoot [32]byte
-	copy(stateRoot[:], stateId)
-	headState, err := bs.ChainInfoFetcher.HeadState(ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get head state")
-	}
-	for _, root := range headState.StateRoots() {
-		if bytes.Equal(root, stateRoot[:]) {
-			return stateRoot[:], nil
-		}
-	}
-	return nil, fmt.Errorf("state not found in the last %d state roots", len(headState.StateRoots()))
-}
-
-func (bs *Server) stateRootBySlot(ctx context.Context, slot types.Slot) ([]byte, error) {
-	currentSlot := bs.GenesisTimeFetcher.CurrentSlot()
-	if slot > currentSlot {
-		return nil, errors.New("slot cannot be in the future")
-	}
-	found, blks, err := bs.BeaconDB.BlocksBySlot(ctx, slot)
-	if err != nil {
-		return nil, errors.Wrap(err, "could not get blocks")
-	}
-	if !found {
-		return nil, errors.New("no block exists")
-	}
-	if len(blks) != 1 {
-		return nil, errors.New("multiple blocks exist in same slot")
-	}
-	if blks[0] == nil || blks[0].IsNil() || blks[0].Block().IsNil() {
-		return nil, errors.New("nil block")
-	}
-	return blks[0].Block().StateRoot(), nil
-}
-
 func checkpoint(sourceCheckpoint *eth.Checkpoint) *ethpb.Checkpoint {
 	if sourceCheckpoint != nil {
 		return &ethpb.Checkpoint{