From 2af642e6b6c432a1f6a08994c602d0f6df0c9960 Mon Sep 17 00:00:00 2001
From: Julien Robert <julien@rbrt.fr>
Date: Tue, 19 Apr 2022 21:08:35 +0200
Subject: [PATCH] refactor: register x/authz module specific errors (#11670)

---
 x/authz/authorization_grant.go   |  8 ++++----
 x/authz/client/testutil/tx.go    | 13 +++++++------
 x/authz/errors.go                | 15 +++++++++++++++
 x/authz/keeper/grpc_query.go     |  2 +-
 x/authz/keeper/keeper.go         | 19 +++++++++++--------
 x/authz/keeper/msg_server.go     |  2 +-
 x/authz/msgs.go                  | 15 ++++++++-------
 x/authz/simulation/operations.go |  6 +++---
 x/staking/types/authz.go         |  8 ++++----
 9 files changed, 54 insertions(+), 34 deletions(-)

diff --git a/x/authz/authorization_grant.go b/x/authz/authorization_grant.go
index bd94977ab67b..088ff40556f2 100644
--- a/x/authz/authorization_grant.go
+++ b/x/authz/authorization_grant.go
@@ -14,11 +14,11 @@ import (
 // which is passed into the `blockTime` arg.
 func NewGrant(blockTime time.Time, a Authorization, expiration *time.Time) (Grant, error) {
 	if expiration != nil && !expiration.After(blockTime) {
-		return Grant{}, sdkerrors.ErrInvalidRequest.Wrapf("expiration must be after the current block time (%v), got %v", blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
+		return Grant{}, sdkerrors.Wrapf(ErrInvalidExpirationTime, "expiration must be after the current block time (%v), got %v", blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
 	}
 	msg, ok := a.(proto.Message)
 	if !ok {
-		return Grant{}, sdkerrors.Wrapf(sdkerrors.ErrPackAny, "cannot proto marshal %T", a)
+		return Grant{}, sdkerrors.ErrPackAny.Wrapf("cannot proto marshal %T", a)
 	}
 	any, err := cdctypes.NewAnyWithValue(msg)
 	if err != nil {
@@ -43,7 +43,7 @@ func (g Grant) UnpackInterfaces(unpacker cdctypes.AnyUnpacker) error {
 // GetAuthorization returns the cached value from the Grant.Authorization if present.
 func (g Grant) GetAuthorization() (Authorization, error) {
 	if g.Authorization == nil {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidType, "authorization is nil")
+		return nil, sdkerrors.ErrInvalidType.Wrap("authorization is nil")
 	}
 	a, ok := g.Authorization.GetCachedValue().(Authorization)
 	if !ok {
@@ -56,7 +56,7 @@ func (g Grant) ValidateBasic() error {
 	av := g.Authorization.GetCachedValue()
 	a, ok := av.(Authorization)
 	if !ok {
-		return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (Authorization)(nil), av)
+		return sdkerrors.ErrInvalidType.Wrapf("expected %T, got %T", (Authorization)(nil), av)
 	}
 	return a.ValidateBasic()
 }
diff --git a/x/authz/client/testutil/tx.go b/x/authz/client/testutil/tx.go
index f7db4fe644be..da1cf903ed60 100644
--- a/x/authz/client/testutil/tx.go
+++ b/x/authz/client/testutil/tx.go
@@ -14,6 +14,7 @@ import (
 	clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
 	"github.com/cosmos/cosmos-sdk/testutil/network"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/authz"
 	"github.com/cosmos/cosmos-sdk/x/authz/client/cli"
 	banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
 	bank "github.com/cosmos/cosmos-sdk/x/bank/types"
@@ -563,7 +564,7 @@ func (s *IntegrationTestSuite) TestExecAuthorizationWithExpiration() {
 		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 	})
 	s.Require().NoError(err)
-	s.Require().Contains(res.String(), "authorization not found")
+	s.Require().Contains(res.String(), authz.ErrNoAuthorizationFound.Error())
 }
 
 func (s *IntegrationTestSuite) TestNewExecGenericAuthorized() {
@@ -745,7 +746,7 @@ func (s *IntegrationTestSuite) TestNewExecGrantAuthorized() {
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 			},
-			4,
+			authz.ErrNoAuthorizationFound.ABCICode(),
 			false,
 			"",
 		},
@@ -844,9 +845,9 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 			},
-			4,
+			authz.ErrNoAuthorizationFound.ABCICode(),
 			false,
-			"authorization not found",
+			authz.ErrNoAuthorizationFound.Error(),
 		},
 	}
 
@@ -1065,9 +1066,9 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
 				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
 				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 			},
-			4,
+			authz.ErrNoAuthorizationFound.ABCICode(),
 			false,
-			"authorization not found",
+			authz.ErrNoAuthorizationFound.Error(),
 		},
 	}
 
diff --git a/x/authz/errors.go b/x/authz/errors.go
index 02251c8d6f7e..8d104d0c920e 100644
--- a/x/authz/errors.go
+++ b/x/authz/errors.go
@@ -6,5 +6,20 @@ import (
 
 // x/authz module sentinel errors
 var (
+	//ErrNoAuthorizationFound error if there is no authorization found given a grant key
+	ErrNoAuthorizationFound = sdkerrors.Register(ModuleName, 2, "authorization not found")
+	// ErrInvalidExpirationTime error if the set expiration time is in the past
 	ErrInvalidExpirationTime = sdkerrors.Register(ModuleName, 3, "expiration time of authorization should be more than current time")
+	// ErrUnknownAuthorizationType error for unknown authorization type
+	ErrUnknownAuthorizationType = sdkerrors.Register(ModuleName, 4, "unknown authorization type")
+	// ErrNoGrantKeyFound error if the requested grant key does not exist
+	ErrNoGrantKeyFound = sdkerrors.Register(ModuleName, 5, "grant key not found")
+	// ErrAuthorizationExpired error if the authorization has expired
+	ErrAuthorizationExpired = sdkerrors.Register(ModuleName, 6, "authorization expired")
+	// ErrGranteeIsGranter error if the grantee and the granter are the same
+	ErrGranteeIsGranter = sdkerrors.Register(ModuleName, 7, "grantee and granter should be different")
+	// ErrAuthorizationNumOfSigners error if an authorization message does not have only one signer
+	ErrAuthorizationNumOfSigners = sdkerrors.Register(ModuleName, 9, "authorization can be given to msg with only one signer")
+	// ErrNegativeMaxTokens error if the max tokens is negative
+	ErrNegativeMaxTokens = sdkerrors.Register(ModuleName, 12, "max tokens should be positive")
 )
diff --git a/x/authz/keeper/grpc_query.go b/x/authz/keeper/grpc_query.go
index 6f1947b1bbb7..11bf0715efe7 100644
--- a/x/authz/keeper/grpc_query.go
+++ b/x/authz/keeper/grpc_query.go
@@ -39,7 +39,7 @@ func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz
 	if req.MsgTypeUrl != "" {
 		grant, found := k.getGrant(ctx, grantStoreKey(grantee, granter, req.MsgTypeUrl))
 		if !found {
-			return nil, sdkerrors.ErrNotFound.Wrapf("authorization not found for %s type", req.MsgTypeUrl)
+			return nil, sdkerrors.Wrapf(authz.ErrNoAuthorizationFound, "authorization not found for %s type", req.MsgTypeUrl)
 		}
 
 		authorization, err := grant.GetAuthorization()
diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go
index a9917cf12119..a87375b044f1 100644
--- a/x/authz/keeper/keeper.go
+++ b/x/authz/keeper/keeper.go
@@ -60,12 +60,12 @@ func (k Keeper) update(ctx sdk.Context, grantee sdk.AccAddress, granter sdk.AccA
 	skey := grantStoreKey(grantee, granter, updated.MsgTypeURL())
 	grant, found := k.getGrant(ctx, skey)
 	if !found {
-		return sdkerrors.ErrNotFound.Wrap("authorization not found")
+		return authz.ErrNoAuthorizationFound
 	}
 
 	msg, ok := updated.(proto.Message)
 	if !ok {
-		sdkerrors.ErrPackAny.Wrapf("cannot proto marshal %T", updated)
+		return sdkerrors.ErrPackAny.Wrapf("cannot proto marshal %T", updated)
 	}
 
 	any, err := codectypes.NewAnyWithValue(msg)
@@ -76,6 +76,7 @@ func (k Keeper) update(ctx sdk.Context, grantee sdk.AccAddress, granter sdk.AccA
 	grant.Authorization = any
 	store := ctx.KVStore(k.storeKey)
 	store.Set(skey, k.cdc.MustMarshal(&grant))
+
 	return nil
 }
 
@@ -87,19 +88,21 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []
 	for i, msg := range msgs {
 		signers := msg.GetSigners()
 		if len(signers) != 1 {
-			return nil, sdkerrors.ErrInvalidRequest.Wrap("authorization can be given to msg with only one signer")
+			return nil, authz.ErrAuthorizationNumOfSigners
 		}
 		granter := signers[0]
 
 		// if granter != grantee then check authorization.Accept, otherwise we implicitly accept.
 		if !granter.Equals(grantee) {
-			grant, found := k.getGrant(ctx, grantStoreKey(grantee, granter, sdk.MsgTypeURL(msg)))
+			skey := grantStoreKey(grantee, granter, sdk.MsgTypeURL(msg))
+
+			grant, found := k.getGrant(ctx, skey)
 			if !found {
-				return nil, sdkerrors.ErrUnauthorized.Wrap("authorization not found")
+				return nil, sdkerrors.Wrapf(authz.ErrNoAuthorizationFound, "failed to update grant with key %s", string(skey))
 			}
 
 			if grant.Expiration != nil && grant.Expiration.Before(now) {
-				return nil, sdkerrors.ErrUnauthorized.Wrap("authorization expired")
+				return nil, authz.ErrAuthorizationExpired
 			}
 
 			authorization, err := grant.GetAuthorization()
@@ -193,7 +196,7 @@ func (k Keeper) DeleteGrant(ctx sdk.Context, grantee sdk.AccAddress, granter sdk
 	skey := grantStoreKey(grantee, granter, msgType)
 	grant, found := k.getGrant(ctx, skey)
 	if !found {
-		return sdkerrors.ErrNotFound.Wrap("authorization not found")
+		return sdkerrors.Wrapf(authz.ErrNoAuthorizationFound, "failed to delete grant with key %s", string(skey))
 	}
 
 	store.Delete(skey)
@@ -355,7 +358,7 @@ func (keeper Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, gran
 	key := GrantQueueKey(expiration, granter, grantee)
 	bz := store.Get(key)
 	if bz == nil {
-		return sdkerrors.ErrLogic.Wrap("can't remove grant from the expire queue, grant key not found")
+		return sdkerrors.Wrap(authz.ErrNoGrantKeyFound, "can't remove grant from the expire queue, grant key not found")
 	}
 
 	var queueItem authz.GrantQueueItem
diff --git a/x/authz/keeper/msg_server.go b/x/authz/keeper/msg_server.go
index ac81b60b6469..24047e921c8a 100644
--- a/x/authz/keeper/msg_server.go
+++ b/x/authz/keeper/msg_server.go
@@ -35,7 +35,7 @@ func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGra
 	}
 	t := authorization.MsgTypeURL()
 	if k.router.HandlerByTypeURL(t) == nil {
-		return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "%s doesn't exist.", t)
+		return nil, sdkerrors.ErrInvalidType.Wrapf("%s doesn't exist.", t)
 	}
 
 	err = k.SaveGrant(ctx, grantee, granter, authorization, msg.Grant.Expiration)
diff --git a/x/authz/msgs.go b/x/authz/msgs.go
index 877932b75e11..3aa145bd7d12 100644
--- a/x/authz/msgs.go
+++ b/x/authz/msgs.go
@@ -1,9 +1,10 @@
 package authz
 
 import (
-	"github.com/cosmos/cosmos-sdk/codec/legacy"
 	"time"
 
+	"github.com/cosmos/cosmos-sdk/codec/legacy"
+
 	"github.com/gogo/protobuf/proto"
 
 	cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
@@ -59,7 +60,7 @@ func (msg MsgGrant) ValidateBasic() error {
 	}
 
 	if granter.Equals(grantee) {
-		return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "granter and grantee cannot be same")
+		return ErrGranteeIsGranter
 	}
 	return msg.Grant.ValidateBasic()
 }
@@ -88,7 +89,7 @@ func (msg *MsgGrant) GetAuthorization() (Authorization, error) {
 func (msg *MsgGrant) SetAuthorization(a Authorization) error {
 	m, ok := a.(proto.Message)
 	if !ok {
-		return sdkerrors.Wrapf(sdkerrors.ErrPackAny, "can't proto marshal %T", m)
+		return sdkerrors.ErrPackAny.Wrapf("can't proto marshal %T", m)
 	}
 	any, err := cdctypes.NewAnyWithValue(m)
 	if err != nil {
@@ -144,11 +145,11 @@ func (msg MsgRevoke) ValidateBasic() error {
 	}
 
 	if granter.Equals(grantee) {
-		return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "granter and grantee cannot be same")
+		return ErrGranteeIsGranter
 	}
 
 	if msg.MsgTypeUrl == "" {
-		return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "missing method name")
+		return sdkerrors.ErrInvalidRequest.Wrap("missing method name")
 	}
 
 	return nil
@@ -194,7 +195,7 @@ func (msg MsgExec) GetMessages() ([]sdk.Msg, error) {
 	for i, msgAny := range msg.Msgs {
 		msg, ok := msgAny.GetCachedValue().(sdk.Msg)
 		if !ok {
-			return nil, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "messages contains %T which is not a sdk.MsgRequest", msgAny)
+			return nil, sdkerrors.ErrInvalidRequest.Wrapf("messages contains %T which is not a sdk.MsgRequest", msgAny)
 		}
 		msgs[i] = msg
 	}
@@ -215,7 +216,7 @@ func (msg MsgExec) ValidateBasic() error {
 	}
 
 	if len(msg.Msgs) == 0 {
-		return sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "messages cannot be empty")
+		return sdkerrors.ErrInvalidRequest.Wrapf("messages cannot be empty")
 	}
 
 	return nil
diff --git a/x/authz/simulation/operations.go b/x/authz/simulation/operations.go
index 1879522b0ce1..ef5db52fdcc5 100644
--- a/x/authz/simulation/operations.go
+++ b/x/authz/simulation/operations.go
@@ -170,7 +170,7 @@ func SimulateMsgRevoke(ak authz.AccountKeeper, bk authz.BankKeeper, k keeper.Kee
 
 		granterAcc, ok := simtypes.FindAccount(accs, granterAddr)
 		if !ok {
-			return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "account not found"), nil, sdkerrors.Wrapf(sdkerrors.ErrNotFound, "account not found")
+			return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "account not found"), nil, sdkerrors.ErrNotFound.Wrapf("account not found")
 		}
 
 		spendableCoins := bk.SpendableCoins(ctx, granterAddr)
@@ -241,11 +241,11 @@ func SimulateMsgExec(ak authz.AccountKeeper, bk authz.BankKeeper, k keeper.Keepe
 
 		grantee, ok := simtypes.FindAccount(accs, granteeAddr)
 		if !ok {
-			return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "Account not found"), nil, sdkerrors.Wrapf(sdkerrors.ErrNotFound, "grantee account not found")
+			return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "Account not found"), nil, sdkerrors.ErrNotFound.Wrapf("grantee account not found")
 		}
 
 		if _, ok := simtypes.FindAccount(accs, granterAddr); !ok {
-			return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "Account not found"), nil, sdkerrors.Wrapf(sdkerrors.ErrNotFound, "granter account not found")
+			return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "Account not found"), nil, sdkerrors.ErrNotFound.Wrapf("granter account not found")
 		}
 
 		granterspendableCoins := bk.SpendableCoins(ctx, granterAddr)
diff --git a/x/staking/types/authz.go b/x/staking/types/authz.go
index 6419ede7a702..4bd567c464c0 100644
--- a/x/staking/types/authz.go
+++ b/x/staking/types/authz.go
@@ -48,10 +48,10 @@ func (a StakeAuthorization) MsgTypeURL() string {
 
 func (a StakeAuthorization) ValidateBasic() error {
 	if a.MaxTokens != nil && a.MaxTokens.IsNegative() {
-		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "negative coin amount: %v", a.MaxTokens)
+		return sdkerrors.Wrapf(authz.ErrNegativeMaxTokens, "negative coin amount: %v", a.MaxTokens)
 	}
 	if a.AuthorizationType == AuthorizationType_AUTHORIZATION_TYPE_UNSPECIFIED {
-		return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "unknown authorization type")
+		return authz.ErrUnknownAuthorizationType
 	}
 
 	return nil
@@ -90,7 +90,7 @@ func (a StakeAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRe
 	for _, validator := range denyList {
 		ctx.GasMeter().ConsumeGas(gasCostPerIteration, "stake authorization")
 		if validator == validatorAddress {
-			return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrapf(" cannot delegate/undelegate to %s validator", validator)
+			return authz.AcceptResponse{}, sdkerrors.ErrUnauthorized.Wrapf("cannot delegate/undelegate to %s validator", validator)
 		}
 	}
 
@@ -148,6 +148,6 @@ func normalizeAuthzType(authzType AuthorizationType) (string, error) {
 	case AuthorizationType_AUTHORIZATION_TYPE_REDELEGATE:
 		return sdk.MsgTypeURL(&MsgBeginRedelegate{}), nil
 	default:
-		return "", sdkerrors.ErrInvalidType.Wrapf("unknown authorization type %T", authzType)
+		return "", sdkerrors.Wrapf(authz.ErrUnknownAuthorizationType, "cannot normalize authz type with %T", authzType)
 	}
 }