refactor: x/authz minor code improvements (#11840)

## Description Closes: #11569 --- ### Author Checklist *All items are required. Please add a note to the item if the item is not applicable and please add links to any relevant follow up issues.* I have... - [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] added `!` to the type prefix if API or client breaking change - [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting)) - [ ] provided a link to the relevant issue or specification - [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules) - [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing) - [ ] added a changelog entry to `CHANGELOG.md` - [ ] included comments for [documenting Go code](https://blog.golang.org/godoc) - [ ] updated the relevant documentation or specification - [ ] reviewed "Files changed" and left comments if necessary - [ ] confirmed all CI checks have passed ### Reviewers Checklist *All items are required. Please add a note if the item is not applicable and please add your handle next to the items reviewed if you only reviewed selected items.* I have... - [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [ ] confirmed `!` in the type prefix if API or client breaking change - [ ] confirmed all author checklist items have been addressed - [ ] reviewed state machine logic - [ ] reviewed API design and naming - [ ] reviewed documentation is accurate - [ ] reviewed tests and test coverage - [ ] manually tested (if applicable)
2022-05-09 17:30:16 +05:30 · 2022-05-09 17:30:16 +05:30 · ead6e6f948
commit ead6e6f948
parent 986008422c
7 changed files with 132 additions and 20 deletions
--- a/x/authz/authorization_grant.go
+++ b/x/authz/authorization_grant.go
@ -45,9 +45,10 @@ func (g Grant) GetAuthorization() (Authorization, error) {
 	if g.Authorization == nil {
 		return nil, sdkerrors.ErrInvalidType.Wrap("authorization is nil")
 	}
-	a, ok := g.Authorization.GetCachedValue().(Authorization)
+	av := g.Authorization.GetCachedValue()
+	a, ok := av.(Authorization)
 	if !ok {
-		return nil, sdkerrors.ErrInvalidType.Wrapf("expected %T, got %T", (Authorization)(nil), g.Authorization.GetCachedValue())
+		return nil, sdkerrors.ErrInvalidType.Wrapf("expected %T, got %T", (Authorization)(nil), av)
 	}
 	return a, nil
 }
--- a/x/authz/client/cli/tx.go
+++ b/x/authz/client/cli/tx.go
@ -118,15 +118,25 @@ Examples:

 				var delegateLimit *sdk.Coin
 				if limit != "" {
-					spendLimit, err := sdk.ParseCoinsNormalized(limit)
+					spendLimit, err := sdk.ParseCoinNormalized(limit)
+					if err != nil {
+						return err
+					}
+					queryClient := staking.NewQueryClient(clientCtx)
+
+					res, err := queryClient.Params(cmd.Context(), &staking.QueryParamsRequest{})
 					if err != nil {
 						return err
 					}

-					if !spendLimit.IsAllPositive() {
+					if spendLimit.Denom != res.Params.BondDenom {
+						return fmt.Errorf("invalid denom %s; coin denom should match the current bond denom %s", spendLimit.Denom, res.Params.BondDenom)
+					}
+
+					if !spendLimit.IsPositive() {
 						return fmt.Errorf("spend-limit should be greater than zero")
 					}
-					delegateLimit = &spendLimit[0]
+					delegateLimit = &spendLimit
 				}

 				allowed, err := bech32toValidatorAddresses(allowValidators)
--- a/x/authz/client/testutil/query.go
+++ b/x/authz/client/testutil/query.go
@ -25,7 +25,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorizations() {
 		[]string{
 			grantee.String(),
 			"send",
-			fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+			fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 			fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 			fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address),
 			fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
@ -103,7 +103,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorization() {
 		[]string{
 			grantee.String(),
 			"send",
-			fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+			fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 			fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 			fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address),
 			fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
@ -161,7 +161,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorization() {
 				fmt.Sprintf("--%s=json", tmcli.OutputFlag),
 			},
 			false,
-			`{"@type":"/cosmos.bank.v1beta1.SendAuthorization","spend_limit":[{"denom":"steak","amount":"100"}]}`,
+			`{"@type":"/cosmos.bank.v1beta1.SendAuthorization","spend_limit":[{"denom":"stake","amount":"100"}]}`,
 		},
 	}
 	for _, tc := range testCases {
--- a/x/authz/client/testutil/tx.go
+++ b/x/authz/client/testutil/tx.go
@ -69,7 +69,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
 	out, err := CreateGrant(val, []string{
 		s.grantee[1].String(),
 		"send",
-		fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+		fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 		fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
 		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
@ -89,7 +89,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
 	out, err = CreateGrant(val, []string{
 		s.grantee[2].String(),
 		"send",
-		fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+		fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 		fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
 		fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 		fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
@ -156,45 +156,49 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 		args         []string
 		expectedCode uint32
 		expectErr    bool
+		expErrMsg    string
 	}{
 		{
 			"Invalid granter Address",
 			[]string{
 				"grantee_addr",
 				"send",
-				fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, "granter"),
 				fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 			},
 			0,
 			true,
+			"key not found",
 		},
 		{
 			"Invalid grantee Address",
 			[]string{
 				"grantee_addr",
 				"send",
-				fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
 				fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
 			},
 			0,
 			true,
+			"invalid separator index",
 		},
 		{
 			"Invalid expiration time",
 			[]string{
 				grantee.String(),
 				"send",
-				fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
 				fmt.Sprintf("--%s=true", flags.FlagBroadcastMode),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, pastHour),
 			},
 			0,
 			true,
+			"",
 		},
 		{
 			"fail with error invalid msg-type",
@ -210,6 +214,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0x1d,
 			false,
+			"",
 		},
 		{
 			"failed with error both validators not allowed",
@ -227,6 +232,92 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			true,
+			"cannot set both allowed & deny list",
+		},
+		{
+			"invalid bond denom for tx delegate authorization allowed validators",
+			[]string{
+				grantee.String(),
+				"delegate",
+				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
+				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+			},
+			0,
+			true,
+			"invalid denom",
+		},
+		{
+			"invalid bond denom for tx delegate authorization deny validators",
+			[]string{
+				grantee.String(),
+				"delegate",
+				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
+				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
+				fmt.Sprintf("--%s=%s", cli.FlagDenyValidators, val.ValAddress.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+			},
+			0,
+			true,
+			"invalid denom",
+		},
+		{
+			"invalid bond denom for tx undelegate authorization",
+			[]string{
+				grantee.String(),
+				"unbond",
+				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
+				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+			},
+			0,
+			true,
+			"invalid denom",
+		},
+		{
+			"invalid bond denon for tx redelegate authorization",
+			[]string{
+				grantee.String(),
+				"redelegate",
+				fmt.Sprintf("--%s=100xyz", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
+				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+			},
+			0,
+			true,
+			"invalid denom",
+		},
+		{
+			"invalid decimal coin expression with more than single coin",
+			[]string{
+				grantee.String(),
+				"delegate",
+				fmt.Sprintf("--%s=100stake,20xyz", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
+				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
+				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
+				fmt.Sprintf("--%s=%s", cli.FlagAllowedValidators, val.ValAddress.String()),
+				fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
+			},
+			0,
+			true,
+			"invalid decimal coin expression",
 		},
 		{
 			"valid tx delegate authorization allowed validators",
@ -243,6 +334,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 		{
 			"valid tx delegate authorization deny validators",
@ -259,6 +351,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 		{
 			"valid tx undelegate authorization",
@ -275,6 +368,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 		{
 			"valid tx redelegate authorization",
@ -291,13 +385,14 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 		{
 			"Valid tx send authorization",
 			[]string{
 				grantee.String(),
 				"send",
-				fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+				fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 				fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
 				fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
 				fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours),
@ -306,6 +401,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 		{
 			"Valid tx generic authorization",
@ -321,6 +417,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 		{
 			"fail when granter = grantee",
@ -336,6 +433,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			true,
+			"grantee and granter should be different",
 		},
 		{
 			"Valid tx with amino",
@ -352,6 +450,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			},
 			0,
 			false,
+			"",
 		},
 	}

@ -364,6 +463,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
 			)
 			if tc.expectErr {
 				s.Require().Error(err, out)
+				s.Require().Contains(err.Error(), tc.expErrMsg)
 			} else {
 				var txResp sdk.TxResponse
 				s.Require().NoError(err)
@ -392,7 +492,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
 		[]string{
 			grantee.String(),
 			"send",
-			fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
+			fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit),
 			fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
 			fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address),
 			fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
--- a/x/authz/keeper/grpc_query.go
+++ b/x/authz/keeper/grpc_query.go
@ -20,6 +20,7 @@ import (
 var _ authz.QueryServer = Keeper{}

 // Authorizations implements the Query/Grants gRPC method.
+// It returns grants for a granter-grantee pair. If msg type URL is set, it returns grants only for that msg type.
 func (k Keeper) Grants(c context.Context, req *authz.QueryGrantsRequest) (*authz.QueryGrantsResponse, error) {
 	if req == nil {
 		return nil, status.Errorf(codes.InvalidArgument, "empty request")
--- a/x/authz/keeper/keeper.go
+++ b/x/authz/keeper/keeper.go
@ -199,8 +199,6 @@ func (k Keeper) DeleteGrant(ctx sdk.Context, grantee sdk.AccAddress, granter sdk
 		return sdkerrors.Wrapf(authz.ErrNoAuthorizationFound, "failed to delete grant with key %s", string(skey))
 	}

-	store.Delete(skey)
-
 	if grant.Expiration != nil {
 		err := k.removeFromGrantQueue(ctx, skey, granter, grantee, *grant.Expiration)
 		if err != nil {
@ -208,6 +206,8 @@ func (k Keeper) DeleteGrant(ctx sdk.Context, grantee sdk.AccAddress, granter sdk
 		}
 	}

+	store.Delete(skey)
+
 	return ctx.EventManager().EmitTypedEvent(&authz.EventRevoke{
 		MsgTypeUrl: msgType,
 		Granter:    granter.String(),
--- a/x/staking/types/authz.go
+++ b/x/staking/types/authz.go
@ -10,14 +10,13 @@ import (
 // Tracking issues https://github.com/cosmos/cosmos-sdk/issues/9054, https://github.com/cosmos/cosmos-sdk/discussions/9072
 const gasCostPerIteration = uint64(10)

-// Normalized Msg type URLs
 var (
 	_ authz.Authorization = &StakeAuthorization{}
 )

 // NewStakeAuthorization creates a new StakeAuthorization object.
 func NewStakeAuthorization(allowed []sdk.ValAddress, denied []sdk.ValAddress, authzType AuthorizationType, amount *sdk.Coin) (*StakeAuthorization, error) {
-	allowedValidators, deniedValidators, err := validateAndBech32fy(allowed, denied)
+	allowedValidators, deniedValidators, err := validateAllowAndDenyValidators(allowed, denied)
 	if err != nil {
 		return nil, err
 	}
@ -114,7 +113,7 @@ func (a StakeAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRe
 		Updated: &StakeAuthorization{Validators: a.GetValidators(), AuthorizationType: a.GetAuthorizationType(), MaxTokens: &limitLeft}}, nil
 }

-func validateAndBech32fy(allowed []sdk.ValAddress, denied []sdk.ValAddress) ([]string, []string, error) {
+func validateAllowAndDenyValidators(allowed []sdk.ValAddress, denied []sdk.ValAddress) ([]string, []string, error) {
 	if len(allowed) == 0 && len(denied) == 0 {
 		return nil, nil, sdkerrors.ErrInvalidRequest.Wrap("both allowed & deny list cannot be empty")
 	}
@ -139,6 +138,7 @@ func validateAndBech32fy(allowed []sdk.ValAddress, denied []sdk.ValAddress) ([]s
 	return nil, deniedValidators, nil
 }

+// Normalized Msg type URLs
 func normalizeAuthzType(authzType AuthorizationType) (string, error) {
 	switch authzType {
 	case AuthorizationType_AUTHORIZATION_TYPE_DELEGATE: