From 86579f295110d8bdc6cfd55097e87f3c223d6b2c Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Fri, 23 Apr 2021 15:10:26 -0700 Subject: [PATCH 1/4] Squashed commit of the following: commit 58dc50051226a9eeb8d0ebea5bb0908fe5b9637f Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Fri Apr 23 15:09:27 2021 -0700 remove comments commit a84107e1b3eaa31324cb0f4f097b49f02af79c69 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Fri Apr 23 15:02:41 2021 -0700 add tests for msgs.go commit 2ad16869237e9631b402c93cde650c3fc554daf2 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Fri Apr 23 13:20:21 2021 -0700 specify test name commit b7121277c9be586a7c80d010ec401e50b510e02a Merge: c0c134d971 3c65c3dacd Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Fri Apr 23 12:54:55 2021 -0700 Merge branch 'master' into ty-9115-types_tests commit c0c134d97107194dc4f9d3c501a15d023ae083e5 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Thu Apr 22 19:59:11 2021 -0700 -add test case to cli_test.go for filtered fee -clean up identifiers -remove redundant import alias from filtered_fee.go commit f7ab3699da39be3ab886f96962d28d23438d2e8e Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Thu Apr 22 09:57:31 2021 -0700 remove unecessary constant commit 9db59a82a7337cf5a7a3569c6900a44a6c81e8b4 Merge: a3e75ceb8a e28271b8e6 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Thu Apr 22 09:19:20 2021 -0700 Merge branch 'master' into ty-9115-types_tests commit a3e75ceb8a510ad9db43dd96073c43b7a8b062b0 Merge: 4d3dafab85 bffcae54a1 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Wed Apr 21 12:04:39 2021 -0700 Merge branch 'master' into ty-9115-types_tests commit 4d3dafab85d85526a7c94b045289605289ee6b0d Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Wed Apr 21 12:03:41 2021 -0700 cleanup id's / add test case commit e8f6924931ba95e592bfc3057ba167700458da41 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Wed Apr 21 10:55:29 2021 -0700 add test for 0 allowance / remove unused field on exp test commit 516b6ef89a4582ad681cc6c5c101cf50a9a54fb5 Author: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Tue Apr 20 15:22:48 2021 -0700 make names more clear, remove unused field --- x/feegrant/client/testutil/suite.go | 41 ++++++++-------- x/feegrant/simulation/operations.go | 14 +++--- x/feegrant/types/basic_fee_test.go | 46 +++++++----------- x/feegrant/types/expiration_test.go | 9 ---- x/feegrant/types/filtered_fee.go | 3 +- x/feegrant/types/grant_test.go | 16 +++++++ x/feegrant/types/msgs.go | 14 +++--- x/feegrant/types/msgs_test.go | 69 +++++++++++++++++++++++++++ x/feegrant/types/periodic_fee_test.go | 36 +++++++------- 9 files changed, 157 insertions(+), 91 deletions(-) create mode 100644 x/feegrant/types/msgs_test.go diff --git a/x/feegrant/client/testutil/suite.go b/x/feegrant/client/testutil/suite.go index a247ac7e8fea..be64a0b892e3 100644 --- a/x/feegrant/client/testutil/suite.go +++ b/x/feegrant/client/testutil/suite.go @@ -629,7 +629,7 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { } spendLimit := sdk.NewCoin("stake", sdk.NewInt(1000)) - allowMsgs := "/cosmos.gov.v1beta1.Msg/SubmitProposal" + allowMsgs := "/cosmos.gov.v1beta1.Msg/SubmitProposal,weighted_vote" testCases := []struct { name string @@ -639,10 +639,10 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { expectedCode uint32 }{ { - "wrong granter", + "invalid granter address", append( []string{ - "wrong granter", + "not an address", "cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl", fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs), fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()), @@ -653,11 +653,11 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { true, &sdk.TxResponse{}, 0, }, { - "wrong grantee", + "invalid grantee address", append( []string{ granter.String(), - "wrong grantee", + "not an address", fmt.Sprintf("--%s=%s", cli.FlagAllowedMsgs, allowMsgs), fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, spendLimit.String()), fmt.Sprintf("--%s=%s", flags.FlagFrom, granter), @@ -733,22 +733,30 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { cases := []struct { name string malleate func() (testutil.BufferWriter, error) - expectErr bool respType proto.Message expectedCode uint32 }{ { - "valid tx", + "valid proposal tx", func() (testutil.BufferWriter, error) { return govtestutil.MsgSubmitProposal(val.ClientCtx, grantee.String(), "Text Proposal", "No desc", govtypes.ProposalTypeText, fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()), ) }, - false, &sdk.TxResponse{}, 0, }, + { + "valid weighted_vote tx", + func() (testutil.BufferWriter, error) { + return govtestutil.MsgVote(val.ClientCtx, grantee.String(), "0", "yes", + fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()), + ) + }, + &sdk.TxResponse{}, + 2, + }, { "should fail with unauthorized msgs", func() (testutil.BufferWriter, error) { @@ -764,7 +772,8 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { cmd := cli.NewCmdFeeGrant() return clitestutil.ExecTestCLICmd(clientCtx, cmd, args) }, - false, &sdk.TxResponse{}, 7, + &sdk.TxResponse{}, + 7, }, } @@ -773,16 +782,10 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { s.Run(tc.name, func() { out, err := tc.malleate() - - if tc.expectErr { - s.Require().Error(err) - } else { - s.Require().NoError(err) - s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), tc.respType), out.String()) - - txResp := tc.respType.(*sdk.TxResponse) - s.Require().Equal(tc.expectedCode, txResp.Code, out.String()) - } + s.Require().NoError(err) + s.Require().NoError(clientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), tc.respType), out.String()) + txResp := tc.respType.(*sdk.TxResponse) + s.Require().Equal(tc.expectedCode, txResp.Code, out.String()) }) } } diff --git a/x/feegrant/simulation/operations.go b/x/feegrant/simulation/operations.go index 6e53738b2df6..a9a7978b428f 100644 --- a/x/feegrant/simulation/operations.go +++ b/x/feegrant/simulation/operations.go @@ -68,11 +68,11 @@ func SimulateMsgGrantFeeAllowance(ak types.AccountKeeper, bk types.BankKeeper, k granter, _ := simtypes.RandomAcc(r, accs) grantee, _ := simtypes.RandomAcc(r, accs) if grantee.Address.String() == granter.Address.String() { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, "grantee and granter cannot be same"), nil, nil + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, "grantee and granter cannot be same"), nil, nil } if f, _ := k.GetFeeAllowance(ctx, granter.Address, grantee.Address); f != nil { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, "fee allowance exists"), nil, nil + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, "fee allowance exists"), nil, nil } account := ak.GetAccount(ctx, granter.Address) @@ -80,12 +80,12 @@ func SimulateMsgGrantFeeAllowance(ak types.AccountKeeper, bk types.BankKeeper, k spendableCoins := bk.SpendableCoins(ctx, account.GetAddress()) fees, err := simtypes.RandomFees(r, ctx, spendableCoins) if err != nil { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, err.Error()), nil, err + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, err.Error()), nil, err } spendableCoins = spendableCoins.Sub(fees) if spendableCoins.Empty() { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, "unable to grant empty coins as SpendLimit"), nil, nil + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, "unable to grant empty coins as SpendLimit"), nil, nil } msg, err := types.NewMsgGrantFeeAllowance(&types.BasicFeeAllowance{ @@ -94,14 +94,14 @@ func SimulateMsgGrantFeeAllowance(ak types.AccountKeeper, bk types.BankKeeper, k }, granter.Address, grantee.Address) if err != nil { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, err.Error()), nil, err + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, err.Error()), nil, err } txGen := simappparams.MakeTestEncodingConfig().TxConfig svcMsgClientConn := &msgservice.ServiceMsgClientConn{} feegrantMsgClient := types.NewMsgClient(svcMsgClientConn) _, err = feegrantMsgClient.GrantFeeAllowance(context.Background(), msg) if err != nil { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, err.Error()), nil, err + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, err.Error()), nil, err } tx, err := helpers.GenTx( txGen, @@ -175,7 +175,7 @@ func SimulateMsgRevokeFeeAllowance(ak types.AccountKeeper, bk types.BankKeeper, feegrantMsgClient := types.NewMsgClient(svcMsgClientConn) _, err = feegrantMsgClient.RevokeFeeAllowance(context.Background(), &msg) if err != nil { - return simtypes.NoOpMsg(types.ModuleName, types.TypeMsgGrantFeeAllowance, err.Error()), nil, err + return simtypes.NoOpMsg(types.ModuleName, TypeMsgGrantFeeAllowance, err.Error()), nil, err } tx, err := helpers.GenTx( diff --git a/x/feegrant/types/basic_fee_test.go b/x/feegrant/types/basic_fee_test.go index 25a5eb755cdd..b6113e52a69e 100644 --- a/x/feegrant/types/basic_fee_test.go +++ b/x/feegrant/types/basic_fee_test.go @@ -22,53 +22,47 @@ func TestBasicFeeValidAllow(t *testing.T) { leftAtom := sdk.NewCoins(sdk.NewInt64Coin("atom", 512)) cases := map[string]struct { - allow *types.BasicFeeAllowance + allowance *types.BasicFeeAllowance // all other checks are ignored if valid=false fee sdk.Coins blockHeight int64 - valid bool accept bool - remove bool + remove bool remains sdk.Coins }{ "empty": { - allow: &types.BasicFeeAllowance{}, - valid: true, + allowance: &types.BasicFeeAllowance{}, accept: true, }, "small fee without expire": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ SpendLimit: atom, }, - valid: true, fee: smallAtom, accept: true, remove: false, remains: leftAtom, }, "all fee without expire": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ SpendLimit: smallAtom, }, - valid: true, fee: smallAtom, accept: true, remove: true, }, "wrong fee": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ SpendLimit: smallAtom, }, - valid: true, fee: eth, accept: false, }, "non-expired": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), }, - valid: true, fee: smallAtom, blockHeight: 85, accept: true, @@ -76,40 +70,36 @@ func TestBasicFeeValidAllow(t *testing.T) { remains: leftAtom, }, "expired": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), }, - valid: true, fee: smallAtom, blockHeight: 121, accept: false, remove: true, }, "fee more than allowed": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), }, - valid: true, fee: bigAtom, blockHeight: 85, accept: false, }, "with out spend limit": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ Expiration: types.ExpiresAtHeight(100), }, - valid: true, fee: bigAtom, blockHeight: 85, accept: true, }, "expired no spend limit": { - allow: &types.BasicFeeAllowance{ + allowance: &types.BasicFeeAllowance{ Expiration: types.ExpiresAtHeight(100), }, - valid: true, fee: bigAtom, blockHeight: 120, accept: false, @@ -119,26 +109,22 @@ func TestBasicFeeValidAllow(t *testing.T) { for name, stc := range cases { tc := stc // to make scopelint happy t.Run(name, func(t *testing.T) { - err := tc.allow.ValidateBasic() - if !tc.valid { - require.Error(t, err) - return - } + err := tc.allowance.ValidateBasic() require.NoError(t, err) ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockHeight(tc.blockHeight) // now try to deduct - remove, err := tc.allow.Accept(ctx, tc.fee, []sdk.Msg{}) + removed, err := tc.allowance.Accept(ctx, tc.fee, []sdk.Msg{}) if !tc.accept { require.Error(t, err) return } require.NoError(t, err) - require.Equal(t, tc.remove, remove) - if !remove { - assert.Equal(t, tc.allow.SpendLimit, tc.remains) + require.Equal(t, tc.remove, removed) + if !removed { + assert.Equal(t, tc.allowance.SpendLimit, tc.remains) } }) } diff --git a/x/feegrant/types/expiration_test.go b/x/feegrant/types/expiration_test.go index 5f390c328a8b..b164359e755d 100644 --- a/x/feegrant/types/expiration_test.go +++ b/x/feegrant/types/expiration_test.go @@ -15,32 +15,27 @@ func TestExpiresAt(t *testing.T) { cases := map[string]struct { example types.ExpiresAt - valid bool zero bool before types.ExpiresAt after types.ExpiresAt }{ "basic": { example: types.ExpiresAtHeight(100), - valid: true, before: types.ExpiresAtHeight(50), after: types.ExpiresAtHeight(122), }, "zero": { example: types.ExpiresAt{}, zero: true, - valid: true, before: types.ExpiresAtHeight(1), }, "match height": { example: types.ExpiresAtHeight(1000), - valid: true, before: types.ExpiresAtHeight(999), after: types.ExpiresAtHeight(1000), }, "match time": { example: types.ExpiresAtTime(now), - valid: true, before: types.ExpiresAtTime(now.Add(-1 * time.Second)), after: types.ExpiresAtTime(now.Add(1 * time.Second)), }, @@ -51,10 +46,6 @@ func TestExpiresAt(t *testing.T) { t.Run(name, func(t *testing.T) { err := tc.example.ValidateBasic() assert.Equal(t, tc.zero, tc.example.Undefined()) - if !tc.valid { - require.Error(t, err) - return - } require.NoError(t, err) if !tc.before.Undefined() { diff --git a/x/feegrant/types/filtered_fee.go b/x/feegrant/types/filtered_fee.go index ae34a687ff68..6b2f825b3f1e 100644 --- a/x/feegrant/types/filtered_fee.go +++ b/x/feegrant/types/filtered_fee.go @@ -1,10 +1,9 @@ package types import ( + "github.com/gogo/protobuf/proto" "time" - proto "github.com/gogo/protobuf/proto" - "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" diff --git a/x/feegrant/types/grant_test.go b/x/feegrant/types/grant_test.go index 8b29d47ee079..911c0cd6860d 100644 --- a/x/feegrant/types/grant_test.go +++ b/x/feegrant/types/grant_test.go @@ -2,6 +2,7 @@ package types_test import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -18,6 +19,7 @@ func TestGrant(t *testing.T) { addr2, err := sdk.AccAddressFromBech32("cosmos1p9qh4ldfd6n0qehujsal4k7g0e37kel90rc4ts") require.NoError(t, err) atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555)) + zeroAtoms := sdk.NewCoins(sdk.NewInt64Coin("atom", 0)) goodGrant, err := types.NewFeeAllowanceGrant(addr2, addr, &types.BasicFeeAllowance{ SpendLimit: atom, @@ -49,6 +51,12 @@ func TestGrant(t *testing.T) { }) require.NoError(t, err) + zeroAllowance, err := types.NewFeeAllowanceGrant(addr, addr2, &types.BasicFeeAllowance{ + SpendLimit: zeroAtoms, + Expiration: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)), + }) + require.NoError(t, err) + cdc := app.AppCodec() // RegisterLegacyAminoCodec(cdc) @@ -62,15 +70,23 @@ func TestGrant(t *testing.T) { }, "no grantee": { grant: noGranteeGrant, + valid: false, }, "no granter": { grant: noGranterGrant, + valid: false, }, "self-grant": { grant: selfGrant, + valid: false, }, "bad allowance": { grant: badAllowanceGrant, + valid: false, + }, + "zero allowance": { + grant: zeroAllowance, + valid: false, }, } diff --git a/x/feegrant/types/msgs.go b/x/feegrant/types/msgs.go index 1f2be972d831..e346763f9200 100644 --- a/x/feegrant/types/msgs.go +++ b/x/feegrant/types/msgs.go @@ -13,12 +13,6 @@ var ( _ types.UnpackInterfacesMessage = &MsgGrantFeeAllowance{} ) -// feegrant message types -const ( - TypeMsgGrantFeeAllowance = "grant_fee_allowance" - TypeMsgRevokeFeeAllowance = "revoke_fee_allowance" -) - // NewMsgGrantFeeAllowance creates a new MsgGrantFeeAllowance. //nolint:interfacer func NewMsgGrantFeeAllowance(feeAllowance FeeAllowanceI, granter, grantee sdk.AccAddress) (*MsgGrantFeeAllowance, error) { @@ -58,6 +52,7 @@ func (msg MsgGrantFeeAllowance) ValidateBasic() error { return allowance.ValidateBasic() } +// GetSigners gets the granter account associated with an allowance func (msg MsgGrantFeeAllowance) GetSigners() []sdk.AccAddress { granter, err := sdk.AccAddressFromBech32(msg.Granter) if err != nil { @@ -82,6 +77,8 @@ func (msg MsgGrantFeeAllowance) UnpackInterfaces(unpacker types.AnyUnpacker) err return unpacker.UnpackAny(msg.Allowance, &allowance) } +// NewMsgRevokeFeeAllowance returns a message to revoke a fee allowance for a given +// granter and grantee //nolint:interfacer func NewMsgRevokeFeeAllowance(granter sdk.AccAddress, grantee sdk.AccAddress) MsgRevokeFeeAllowance { return MsgRevokeFeeAllowance{Granter: granter.String(), Grantee: grantee.String()} @@ -94,10 +91,15 @@ func (msg MsgRevokeFeeAllowance) ValidateBasic() error { if msg.Grantee == "" { return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "missing grantee address") } + if msg.Grantee == msg.Granter { + return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "addresses must be different") + } return nil } +// GetSigners gets the granter address associated with an Allowance +// to revoke. func (msg MsgRevokeFeeAllowance) GetSigners() []sdk.AccAddress { granter, err := sdk.AccAddressFromBech32(msg.Granter) if err != nil { diff --git a/x/feegrant/types/msgs_test.go b/x/feegrant/types/msgs_test.go new file mode 100644 index 000000000000..8bbf6ce3386c --- /dev/null +++ b/x/feegrant/types/msgs_test.go @@ -0,0 +1,69 @@ +package types_test + +import ( + "github.com/cosmos/cosmos-sdk/codec" + codectypes "github.com/cosmos/cosmos-sdk/codec/types" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/feegrant/types" + "github.com/stretchr/testify/require" + "testing" + "time" +) + +func TestMsgs(t *testing.T) { + addr, _ := sdk.AccAddressFromBech32("cosmos1aeuqja06474dfrj7uqsvukm6rael982kk89mqr") + addr2, _ := sdk.AccAddressFromBech32("cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl") + atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555)) + basic := &types.BasicFeeAllowance{ + SpendLimit: atom, + Expiration: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)), + } + cases := map[string]struct { + grantee sdk.AccAddress + granter sdk.AccAddress + grant *types.BasicFeeAllowance + valid bool + }{ + "valid":{ + grantee: addr, + granter: addr2, + grant: basic, + valid: true, + }, + "grantee == granter":{ + grantee: addr, + granter: addr, + grant: basic, + valid: false, + }, + } + + for _,tc := range cases { + msg, err := types.NewMsgGrantFeeAllowance(tc.grant, tc.granter, tc.grantee) + require.NoError(t, err) + msgRevoke := types.NewMsgRevokeFeeAllowance(tc.granter, tc.grantee) + valid := msg.ValidateBasic() + validRevoke := msgRevoke.ValidateBasic() + if tc.valid { + require.NoError(t, valid) + require.NoError(t, validRevoke) + + addrSlice := msg.GetSigners() + require.Equal(t, tc.granter.String(), addrSlice[0].String()) + + allowance, err := msg.GetFeeAllowanceI() + require.NoError(t, err) + require.Equal(t, tc.grant, allowance) + + addrSlice = msgRevoke.GetSigners() + require.Equal(t, tc.granter.String(), addrSlice[0].String()) + + cdc := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) + err = msg.UnpackInterfaces(cdc) + require.NoError(t, err) + } else { + require.Error(t, valid) + require.Error(t, validRevoke) + } + } +} diff --git a/x/feegrant/types/periodic_fee_test.go b/x/feegrant/types/periodic_fee_test.go index aa66828babac..3255c2ccc757 100644 --- a/x/feegrant/types/periodic_fee_test.go +++ b/x/feegrant/types/periodic_fee_test.go @@ -21,7 +21,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 1)) cases := map[string]struct { - allow types.PeriodicFeeAllowance + allowance types.PeriodicFeeAllowance // all other checks are ignored if valid=false fee sdk.Coins blockHeight int64 @@ -33,11 +33,11 @@ func TestPeriodicFeeValidAllow(t *testing.T) { periodReset types.ExpiresAt }{ "empty": { - allow: types.PeriodicFeeAllowance{}, + allowance: types.PeriodicFeeAllowance{}, valid: false, }, "only basic": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -46,7 +46,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { valid: false, }, "empty basic": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Period: types.BlockDuration(10), PeriodSpendLimit: smallAtom, PeriodReset: types.ExpiresAtHeight(70), @@ -58,7 +58,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { periodReset: types.ExpiresAtHeight(80), }, "mismatched currencies": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -69,7 +69,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { valid: false, }, "first time": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -87,7 +87,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { periodReset: types.ExpiresAtHeight(85), }, "same period": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -107,7 +107,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { periodReset: types.ExpiresAtHeight(80), }, "step one period": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -126,7 +126,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { periodReset: types.ExpiresAtHeight(80), // one step from last reset, not now }, "step limited by global allowance": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: smallAtom, Expiration: types.ExpiresAtHeight(100), @@ -145,7 +145,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { periodReset: types.ExpiresAtHeight(80), // one step from last reset, not now }, "expired": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -160,7 +160,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { remove: true, }, "over period limit": { - allow: types.PeriodicFeeAllowance{ + allowance: types.PeriodicFeeAllowance{ Basic: types.BasicFeeAllowance{ SpendLimit: atom, Expiration: types.ExpiresAtHeight(100), @@ -181,7 +181,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) { for name, stc := range cases { tc := stc // to make scopelint happy t.Run(name, func(t *testing.T) { - err := tc.allow.ValidateBasic() + err := tc.allowance.ValidateBasic() if !tc.valid { require.Error(t, err) return @@ -190,18 +190,18 @@ func TestPeriodicFeeValidAllow(t *testing.T) { ctx := app.BaseApp.NewContext(false, tmproto.Header{}).WithBlockHeight(tc.blockHeight) // now try to deduct - remove, err := tc.allow.Accept(ctx, tc.fee, []sdk.Msg{}) + removed, err := tc.allowance.Accept(ctx, tc.fee, []sdk.Msg{}) if !tc.accept { require.Error(t, err) return } require.NoError(t, err) - require.Equal(t, tc.remove, remove) - if !remove { - assert.Equal(t, tc.remains, tc.allow.Basic.SpendLimit) - assert.Equal(t, tc.remainsPeriod, tc.allow.PeriodCanSpend) - assert.Equal(t, tc.periodReset, tc.allow.PeriodReset) + require.Equal(t, tc.remove, removed) + if !removed { + assert.Equal(t, tc.remains, tc.allowance.Basic.SpendLimit) + assert.Equal(t, tc.remainsPeriod, tc.allowance.PeriodCanSpend) + assert.Equal(t, tc.periodReset, tc.allowance.PeriodReset) } }) } From 441a18c882463eb5d17feb907f4240fe5f79d94c Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Fri, 23 Apr 2021 15:50:43 -0700 Subject: [PATCH 2/4] fix imports --- x/feegrant/types/filtered_fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feegrant/types/filtered_fee.go b/x/feegrant/types/filtered_fee.go index 6b2f825b3f1e..7fdbd0f21c5d 100644 --- a/x/feegrant/types/filtered_fee.go +++ b/x/feegrant/types/filtered_fee.go @@ -1,12 +1,12 @@ package types import ( - "github.com/gogo/protobuf/proto" "time" "github.com/cosmos/cosmos-sdk/codec/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/gogo/protobuf/proto" ) // TODO: Revisit this once we have propoer gas fee framework. From 3e6b75191028c31c011088b11ee063e33f88d506 Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Mon, 26 Apr 2021 16:55:33 -0700 Subject: [PATCH 3/4] fix tests --- x/feegrant/types/grant_test.go | 89 +++++++++++++++------------------- x/feegrant/types/msgs_test.go | 83 ++++++++++++++++++++++++++----- 2 files changed, 109 insertions(+), 63 deletions(-) diff --git a/x/feegrant/types/grant_test.go b/x/feegrant/types/grant_test.go index 911c0cd6860d..bd6907faa9de 100644 --- a/x/feegrant/types/grant_test.go +++ b/x/feegrant/types/grant_test.go @@ -4,7 +4,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/cosmos/cosmos-sdk/simapp" @@ -20,72 +19,55 @@ func TestGrant(t *testing.T) { require.NoError(t, err) atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555)) zeroAtoms := sdk.NewCoins(sdk.NewInt64Coin("atom", 0)) - - goodGrant, err := types.NewFeeAllowanceGrant(addr2, addr, &types.BasicFeeAllowance{ - SpendLimit: atom, - Expiration: types.ExpiresAtHeight(100), - }) - require.NoError(t, err) - - noGranteeGrant, err := types.NewFeeAllowanceGrant(addr2, nil, &types.BasicFeeAllowance{ - SpendLimit: atom, - Expiration: types.ExpiresAtHeight(100), - }) - require.NoError(t, err) - - noGranterGrant, err := types.NewFeeAllowanceGrant(nil, addr, &types.BasicFeeAllowance{ - SpendLimit: atom, - Expiration: types.ExpiresAtHeight(100), - }) - require.NoError(t, err) - - selfGrant, err := types.NewFeeAllowanceGrant(addr2, addr2, &types.BasicFeeAllowance{ - SpendLimit: atom, - Expiration: types.ExpiresAtHeight(100), - }) - require.NoError(t, err) - - badAllowanceGrant, err := types.NewFeeAllowanceGrant(addr2, addr, &types.BasicFeeAllowance{ - SpendLimit: atom, - Expiration: types.ExpiresAtHeight(-1), - }) - require.NoError(t, err) - - zeroAllowance, err := types.NewFeeAllowanceGrant(addr, addr2, &types.BasicFeeAllowance{ - SpendLimit: zeroAtoms, - Expiration: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)), - }) - require.NoError(t, err) - cdc := app.AppCodec() - // RegisterLegacyAminoCodec(cdc) cases := map[string]struct { - grant types.FeeAllowanceGrant + granter sdk.AccAddress + grantee sdk.AccAddress + limit sdk.Coins + expires types.ExpiresAt valid bool }{ "good": { - grant: goodGrant, + granter: addr2, + grantee: addr, + limit: atom, + expires: types.ExpiresAtHeight(100), valid: true, }, "no grantee": { - grant: noGranteeGrant, + granter: addr2, + grantee: nil, + limit: atom, + expires: types.ExpiresAtHeight(100), valid: false, }, "no granter": { - grant: noGranterGrant, + granter: nil, + grantee: addr, + limit: atom, + expires: types.ExpiresAtHeight(100), valid: false, }, "self-grant": { - grant: selfGrant, + granter: addr2, + grantee: addr2, + limit: atom, + expires: types.ExpiresAtHeight(100), valid: false, }, - "bad allowance": { - grant: badAllowanceGrant, + "bad height": { + granter: addr2, + grantee: addr, + limit: atom, + expires: types.ExpiresAtHeight(-100), valid: false, }, "zero allowance": { - grant: zeroAllowance, + granter: addr2, + grantee: addr, + limit: zeroAtoms, + expires: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)), valid: false, }, } @@ -93,7 +75,13 @@ func TestGrant(t *testing.T) { for name, tc := range cases { tc := tc t.Run(name, func(t *testing.T) { - err := tc.grant.ValidateBasic() + grant, err := types.NewFeeAllowanceGrant(tc.granter, tc.grantee, &types.BasicFeeAllowance{ + SpendLimit: tc.limit, + Expiration: tc.expires, + }) + require.NoError(t, err) + err = grant.ValidateBasic() + if !tc.valid { require.Error(t, err) return @@ -101,7 +89,7 @@ func TestGrant(t *testing.T) { require.NoError(t, err) // if it is valid, let's try to serialize, deserialize, and make sure it matches - bz, err := cdc.MarshalBinaryBare(&tc.grant) + bz, err := cdc.MarshalBinaryBare(&grant) require.NoError(t, err) var loaded types.FeeAllowanceGrant err = cdc.UnmarshalBinaryBare(bz, &loaded) @@ -109,8 +97,7 @@ func TestGrant(t *testing.T) { err = loaded.ValidateBasic() require.NoError(t, err) - - assert.Equal(t, tc.grant, loaded) + require.Equal(t, grant, loaded) }) } } diff --git a/x/feegrant/types/msgs_test.go b/x/feegrant/types/msgs_test.go index 8bbf6ce3386c..57d103fc2e24 100644 --- a/x/feegrant/types/msgs_test.go +++ b/x/feegrant/types/msgs_test.go @@ -10,7 +10,8 @@ import ( "time" ) -func TestMsgs(t *testing.T) { +func TestMsgGrantFeeAllowance(t *testing.T) { + cdc := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) addr, _ := sdk.AccAddressFromBech32("cosmos1aeuqja06474dfrj7uqsvukm6rael982kk89mqr") addr2, _ := sdk.AccAddressFromBech32("cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl") atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555)) @@ -30,6 +31,18 @@ func TestMsgs(t *testing.T) { grant: basic, valid: true, }, + "no grantee": { + granter: addr2, + grantee: sdk.AccAddress{}, + grant: basic, + valid: false, + }, + "no granter": { + granter: sdk.AccAddress{}, + grantee: addr, + grant: basic, + valid: false, + }, "grantee == granter":{ grantee: addr, granter: addr, @@ -41,12 +54,10 @@ func TestMsgs(t *testing.T) { for _,tc := range cases { msg, err := types.NewMsgGrantFeeAllowance(tc.grant, tc.granter, tc.grantee) require.NoError(t, err) - msgRevoke := types.NewMsgRevokeFeeAllowance(tc.granter, tc.grantee) - valid := msg.ValidateBasic() - validRevoke := msgRevoke.ValidateBasic() + err = msg.ValidateBasic() + if tc.valid { - require.NoError(t, valid) - require.NoError(t, validRevoke) + require.NoError(t, err) addrSlice := msg.GetSigners() require.Equal(t, tc.granter.String(), addrSlice[0].String()) @@ -55,15 +66,63 @@ func TestMsgs(t *testing.T) { require.NoError(t, err) require.Equal(t, tc.grant, allowance) - addrSlice = msgRevoke.GetSigners() - require.Equal(t, tc.granter.String(), addrSlice[0].String()) - - cdc := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) err = msg.UnpackInterfaces(cdc) require.NoError(t, err) } else { - require.Error(t, valid) - require.Error(t, validRevoke) + require.Error(t, err) + } + } +} + +func TestMsgRevokeFeeAllowance(t *testing.T) { + addr, _ := sdk.AccAddressFromBech32("cosmos1aeuqja06474dfrj7uqsvukm6rael982kk89mqr") + addr2, _ := sdk.AccAddressFromBech32("cosmos1nph3cfzk6trsmfxkeu943nvach5qw4vwstnvkl") + atom := sdk.NewCoins(sdk.NewInt64Coin("atom", 555)) + basic := &types.BasicFeeAllowance{ + SpendLimit: atom, + Expiration: types.ExpiresAtTime(time.Now().Add(3 * time.Hour)), + } + cases := map[string]struct { + grantee sdk.AccAddress + granter sdk.AccAddress + grant *types.BasicFeeAllowance + valid bool + }{ + "valid":{ + grantee: addr, + granter: addr2, + grant: basic, + valid: true, + }, + "no grantee": { + granter: addr2, + grantee: sdk.AccAddress{}, + grant: basic, + valid: false, + }, + "no granter": { + granter: sdk.AccAddress{}, + grantee: addr, + grant: basic, + valid: false, + }, + "grantee == granter":{ + grantee: addr, + granter: addr, + grant: basic, + valid: false, + }, + } + + for _,tc := range cases { + msg := types.NewMsgRevokeFeeAllowance(tc.granter, tc.grantee) + err := msg.ValidateBasic() + if tc.valid { + require.NoError(t, err) + addrSlice := msg.GetSigners() + require.Equal(t, tc.granter.String(), addrSlice[0].String()) + } else { + require.Error(t, err) } } } From 6bde6b7d01281ea6bd723b4be137771c1025aaba Mon Sep 17 00:00:00 2001 From: technicallyty <48813565+tytech3@users.noreply.github.com> Date: Tue, 27 Apr 2021 10:57:19 -0700 Subject: [PATCH 4/4] rename test field --- x/feegrant/types/expiration_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/x/feegrant/types/expiration_test.go b/x/feegrant/types/expiration_test.go index b164359e755d..3465dc1b4bb3 100644 --- a/x/feegrant/types/expiration_test.go +++ b/x/feegrant/types/expiration_test.go @@ -14,28 +14,28 @@ func TestExpiresAt(t *testing.T) { now := time.Now() cases := map[string]struct { - example types.ExpiresAt + expires types.ExpiresAt zero bool before types.ExpiresAt after types.ExpiresAt }{ "basic": { - example: types.ExpiresAtHeight(100), + expires: types.ExpiresAtHeight(100), before: types.ExpiresAtHeight(50), after: types.ExpiresAtHeight(122), }, "zero": { - example: types.ExpiresAt{}, + expires: types.ExpiresAt{}, zero: true, before: types.ExpiresAtHeight(1), }, "match height": { - example: types.ExpiresAtHeight(1000), + expires: types.ExpiresAtHeight(1000), before: types.ExpiresAtHeight(999), after: types.ExpiresAtHeight(1000), }, "match time": { - example: types.ExpiresAtTime(now), + expires: types.ExpiresAtTime(now), before: types.ExpiresAtTime(now.Add(-1 * time.Second)), after: types.ExpiresAtTime(now.Add(1 * time.Second)), }, @@ -44,15 +44,15 @@ func TestExpiresAt(t *testing.T) { for name, stc := range cases { tc := stc // to make scopelint happy t.Run(name, func(t *testing.T) { - err := tc.example.ValidateBasic() - assert.Equal(t, tc.zero, tc.example.Undefined()) + err := tc.expires.ValidateBasic() + assert.Equal(t, tc.zero, tc.expires.Undefined()) require.NoError(t, err) if !tc.before.Undefined() { - assert.Equal(t, false, tc.example.IsExpired(tc.before.GetTime(), tc.before.GetHeight())) + assert.Equal(t, false, tc.expires.IsExpired(tc.before.GetTime(), tc.before.GetHeight())) } if !tc.after.Undefined() { - assert.Equal(t, true, tc.example.IsExpired(tc.after.GetTime(), tc.after.GetHeight())) + assert.Equal(t, true, tc.expires.IsExpired(tc.after.GetTime(), tc.after.GetHeight())) } }) }