diff --git a/x/authz/authorization_grant.go b/x/authz/authorization_grant.go index 9e0005a5cee5..4344bd3b7b58 100644 --- a/x/authz/authorization_grant.go +++ b/x/authz/authorization_grant.go @@ -43,9 +43,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 } diff --git a/x/authz/client/cli/tx.go b/x/authz/client/cli/tx.go index 161ef1feb878..f094faea46d3 100644 --- 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) - if !spendLimit.IsAllPositive() { + res, err := queryClient.Params(cmd.Context(), &staking.QueryParamsRequest{}) + if err != nil { + return err + } + + 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) diff --git a/x/authz/client/testutil/query.go b/x/authz/client/testutil/query.go index 56296576507b..45921a054de6 100644 --- 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 { diff --git a/x/authz/client/testutil/tx.go b/x/authz/client/testutil/tx.go index 3102c21e3bfc..ecac6abce78e 100644 --- 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), @@ -157,45 +157,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", @@ -211,6 +215,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0x1d, false, + "", }, { "failed with error both validators not allowed", @@ -228,6 +233,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", @@ -244,6 +335,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, false, + "", }, { "valid tx delegate authorization deny validators", @@ -260,6 +352,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, false, + "", }, { "valid tx undelegate authorization", @@ -276,6 +369,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, false, + "", }, { "valid tx redelegate authorization", @@ -292,13 +386,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), @@ -307,6 +402,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, false, + "", }, { "Valid tx generic authorization", @@ -322,6 +418,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, false, + "", }, { "fail when granter = grantee", @@ -337,6 +434,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, true, + "grantee and granter should be different", }, { "Valid tx with amino", @@ -353,6 +451,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() { }, 0, false, + "", }, } @@ -365,6 +464,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) @@ -393,7 +493,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), diff --git a/x/authz/keeper/grpc_query.go b/x/authz/keeper/grpc_query.go index 8d70e468e873..488b948d252f 100644 --- 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") diff --git a/x/authz/keeper/keeper.go b/x/authz/keeper/keeper.go index 605e54d35a1b..21d49651bd0a 100644 --- 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(), diff --git a/x/staking/types/authz.go b/x/staking/types/authz.go index 3dd00064061b..9f2efe851dd8 100644 --- 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 } @@ -118,7 +117,7 @@ func (a StakeAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.AcceptRe }, 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") } @@ -143,6 +142,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: