refactor(x/gov): reduce "if block" indentation (#18918)

cosmos · Dec 30, 2023 · 14037ae · 14037ae
1 parent 4f1d7f9
commit 14037ae
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 30 deletions.
diff --git a/errors/CHANGELOG.md b/errors/CHANGELOG.md
@@ -31,6 +31,10 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
 
+### Improvements
+
+* [#18918](https://github.com/cosmos/cosmos-sdk/pull/18918) Improve `IsOf` by returning earlier when the checked error is nil.
+
 ## [v1.0.0](https://github.com/cosmos/cosmos-sdk/releases/tag/errors%2Fv1.0.0)
 
 ### Features

diff --git a/errors/errors.go b/errors/errors.go
@@ -284,6 +284,9 @@ func WithType(err error, obj interface{}) error {
 // IsOf checks if a received error is caused by one of the target errors.
 // It extends the errors.Is functionality to a list of errors.
 func IsOf(received error, targets ...error) bool {
+	if received == nil {
+		return false
+	}
 	for _, t := range targets {
 		if errors.Is(received, t) {
 			return true

diff --git a/errors/errors_test.go b/errors/errors_test.go
@@ -118,6 +118,8 @@ func (s *errorsTestSuite) TestIsOf() {
 	err := ErrInvalidAddress
 	errW := Wrap(ErrLogic, "more info")
 
+	require.False(IsOf(nil), "nil should always have no causer")
+	require.False(IsOf(nil, err), "nil should always have no causer")
 	require.False(IsOf(errNil), "nil error should always have no causer")
 	require.False(IsOf(errNil, err), "nil error should always have no causer")
 

diff --git a/x/gov/abci.go b/x/gov/abci.go
@@ -267,10 +267,7 @@ func EndBlocker(ctx sdk.Context, keeper *keeper.Keeper) error {
 
 		return false, nil
 	})
-	if err != nil {
-		return err
-	}
-	return nil
+	return err
 }
 
 // executes handle(msg) and recovers from panic.

diff --git a/x/gov/keeper/grpc_query.go b/x/gov/keeper/grpc_query.go
@@ -44,14 +44,13 @@ func (q queryServer) Proposal(ctx context.Context, req *v1.QueryProposalRequest)
 	}
 
 	proposal, err := q.k.Proposals.Get(ctx, req.ProposalId)
-	if err != nil {
-		if errors.IsOf(err, collections.ErrNotFound) {
-			return nil, status.Errorf(codes.NotFound, "proposal %d doesn't exist", req.ProposalId)
-		}
-		return nil, status.Error(codes.Internal, err.Error())
+	if err == nil {
+		return &v1.QueryProposalResponse{Proposal: &proposal}, nil
 	}
-
-	return &v1.QueryProposalResponse{Proposal: &proposal}, nil
+	if errors.IsOf(err, collections.ErrNotFound) {
+		return nil, status.Errorf(codes.NotFound, "proposal %d doesn't exist", req.ProposalId)
+	}
+	return nil, status.Error(codes.Internal, err.Error())
 }
 
 // Proposals implements the Query/Proposals gRPC method
@@ -123,15 +122,14 @@ func (q queryServer) Vote(ctx context.Context, req *v1.QueryVoteRequest) (*v1.Qu
 		return nil, err
 	}
 	vote, err := q.k.Votes.Get(ctx, collections.Join(req.ProposalId, sdk.AccAddress(voter)))
-	if err != nil {
-		if errors.IsOf(err, collections.ErrNotFound) {
-			return nil, status.Errorf(codes.InvalidArgument,
-				"voter: %v not found for proposal: %v", req.Voter, req.ProposalId)
-		}
-		return nil, status.Error(codes.Internal, err.Error())
+	if err == nil {
+		return &v1.QueryVoteResponse{Vote: &vote}, nil
 	}
-
-	return &v1.QueryVoteResponse{Vote: &vote}, nil
+	if errors.IsOf(err, collections.ErrNotFound) {
+		return nil, status.Errorf(codes.InvalidArgument,
+			"voter: %v not found for proposal: %v", req.Voter, req.ProposalId)
+	}
+	return nil, status.Error(codes.Internal, err.Error())
 }
 
 // Votes returns single proposal's votes

diff --git a/x/gov/keeper/proposal.go b/x/gov/keeper/proposal.go
@@ -29,10 +29,8 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met
 	if proposalType == v1.ProposalType_PROPOSAL_TYPE_OPTIMISTIC {
 		proposerStr, _ := keeper.authKeeper.AddressCodec().BytesToString(proposer)
 
-		if len(params.OptimisticAuthorizedAddresses) > 0 {
-			if !slices.Contains(params.OptimisticAuthorizedAddresses, proposerStr) {
-				return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposer, "proposer is not authorized to submit optimistic proposal")
-			}
+		if len(params.OptimisticAuthorizedAddresses) > 0 && !slices.Contains(params.OptimisticAuthorizedAddresses, proposerStr) {
+			return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposer, "proposer is not authorized to submit optimistic proposal")
 		}
 	}
 
@@ -74,14 +72,16 @@ func (keeper Keeper) SubmitProposal(ctx context.Context, messages []sdk.Msg, met
 		// For other Msgs, we do not verify the proposal messages any further.
 		// They may fail upon execution.
 		// ref: https://github.com/cosmos/cosmos-sdk/pull/10868#discussion_r784872842
-		if msg, ok := msg.(*v1.MsgExecLegacyContent); ok {
-			cacheCtx, _ := sdkCtx.CacheContext()
-			if _, err := handler(cacheCtx, msg); err != nil {
-				if errors.Is(types.ErrNoProposalHandlerExists, err) {
-					return v1.Proposal{}, err
-				}
-				return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalContent, err.Error())
+		msg, ok := msg.(*v1.MsgExecLegacyContent)
+		if !ok {
+			continue
+		}
+		cacheCtx, _ := sdkCtx.CacheContext()
+		if _, err := handler(cacheCtx, msg); err != nil {
+			if errors.Is(types.ErrNoProposalHandlerExists, err) {
+				return v1.Proposal{}, err
 			}
+			return v1.Proposal{}, errorsmod.Wrap(types.ErrInvalidProposalContent, err.Error())
 		}
 
 	}