Skip to content

Commit

Permalink
refactor: register x/authz module specific errors (#11670)
Browse files Browse the repository at this point in the history
  • Loading branch information
julienrbrt committed Apr 19, 2022
1 parent 9bed633 commit 2af642e
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 34 deletions.
8 changes: 4 additions & 4 deletions x/authz/authorization_grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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()
}
13 changes: 7 additions & 6 deletions x/authz/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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,
"",
},
Expand Down Expand Up @@ -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(),
},
}

Expand Down Expand Up @@ -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(),
},
}

Expand Down
15 changes: 15 additions & 0 deletions x/authz/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)
2 changes: 1 addition & 1 deletion x/authz/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
19 changes: 11 additions & 8 deletions x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}

Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion x/authz/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 8 additions & 7 deletions x/authz/msgs.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions x/authz/simulation/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions x/staking/types/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 2af642e

Please sign in to comment.