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.

Sign up for GitHub

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

Jump to bottom

fix: (x/feegrant) expiration when overwriting an existing grant #11845

Merged
merged 13 commits into from
May 6, 2022
Merged

fix: (x/feegrant) expiration when overwriting an existing grant #11845

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8bd37f7
fix: expiration when overwriting an existing grant
atheeshp May 2, 2022
be1851c
add tests
atheeshp May 2, 2022
e810281
Merge branch 'main' of github.com:cosmos/cosmos-sdk into ap/fix-feegr…
atheeshp May 2, 2022
a89e9c4
Merge branch 'main' into ap/fix-feegrant-exp
atheeshp May 2, 2022
b8859bc
remove string match
atheeshp May 4, 2022
0cc7c32
Merge branch 'main' of github.com:cosmos/cosmos-sdk into ap/fix-feegr…
atheeshp May 4, 2022
2f9d227
Merge branch 'ap/fix-feegrant-exp' of github.com:cosmos/cosmos-sdk in…
atheeshp May 4, 2022
9d15d23
Merge branch 'main' into ap/fix-feegrant-exp
alexanderbez May 4, 2022
86ff4f6
try fix errors Is
amaury1093 May 5, 2022
664403c
Merge branch 'main' of github.com:cosmos/cosmos-sdk into ap/fix-feegr…
atheeshp May 5, 2022
9902bf5
Merge branch 'main' into ap/fix-feegrant-exp
alexanderbez May 5, 2022
ceb0641
fix tests
atheeshp May 6, 2022
0cd0f2a
Merge branch 'ap/fix-feegrant-exp' of github.com:cosmos/cosmos-sdk in…
atheeshp May 6, 2022
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
9 changes: 7 additions & 2 deletions x/feegrant/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"fmt"
"strings"
"time"

storetypes "github.com/cosmos/cosmos-sdk/store/types"
Expand Down Expand Up @@ -52,8 +53,12 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress,
key := feegrant.FeeAllowanceKey(granter, grantee)

var oldExp *time.Time
existingGrant, err := k.getGrant(ctx, grantee, granter)
if err != nil && existingGrant != nil && existingGrant.GetAllowance() != nil {
existingGrant, err := k.getGrant(ctx, granter, grantee)
if err != nil && !strings.Contains(err.Error(), "fee-grant not found") {
atheeshp marked this conversation as resolved.
Show resolved Hide resolved
return err
}
atheeshp marked this conversation as resolved.
Show resolved Hide resolved

if existingGrant != nil && existingGrant.GetAllowance() != nil {
grantInfo, err := existingGrant.GetGrant()
if err != nil {
return err
Expand Down
114 changes: 111 additions & 3 deletions x/feegrant/keeper/keeper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
// some helpers
eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 123))
exp := suite.sdkCtx.BlockTime().AddDate(1, 0, 0)
exp2 := suite.sdkCtx.BlockTime().AddDate(2, 0, 0)
basic := &feegrant.BasicAllowance{
SpendLimit: suite.atom,
Expiration: &exp,
Expand All @@ -56,6 +57,11 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
Expiration: &exp,
}

basic3 := &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &exp2,
}

// let's set up some initial state here
err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[0], suite.addrs[1], basic)
suite.Require().NoError(err)
Expand Down Expand Up @@ -85,7 +91,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
err = suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[0], suite.addrs[2], basic)
suite.Require().NoError(err)

err = suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[1], suite.addrs[2], basic2)
err = suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[1], suite.addrs[2], basic3)
suite.Require().NoError(err)

// end state:
Expand Down Expand Up @@ -115,7 +121,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
"addr modified": {
granter: suite.addrs[1],
grantee: suite.addrs[2],
allowance: basic2,
allowance: basic3,
},
}

Expand Down Expand Up @@ -265,6 +271,7 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 123))
now := suite.sdkCtx.BlockTime()
oneYearExpiry := now.AddDate(1, 0, 0)
oneDay := now.AddDate(0, 0, 1)

testCases := []struct {
name string
Expand All @@ -273,6 +280,8 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
grantee sdk.AccAddress
allowance feegrant.FeeAllowanceI
expErrMsg string
preRun func()
postRun func()
}{
{
name: "grant not pruned from state",
Expand Down Expand Up @@ -326,12 +335,108 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
Expiration: &oneYearExpiry,
},
},
{
name: "grant created with a day expiry & overwritten with no expiry shouldn't be pruned: no error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
granter: suite.addrs[2],
grantee: suite.addrs[1],
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
},
preRun: func() {
// create a grant with a day expiry.
allowance := &feegrant.BasicAllowance{
SpendLimit: suite.atom,
Expiration: &oneDay,
}
err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
suite.NoError(err)
},
postRun: func() {
_, err := suite.msgSrvr.RevokeAllowance(suite.sdkCtx, &feegrant.MsgRevokeAllowance{
Granter: suite.addrs[2].String(),
Grantee: suite.addrs[1].String(),
})
suite.NoError(err)
},
},
{
name: "grant created with a day expiry & overwritten with a year expiry shouldn't be pruned: no error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
granter: suite.addrs[2],
grantee: suite.addrs[1],
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &oneYearExpiry,
},
preRun: func() {
// create a grant with a day expiry.
allowance := &feegrant.BasicAllowance{
SpendLimit: suite.atom,
Expiration: &oneDay,
}
err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
suite.NoError(err)
},
postRun: func() {
_, err := suite.msgSrvr.RevokeAllowance(suite.sdkCtx, &feegrant.MsgRevokeAllowance{
Granter: suite.addrs[2].String(),
Grantee: suite.addrs[1].String(),
})
suite.NoError(err)
},
},
{
name: "grant created with a year expiry & overwritten with a day expiry should be pruned after a day: error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
granter: suite.addrs[2],
grantee: suite.addrs[1],
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &oneDay,
},
preRun: func() {
// create a grant with a year expiry.
allowance := &feegrant.BasicAllowance{
SpendLimit: suite.atom,
Expiration: &oneYearExpiry,
}
err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
suite.NoError(err)
},
postRun: func() {},
expErrMsg: "not found",
},
{
name: "grant created with no expiry & overwritten with a day expiry should be pruned after a day: error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
granter: suite.addrs[2],
grantee: suite.addrs[1],
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &oneDay,
},
preRun: func() {
// create a grant with no expiry.
allowance := &feegrant.BasicAllowance{
SpendLimit: suite.atom,
}
err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
suite.NoError(err)
},
postRun: func() {},
expErrMsg: "not found",
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.keeper.GrantAllowance(suite.sdkCtx, tc.granter, tc.grantee, tc.allowance)
if tc.preRun != nil {
tc.preRun()
}
err := suite.keeper.GrantAllowance(suite.sdkCtx, tc.granter, tc.grantee, tc.allowance)
suite.NoError(err)
suite.app.FeeGrantKeeper.RemoveExpiredAllowances(tc.ctx)
grant, err := suite.keeper.GetAllowance(tc.ctx, tc.granter, tc.grantee)
if tc.expErrMsg != "" {
Expand All @@ -340,6 +445,9 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
} else {
suite.NotNil(grant)
}
if tc.postRun != nil {
tc.postRun()
}
})
}
}