Skip to content

Commit

Permalink
x/feegrant audit: clean up / add test coverage to types package (#9193)
Browse files Browse the repository at this point in the history 
* Squashed commit of the following:

commit 58dc50051226a9eeb8d0ebea5bb0908fe5b9637f
Author: technicallyty <[email protected]>
Date:   Fri Apr 23 15:09:27 2021 -0700

    remove comments

commit a84107e1b3eaa31324cb0f4f097b49f02af79c69
Author: technicallyty <[email protected]>
Date:   Fri Apr 23 15:02:41 2021 -0700

    add tests for msgs.go

commit 2ad16869237e9631b402c93cde650c3fc554daf2
Author: technicallyty <[email protected]>
Date:   Fri Apr 23 13:20:21 2021 -0700

    specify test name

commit b7121277c9be586a7c80d010ec401e50b510e02a
Merge: c0c134d971 3c65c3d
Author: technicallyty <[email protected]>
Date:   Fri Apr 23 12:54:55 2021 -0700

    Merge branch 'master' into ty-9115-types_tests

commit c0c134d97107194dc4f9d3c501a15d023ae083e5
Author: technicallyty <[email protected]>
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 <[email protected]>
Date:   Thu Apr 22 09:57:31 2021 -0700

    remove unecessary constant

commit 9db59a82a7337cf5a7a3569c6900a44a6c81e8b4
Merge: a3e75ceb8a e28271b
Author: technicallyty <[email protected]>
Date:   Thu Apr 22 09:19:20 2021 -0700

    Merge branch 'master' into ty-9115-types_tests

commit a3e75ceb8a510ad9db43dd96073c43b7a8b062b0
Merge: 4d3dafab85 bffcae5
Author: technicallyty <[email protected]>
Date:   Wed Apr 21 12:04:39 2021 -0700

    Merge branch 'master' into ty-9115-types_tests

commit 4d3dafab85d85526a7c94b045289605289ee6b0d
Author: technicallyty <[email protected]>
Date:   Wed Apr 21 12:03:41 2021 -0700

    cleanup id's / add test case

commit e8f6924931ba95e592bfc3057ba167700458da41
Author: technicallyty <[email protected]>
Date:   Wed Apr 21 10:55:29 2021 -0700

    add test for 0 allowance / remove unused field on exp test

commit 516b6ef89a4582ad681cc6c5c101cf50a9a54fb5
Author: technicallyty <[email protected]>
Date:   Tue Apr 20 15:22:48 2021 -0700

    make names more clear, remove unused field

* fix imports

* fix tests

* rename test field

Co-authored-by: technicallyty <[email protected]>
Co-authored-by: Marie Gauthier <[email protected]>
Co-authored-by: MD Aleem <[email protected]>
  • Loading branch information
@technicallyty @blushi @aleem1314
4 people authored Apr 28, 2021
1 parent f45838f commit 0cbed20
Show file tree
Hide file tree
Showing 8 changed files with 249 additions and 131 deletions.
41 changes: 22 additions & 19 deletions x/feegrant/client/testutil/suite.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,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
Expand All @@ -659,10 +659,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()),
Expand All @@ -673,11 +673,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),
Expand Down Expand Up @@ -753,22 +753,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) {
Expand All @@ -784,7 +792,8 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() {
cmd := cli.NewCmdFeeGrant()
return clitestutil.ExecTestCLICmd(clientCtx, cmd, args)
},
false, &sdk.TxResponse{}, 7,
&sdk.TxResponse{},
7,
},
}

Expand All @@ -793,16 +802,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())
})
}
}
Expand Down
46 changes: 16 additions & 30 deletions x/feegrant/types/basic_fee_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,94 +22,84 @@ 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,
remove: false,
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,
Expand All @@ -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)
}
})
}
Expand Down
27 changes: 9 additions & 18 deletions x/feegrant/types/expiration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,28 @@ func TestExpiresAt(t *testing.T) {
now := time.Now()

cases := map[string]struct {
example types.ExpiresAt
valid bool
expires types.ExpiresAt
zero bool
before types.ExpiresAt
after types.ExpiresAt
}{
"basic": {
example: types.ExpiresAtHeight(100),
valid: true,
expires: types.ExpiresAtHeight(100),
before: types.ExpiresAtHeight(50),
after: types.ExpiresAtHeight(122),
},
"zero": {
example: types.ExpiresAt{},
expires: types.ExpiresAt{},
zero: true,
valid: true,
before: types.ExpiresAtHeight(1),
},
"match height": {
example: types.ExpiresAtHeight(1000),
valid: true,
expires: types.ExpiresAtHeight(1000),
before: types.ExpiresAtHeight(999),
after: types.ExpiresAtHeight(1000),
},
"match time": {
example: types.ExpiresAtTime(now),
valid: true,
expires: types.ExpiresAtTime(now),
before: types.ExpiresAtTime(now.Add(-1 * time.Second)),
after: types.ExpiresAtTime(now.Add(1 * time.Second)),
},
Expand All @@ -49,19 +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())
if !tc.valid {
require.Error(t, err)
return
}
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()))
}
})
}
Expand Down
3 changes: 1 addition & 2 deletions x/feegrant/types/filtered_fee.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ package types
import (
"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"
"github.com/gogo/protobuf/proto"
)

// TODO: Revisit this once we have propoer gas fee framework.
Expand Down
Loading

0 comments on commit 0cbed20

Please sign in to comment.