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

This reverts commit 077154a882.
This commit is contained in:
Amaury 2022-02-03 17:01:33 +01:00 committed by GitHub
parent ee6bedc742
commit ab226ff7ec
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 57 additions and 77 deletions

View File

@ -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

View File

@ -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,
@ -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
@ -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 {

View File

@ -1,14 +0,0 @@
package authz
import (
"testing"
"time"
"github.com/stretchr/testify/require"
)
func TestNewGrant(t *testing.T) {
a := NewGenericAuthorization("some-type")
_, err := NewGrant(a, time.Unix(10, 0))
require.NoError(t, err)
}

View File

@ -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..

View File

@ -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()),

View File

@ -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(),
@ -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(),

View File

@ -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)

View File

@ -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),
@ -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),
@ -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
@ -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,
@ -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)
@ -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(),
@ -387,7 +388,7 @@ func (s *IntegrationTestSuite) TestCmdRevokeAuthorizations() {
s.Require().NoError(err)
// generic-authorization
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
@ -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(),
@ -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(),
@ -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(),
@ -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(),
@ -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(),
@ -855,7 +856,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
}
// test delegate no spend-limit
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
@ -932,7 +933,7 @@ func (s *IntegrationTestSuite) TestExecDelegateAuthorization() {
}
// test delegating to denied validator
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),
@ -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(),
@ -1076,7 +1077,7 @@ func (s *IntegrationTestSuite) TestExecUndelegateAuthorization() {
}
// grant undelegate authorization without limit
_, err = CreateGrant(
_, err = ExecGrant(
val,
[]string{
grantee.String(),

View File

@ -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 {

View File

@ -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"
@ -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)
@ -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)
@ -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)
@ -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))
@ -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)
@ -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{
@ -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)

View File

@ -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)

View File

@ -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(

View File

@ -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{

View File

@ -2,7 +2,6 @@ package simulation
import (
"math/rand"
"time"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
@ -14,7 +13,7 @@ import (
)
// genGrant returns a slice of authorization grants.
func genGrant(r *rand.Rand, accounts []simtypes.Account, genesisT time.Time) []authz.GrantAuthorization {
func genGrant(r *rand.Rand, accounts []simtypes.Account) []authz.GrantAuthorization {
authorizations := make([]authz.GrantAuthorization, len(accounts)-1)
for i := 0; i < len(accounts)-1; i++ {
granter := accounts[i]
@ -23,7 +22,6 @@ func genGrant(r *rand.Rand, accounts []simtypes.Account, genesisT time.Time) []a
Granter: granter.Address.String(),
Grantee: grantee.Address.String(),
Authorization: generateRandomGrant(r),
Expiration: genesisT.Add(time.Hour + time.Hour*time.Duration(r.Intn(100000))),
}
}
@ -52,7 +50,7 @@ func RandomizedGenState(simState *module.SimulationState) {
var grants []authz.GrantAuthorization
simState.AppParams.GetOrGenerate(
simState.Cdc, "authz", &grants, simState.Rand,
func(r *rand.Rand) { grants = genGrant(r, simState.Accounts, simState.GenTimestamp) },
func(r *rand.Rand) { grants = genGrant(r, simState.Accounts) },
)
authzGrantsGenesis := authz.NewGenesisState(grants)

View File

@ -135,7 +135,7 @@ func SimulateMsgGrant(ak authz.AccountKeeper, bk authz.BankKeeper, _ keeper.Keep
if err != nil {
return simtypes.NoOpMsg(authz.ModuleName, sdk.MsgTypeURL(msg), "unable to deliver tx"), nil, err
}
return simtypes.NewOperationMsg(msg, true, "success", nil), nil, err
return simtypes.NewOperationMsg(msg, true, "", nil), nil, err
}
}
@ -202,7 +202,7 @@ func SimulateMsgRevoke(ak authz.AccountKeeper, bk authz.BankKeeper, k keeper.Kee
return simtypes.NoOpMsg(authz.ModuleName, TypeMsgRevoke, "unable to deliver tx"), nil, err
}
return simtypes.NewOperationMsg(&msg, true, "success", nil), nil, nil
return simtypes.NewOperationMsg(&msg, true, "", nil), nil, nil
}
}

View File

@ -49,11 +49,10 @@ func (suite *SimTestSuite) TestWeightedOperations() {
weight int
opMsgRoute string
opMsgName string
comment string
}{
{simulation.WeightGrant, sdk.MsgTypeURL(&authz.MsgGrant{}), simulation.TypeMsgGrant, "success"},
{simulation.WeightRevoke, sdk.MsgTypeURL(&authz.MsgRevoke{}), simulation.TypeMsgRevoke, "success"},
{simulation.WeightExec, authz.ModuleName, simulation.TypeMsgExec, "no grant found"},
{simulation.WeightGrant, authz.ModuleName, simulation.TypeMsgGrant},
{simulation.WeightRevoke, authz.ModuleName, simulation.TypeMsgRevoke},
{simulation.WeightExec, authz.ModuleName, simulation.TypeMsgExec},
}
for i, w := range weightedOps {
@ -61,10 +60,9 @@ func (suite *SimTestSuite) TestWeightedOperations() {
// the following checks are very much dependent from the ordering of the output given
// by WeightedOperations. if the ordering in WeightedOperations changes some tests
// will fail
suite.Require().Equal(expected[i].weight, w.Weight(), "test: %d, weight should be the same", i)
suite.Require().Equal(expected[i].opMsgRoute, operationMsg.Route, "test: %d, route should be the same", i)
suite.Require().Equal(expected[i].opMsgName, operationMsg.Name, "test: %d, operation Msg name should be the same", i)
suite.Require().Equal(expected[i].comment, operationMsg.Comment, "test: %d, wrong operation comment", i)
suite.Require().Equal(expected[i].weight, w.Weight(), "weight should be the same")
suite.Require().Equal(expected[i].opMsgRoute, operationMsg.Route, "route should be the same")
suite.Require().Equal(expected[i].opMsgName, operationMsg.Name, "operation Msg name should be the same")
}
}