Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

revert: "remove time.now check from authz (#10447)" #11106

Merged
merged 2 commits into from
Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,6 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#10868](https://github.com/cosmos/cosmos-sdk/pull/10868) Bump gov to v1beta2. Both v1beta1 and v1beta2 queries and Msgs are accepted.
* [\#11011](https://github.com/cosmos/cosmos-sdk/pull/11011) Remove burning of deposits when qourum is not reached on a governance proposal and when the deposit is not fully met.
* [\#11019](https://github.com/cosmos/cosmos-sdk/pull/11019) Add `MsgCreatePermanentLockedAccount` and CLI method for creating permanent locked account
* (x/authz) [\#10447](https://github.com/cosmos/cosmos-sdk/pull/10447) Remove time.now() check in authz `NewGrant` validation.

### Deprecated

Expand Down
14 changes: 8 additions & 6 deletions x/authz/authorization_grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)

var (
_ cdctypes.UnpackInterfacesMessage = &Grant{}
)

// NewGrant returns new Grant
// NOTE: a new grant is considered invalid if the expiration time is after the
// "current" time - you should assure that before calling this function.
func NewGrant(a Authorization, expiration time.Time) (Grant, error) {
g := Grant{
Expiration: expiration,
Expand All @@ -34,6 +28,10 @@ func NewGrant(a Authorization, expiration time.Time) (Grant, error) {
return g, nil
}

var (
_ cdctypes.UnpackInterfacesMessage = &Grant{}
)

// UnpackInterfaces implements UnpackInterfacesMessage.UnpackInterfaces
func (g Grant) UnpackInterfaces(unpacker cdctypes.AnyUnpacker) error {
var authorization Authorization
Expand All @@ -53,6 +51,10 @@ func (g Grant) GetAuthorization() Authorization {
}

func (g Grant) ValidateBasic() error {
if g.Expiration.Unix() < time.Now().Unix() {
return sdkerrors.Wrap(ErrInvalidExpirationTime, "Time can't be in the past")
}

av := g.Authorization.GetCachedValue()
a, ok := av.(Authorization)
if !ok {
Expand Down
14 changes: 0 additions & 14 deletions x/authz/authorization_grant_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion x/authz/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func NewCmdGrantAuthorization() *cobra.Command {
Use: "grant <grantee> <authorization_type=\"send\"|\"generic\"|\"delegate\"|\"unbond\"|\"redelegate\"> --from <granter>",
Short: "Grant authorization to an address",
Long: strings.TrimSpace(
fmt.Sprintf(`create a new grant authorization to an address to execute a transaction on your behalf:
fmt.Sprintf(`grant authorization to an address to execute a transaction on your behalf:

Examples:
$ %s tx %s grant cosmos1skjw.. send %s --spend-limit=1000stake --from=cosmos1skl..
Expand Down
2 changes: 1 addition & 1 deletion x/authz/client/testutil/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (s *IntegrationTestSuite) TestQueryGrantsGRPC() {
false,
"",
func() {
_, err := CreateGrant(val, []string{
_, err := ExecGrant(val, []string{
grantee.String(),
"generic",
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
Expand Down
4 changes: 2 additions & 2 deletions x/authz/client/testutil/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorizations() {
grantee := s.grantee[0]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -98,7 +98,7 @@ func (s *IntegrationTestSuite) TestQueryAuthorization() {
grantee := s.grantee[0]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down
2 changes: 1 addition & 1 deletion x/authz/client/testutil/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/cosmos/cosmos-sdk/x/authz/client/cli"
)

func CreateGrant(val *network.Validator, args []string) (testutil.BufferWriter, error) {
func ExecGrant(val *network.Validator, args []string) (testutil.BufferWriter, error) {
cmd := cli.NewCmdGrantAuthorization()
clientCtx := val.ClientCtx
return clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
Expand Down
37 changes: 19 additions & 18 deletions x/authz/client/testutil/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.msgSendExec(s.grantee[1])

// grant send authorization to grantee2
out, err := CreateGrant(val, []string{
out, err := ExecGrant(val, []string{
s.grantee[1].String(),
"send",
fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
Expand All @@ -85,7 +85,7 @@ func (s *IntegrationTestSuite) SetupSuite() {
s.grantee[2] = s.createAccount("grantee3")

// grant send authorization to grantee3
out, err = CreateGrant(val, []string{
out, err = ExecGrant(val, []string{
s.grantee[2].String(),
"send",
fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
Expand Down Expand Up @@ -147,8 +147,8 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
val := s.network.Validators[0]
grantee := s.grantee[0]

twoHours := time.Now().Add(time.Minute * 120).Unix()
pastHour := time.Now().Add(-time.Minute * 60).Unix()
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()
pastHour := time.Now().Add(time.Minute * time.Duration(-60)).Unix()

testCases := []struct {
name string
Expand Down Expand Up @@ -189,7 +189,7 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
"send",
fmt.Sprintf("--%s=100steak", cli.FlagSpendLimit),
fmt.Sprintf("--%s=%s", flags.FlagFrom, val.Address.String()),
fmt.Sprintf("--%s=true", flags.FlagBroadcastMode),
fmt.Sprintf("--%s=true", flags.FlagGenerateOnly),
fmt.Sprintf("--%s=%d", cli.FlagExpiration, pastHour),
},
0,
Expand Down Expand Up @@ -340,14 +340,15 @@ func (s *IntegrationTestSuite) TestCLITxGrantAuthorization() {
}

for _, tc := range testCases {
tc := tc
s.Run(tc.name, func() {
clientCtx := val.ClientCtx
out, err := CreateGrant(
out, err := ExecGrant(
val,
tc.args,
)
if tc.expectErr {
s.Require().Error(err, out)
s.Require().Error(err)
} else {
var txResp sdk.TxResponse
s.Require().NoError(err)
Expand All @@ -371,7 +372,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

// send-authorization
_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand All @@ -387,7 +388,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
s.Require().NoError(err)

// generic-authorization
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
Expand All @@ -403,7 +404,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
s.Require().NoError(err)

// generic-authorization used for amino testing
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -516,7 +517,7 @@ func (s *IntegrationTestSuite) TestExecAuthorizationWithExpiration() {
grantee := s.grantee[0]
tenSeconds := time.Now().Add(time.Second * time.Duration(10)).Unix()

_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -556,7 +557,7 @@ func (s *IntegrationTestSuite) TestNewExecGenericAuthorized() {
grantee := s.grantee[0]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -659,7 +660,7 @@ func (s *IntegrationTestSuite) TestNewExecGrantAuthorized() {
grantee1 := s.grantee[2]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -763,7 +764,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
grantee := s.grantee[0]
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -855,7 +856,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
}

// test delegate no spend-limit
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -932,7 +933,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
}

// test delegating to denied validator
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -967,7 +968,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix()

// granting undelegate msg authorization
_, err := CreateGrant(
_, err := ExecGrant(
val,
[]string{
grantee.String(),
Expand Down Expand Up @@ -1076,7 +1077,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
}

// grant undelegate authorization without limit
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
Expand Down
6 changes: 0 additions & 6 deletions x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,6 @@ func (k Keeper) DispatchActions(ctx sdk.Context, grantee sdk.AccAddress, msgs []
// same `sdk.Msg` type, this grant overwrites that.
func (k Keeper) SaveGrant(ctx sdk.Context, grantee, granter sdk.AccAddress, authorization authz.Authorization, expiration time.Time) error {
store := ctx.KVStore(k.storeKey)
blockTime := ctx.BlockTime()
if !expiration.After(blockTime) {
return sdkerrors.ErrInvalidRequest.Wrapf(
"expiration must be after the current block time (%v), got %v",
blockTime.Format(time.RFC3339), expiration.Format(time.RFC3339))
}

grant, err := authz.NewGrant(authorization, expiration)
if err != nil {
Expand Down
21 changes: 12 additions & 9 deletions x/authz/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"github.com/cosmos/cosmos-sdk/baseapp"
"github.com/cosmos/cosmos-sdk/simapp"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/authz"
"github.com/cosmos/cosmos-sdk/x/bank/testutil"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
Expand Down Expand Up @@ -56,12 +55,13 @@ func (s *TestSuite) TestKeeper() {
s.Require().Nil(authorization)
s.Require().Equal(expiration, time.Time{})
now := s.ctx.BlockHeader().Time
s.Require().NotNil(now)

newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100))
s.T().Log("verify if expired authorization is rejected")
x := &banktypes.SendAuthorization{SpendLimit: newCoins}
err := app.AuthzKeeper.SaveGrant(ctx, granterAddr, granteeAddr, x, now.Add(-1*time.Hour))
s.Require().ErrorIs(err, errors.ErrInvalidRequest)
s.Require().NoError(err)
authorization, _ = app.AuthzKeeper.GetCleanAuthorization(ctx, granteeAddr, granterAddr, bankSendAuthMsgType)
s.Require().Nil(authorization)

Expand All @@ -83,7 +83,7 @@ func (s *TestSuite) TestKeeper() {

s.T().Log("verify revoke fails with wrong information")
err = app.AuthzKeeper.DeleteGrant(ctx, recipientAddr, granterAddr, bankSendAuthMsgType)
s.Require().ErrorIs(err, errors.ErrNotFound)
s.Require().Error(err)
authorization, _ = app.AuthzKeeper.GetCleanAuthorization(ctx, recipientAddr, granterAddr, bankSendAuthMsgType)
s.Require().Nil(authorization)

Expand All @@ -105,13 +105,14 @@ func (s *TestSuite) TestKeeperIter() {
authorization, expiration := app.AuthzKeeper.GetCleanAuthorization(ctx, granteeAddr, granterAddr, "Abcd")
s.Require().Nil(authorization)
s.Require().Equal(time.Time{}, expiration)
now := s.ctx.BlockHeader().Time.Add(time.Second)
now := s.ctx.BlockHeader().Time
s.Require().NotNil(now)

newCoins := sdk.NewCoins(sdk.NewInt64Coin("steak", 100))
s.T().Log("verify if expired authorization is rejected")
x := &banktypes.SendAuthorization{SpendLimit: newCoins}
err := app.AuthzKeeper.SaveGrant(ctx, granteeAddr, granterAddr, x, now.Add(-1*time.Hour))
s.Require().Error(err)
s.Require().NoError(err)
authorization, _ = app.AuthzKeeper.GetCleanAuthorization(ctx, granteeAddr, granterAddr, "abcd")
s.Require().Nil(authorization)

Expand All @@ -130,7 +131,8 @@ func (s *TestSuite) TestKeeperFees() {
granteeAddr := addrs[1]
recipientAddr := addrs[2]
s.Require().NoError(testutil.FundAccount(app.BankKeeper, s.ctx, granterAddr, sdk.NewCoins(sdk.NewInt64Coin("steak", 10000))))
expiration := s.ctx.BlockHeader().Time.Add(1 * time.Second)
now := s.ctx.BlockHeader().Time
s.Require().NotNil(now)

smallCoin := sdk.NewCoins(sdk.NewInt64Coin("steak", 20))
someCoin := sdk.NewCoins(sdk.NewInt64Coin("steak", 123))
Expand All @@ -155,7 +157,7 @@ func (s *TestSuite) TestKeeperFees() {

s.T().Log("verify dispatch executes with correct information")
// grant authorization
err = app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, expiration)
err = app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, now)
s.Require().NoError(err)
authorization, _ := app.AuthzKeeper.GetCleanAuthorization(s.ctx, granteeAddr, granterAddr, bankSendAuthMsgType)
s.Require().NotNil(authorization)
Expand Down Expand Up @@ -204,7 +206,8 @@ func (s *TestSuite) TestDispatchedEvents() {
granteeAddr := addrs[1]
recipientAddr := addrs[2]
require.NoError(testutil.FundAccount(app.BankKeeper, s.ctx, granterAddr, sdk.NewCoins(sdk.NewInt64Coin("steak", 10000))))
expiration := s.ctx.BlockHeader().Time.Add(1 * time.Second) // must be in the future
now := s.ctx.BlockHeader().Time
require.NotNil(now)

smallCoin := sdk.NewCoins(sdk.NewInt64Coin("steak", 20))
msgs := authz.NewMsgExec(granteeAddr, []sdk.Msg{
Expand All @@ -216,7 +219,7 @@ func (s *TestSuite) TestDispatchedEvents() {
})

// grant authorization
err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, expiration)
err := app.AuthzKeeper.SaveGrant(s.ctx, granteeAddr, granterAddr, &banktypes.SendAuthorization{SpendLimit: smallCoin}, now)
require.NoError(err)
authorization, _ := app.AuthzKeeper.GetCleanAuthorization(s.ctx, granteeAddr, granterAddr, bankSendAuthMsgType)
require.NotNil(authorization)
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 @@ -10,7 +10,7 @@ import (

var _ authz.MsgServer = Keeper{}

// GrantAuthorization implements the MsgServer.Grant method to create a new grant.
// GrantAuthorization implements the MsgServer.Grant method.
func (k Keeper) Grant(goCtx context.Context, msg *authz.MsgGrant) (*authz.MsgGrantResponse, error) {
ctx := sdk.UnwrapSDKContext(goCtx)
grantee, err := sdk.AccAddressFromBech32(msg.Grantee)
Expand Down
2 changes: 1 addition & 1 deletion x/authz/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestMsgGrantAuthorization(t *testing.T) {
{"nil granter and grantee address", nil, nil, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now(), false, false},
{"nil authorization", granter, grantee, nil, time.Now(), true, false},
{"valid test case", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 1, 0), false, true},
{"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), true, true},
{"past time", granter, grantee, &banktypes.SendAuthorization{SpendLimit: coinsPos}, time.Now().AddDate(0, 0, -1), false, false},
}
for i, tc := range tests {
msg, err := authz.NewMsgGrant(
Expand Down
3 changes: 1 addition & 2 deletions x/authz/simulation/decoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ func TestDecodeStore(t *testing.T) {
cdc := simapp.MakeTestEncodingConfig().Codec
dec := simulation.NewDecodeStore(cdc)

grant, _ := authz.NewGrant(banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123))),
time.Now().Add(10*time.Minute).UTC())
grant, _ := authz.NewGrant(banktypes.NewSendAuthorization(sdk.NewCoins(sdk.NewInt64Coin("foo", 123))), time.Now().UTC())
grantBz, err := cdc.Marshal(&grant)
require.NoError(t, err)
kvPairs := kv.Pairs{
Expand Down
Loading