Skip to content

Commit

Permalink
feat: adds pruning for feegrant (#10830)
Browse files Browse the repository at this point in the history
Closes: #10685 



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
atheeshp authored Feb 8, 2022
1 parent ebddeee commit 786afb3
Show file tree
Hide file tree
Showing 19 changed files with 569 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ 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/feegrant) [\#10830](https://github.com/cosmos/cosmos-sdk/pull/10830) Expired allowances will be pruned from state.

### Deprecated

Expand Down
6 changes: 6 additions & 0 deletions x/feegrant/basic_fee.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package feegrant

import (
time "time"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
)
Expand Down Expand Up @@ -52,3 +54,7 @@ func (a BasicAllowance) ValidateBasic() error {

return nil
}

func (a BasicAllowance) ExpiresAt() (*time.Time, error) {
return a.Expiration, nil
}
1 change: 1 addition & 0 deletions x/feegrant/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const (
EventTypeUseFeeGrant = "use_feegrant"
EventTypeRevokeFeeGrant = "revoke_feegrant"
EventTypeSetFeeGrant = "set_feegrant"
EventTypeUpdateFeeGrant = "update_feegrant"

AttributeKeyGranter = "granter"
AttributeKeyGrantee = "grantee"
Expand Down
5 changes: 5 additions & 0 deletions x/feegrant/fees.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package feegrant

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
)

Expand All @@ -22,4 +24,7 @@ type FeeAllowanceI interface {
// ValidateBasic should evaluate this FeeAllowance for internal consistency.
// Don't allow negative amounts, or negative periods for example.
ValidateBasic() error

// ExpiresAt returns the expiry time of the allowance.
ExpiresAt() (*time.Time, error)
}
10 changes: 10 additions & 0 deletions x/feegrant/filtered_fee.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package feegrant

import (
"time"

"github.com/gogo/protobuf/proto"

"github.com/cosmos/cosmos-sdk/codec/types"
Expand Down Expand Up @@ -120,3 +122,11 @@ func (a *AllowedMsgAllowance) ValidateBasic() error {

return allowance.ValidateBasic()
}

func (a *AllowedMsgAllowance) ExpiresAt() (*time.Time, error) {
allowance, err := a.GetAllowance()
if err != nil {
return nil, err
}
return allowance.ExpiresAt()
}
6 changes: 4 additions & 2 deletions x/feegrant/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,12 @@ func (q Keeper) AllowancesByGranter(c context.Context, req *feegrant.QueryAllowa
var grants []*feegrant.Grant

store := ctx.KVStore(q.storeKey)
pageRes, err := query.Paginate(store, req.Pagination, func(key []byte, value []byte) error {
prefixStore := prefix.NewStore(store, feegrant.FeeAllowanceKeyPrefix)
pageRes, err := query.Paginate(prefixStore, req.Pagination, func(key []byte, value []byte) error {
var grant feegrant.Grant

granter, _ := feegrant.ParseAddressesFromFeeAllowanceKey(key)
// ParseAddressesFromFeeAllowanceKey expects the full key including the prefix.
granter, _ := feegrant.ParseAddressesFromFeeAllowanceKey(append(feegrant.FeeAllowanceKeyPrefix, key...))
if !granter.Equals(granterAddr) {
return nil
}
Expand Down
102 changes: 101 additions & 1 deletion x/feegrant/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper

import (
"fmt"
"time"

storetypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/tendermint/tendermint/libs/log"
Expand Down Expand Up @@ -49,6 +50,45 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress,

store := ctx.KVStore(k.storeKey)
key := feegrant.FeeAllowanceKey(granter, grantee)

var oldExp *time.Time
existingGrant, err := k.getGrant(ctx, grantee, granter)
if err != nil && existingGrant != nil && existingGrant.GetAllowance() != nil {
grantInfo, err := existingGrant.GetGrant()
if err != nil {
return err
}

oldExp, err = grantInfo.ExpiresAt()
if err != nil {
return err
}
}

newExp, err := feeAllowance.ExpiresAt()
if err != nil {
return err
} else if newExp != nil && newExp.Before(ctx.BlockTime()) {
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "expiration is before current block time")
} else if oldExp == nil && newExp != nil {
// when old oldExp is nil there won't be any key added before to queue.
// add the new key to queue directly.
k.addToFeeAllowanceQueue(ctx, key[1:], newExp)
} else if oldExp != nil && newExp == nil {
// when newExp is nil no need of adding the key to the pruning queue
// remove the old key from queue.
k.removeFromGrantQueue(ctx, oldExp, key[1:])
} else if oldExp != nil && newExp != nil && !oldExp.Equal(*newExp) {
// `key` formed here with the prefix of `FeeAllowanceKeyPrefix` (which is `0x00`)
// remove the 1st byte and reuse the remaining key as it is.

// remove the old key from queue.
k.removeFromGrantQueue(ctx, oldExp, key[1:])

// add the new key to queue.
k.addToFeeAllowanceQueue(ctx, key[1:], newExp)
}

grant, err := feegrant.NewGrant(granter, grantee, feeAllowance)
if err != nil {
return err
Expand All @@ -72,6 +112,39 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress,
return nil
}

// UpdateAllowance updates the existing grant.
func (k Keeper) UpdateAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, feeAllowance feegrant.FeeAllowanceI) error {
store := ctx.KVStore(k.storeKey)
key := feegrant.FeeAllowanceKey(granter, grantee)

_, err := k.getGrant(ctx, granter, grantee)
if err != nil {
return err
}

grant, err := feegrant.NewGrant(granter, grantee, feeAllowance)
if err != nil {
return err
}

bz, err := k.cdc.Marshal(&grant)
if err != nil {
return err
}

store.Set(key, bz)

ctx.EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypeUpdateFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyGranter, grant.Granter),
sdk.NewAttribute(feegrant.AttributeKeyGrantee, grant.Grantee),
),
)

return nil
}

// revokeAllowance removes an existing grant
func (k Keeper) revokeAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress) error {
_, err := k.getGrant(ctx, granter, grantee)
Expand Down Expand Up @@ -177,7 +250,7 @@ func (k Keeper) UseGrantedFees(ctx sdk.Context, granter, grantee sdk.AccAddress,
emitUseGrantEvent(ctx, granter.String(), grantee.String())

// if fee allowance is accepted, store the updated state of the allowance
return k.GrantAllowance(ctx, granter, grantee, grant)
return k.UpdateAllowance(ctx, granter, grantee, grant)
}

func emitUseGrantEvent(ctx sdk.Context, granter, grantee string) {
Expand Down Expand Up @@ -228,3 +301,30 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (*feegrant.GenesisState, error) {
Allowances: grants,
}, err
}

func (k Keeper) removeFromGrantQueue(ctx sdk.Context, exp *time.Time, allowanceKey []byte) {
key := feegrant.FeeAllowancePrefixQueue(exp, allowanceKey)
store := ctx.KVStore(k.storeKey)
store.Delete(key)
}

func (k Keeper) addToFeeAllowanceQueue(ctx sdk.Context, grantKey []byte, exp *time.Time) {
store := ctx.KVStore(k.storeKey)
store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), []byte{})
}

// RemoveExpiredAllowances iterates grantsByExpiryQueue and deletes the expired grants.
func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context) {
exp := ctx.BlockTime()
store := ctx.KVStore(k.storeKey)
iterator := store.Iterator(feegrant.FeeAllowanceQueueKeyPrefix, sdk.InclusiveEndBytes(feegrant.AllowanceByExpTimeKey(&exp)))
defer iterator.Close()

for ; iterator.Valid(); iterator.Next() {
store.Delete(iterator.Key())
expLen := len(sdk.FormatTimeBytes(ctx.BlockTime()))

// extract the fee allowance key by removing the allowance queue prefix length, expiration length from key.
store.Delete(append(feegrant.FeeAllowanceKeyPrefix, iterator.Key()[1+expLen:]...))
}
}
94 changes: 89 additions & 5 deletions x/feegrant/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,18 @@ func (suite *KeeperTestSuite) TestUseGrantedFee() {
})
}

expired := &feegrant.BasicAllowance{
basicAllowance := &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &blockTime,
}
// creating expired feegrant
ctx := suite.sdkCtx.WithBlockTime(oneYear)
err := suite.keeper.GrantAllowance(ctx, suite.addrs[0], suite.addrs[2], expired)

// create basic fee allowance
err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[0], suite.addrs[2], basicAllowance)
suite.Require().NoError(err)

// waiting for future blocks, allowance to be pruned.
ctx := suite.sdkCtx.WithBlockTime(oneYear)

// expect error: feegrant expired
err = suite.keeper.UseGrantedFees(ctx, suite.addrs[0], suite.addrs[2], eth, []sdk.Msg{})
suite.Error(err)
Expand All @@ -232,7 +235,6 @@ func (suite *KeeperTestSuite) TestUseGrantedFee() {
_, err = suite.keeper.GetAllowance(ctx, suite.addrs[0], suite.addrs[2])
suite.Error(err)
suite.Contains(err.Error(), "fee-grant not found")

}

func (suite *KeeperTestSuite) TestIterateGrants() {
Expand All @@ -257,5 +259,87 @@ func (suite *KeeperTestSuite) TestIterateGrants() {
suite.Require().Contains([]string{suite.addrs[0].String(), suite.addrs[2].String()}, grant.Granter)
return true
})
}

func (suite *KeeperTestSuite) TestPruneGrants() {
eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 123))
now := suite.sdkCtx.BlockTime()
oneYearExpiry := now.AddDate(1, 0, 0)

testCases := []struct {
name string
ctx sdk.Context
granter sdk.AccAddress
grantee sdk.AccAddress
allowance feegrant.FeeAllowanceI
expErrMsg string
}{
{
name: "grant not pruned from state",
ctx: suite.sdkCtx,
granter: suite.addrs[0],
grantee: suite.addrs[1],
allowance: &feegrant.BasicAllowance{
SpendLimit: suite.atom,
Expiration: &now,
},
},
{
name: "grant pruned from state after a block: error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 1)),
granter: suite.addrs[2],
grantee: suite.addrs[1],
expErrMsg: "not found",
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &now,
},
},
{
name: "grant not pruned from state after a day: no error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 1)),
granter: suite.addrs[1],
grantee: suite.addrs[0],
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &oneYearExpiry,
},
},
{
name: "grant pruned from state after a year: error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(1, 0, 1)),
granter: suite.addrs[1],
grantee: suite.addrs[2],
expErrMsg: "not found",
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &oneYearExpiry,
},
},
{
name: "no expiry: no error",
ctx: suite.sdkCtx.WithBlockTime(now.AddDate(1, 0, 0)),
granter: suite.addrs[1],
grantee: suite.addrs[2],
allowance: &feegrant.BasicAllowance{
SpendLimit: eth,
Expiration: &oneYearExpiry,
},
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.keeper.GrantAllowance(suite.sdkCtx, tc.granter, tc.grantee, tc.allowance)
suite.app.FeeGrantKeeper.RemoveExpiredAllowances(tc.ctx)
grant, err := suite.keeper.GetAllowance(tc.ctx, tc.granter, tc.grantee)
if tc.expErrMsg != "" {
suite.Error(err)
suite.Contains(err.Error(), tc.expErrMsg)
} else {
suite.NotNil(grant)
}
})
}
}
21 changes: 21 additions & 0 deletions x/feegrant/keeper/migrations.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
v046 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v046"
)

// Migrator is a struct for handling in-place store migrations.
type Migrator struct {
keeper Keeper
}

// NewMigrator returns a new Migrator.
func NewMigrator(keeper Keeper) Migrator {
return Migrator{keeper: keeper}
}

// Migrate1to2 migrates from version 1 to 2.
func (m Migrator) Migrate1to2(ctx sdk.Context) error {
return v046.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc)
}
Loading

0 comments on commit 786afb3

Please sign in to comment.