diff --git a/errors/CHANGELOG.md b/errors/CHANGELOG.md index f49d3c8e6d..9330f2fe19 100644 --- 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 index 2e5140d728..16f10f36a6 100644 --- 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 index 48a978c8f3..54a33e1842 100644 --- 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 index db0cb1dbb4..cc1fa97247 100644 --- 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 index 5a76cff8e0..936657930e 100644 --- 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 index 7ece5a6cc0..24b0b12320 100644 --- 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()) } }