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.

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

feat(x/feegrant): Add limits to grant pruning and enable message to aid manually #18047

Merged
merged 33 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
99f3513
feat(x/feegrant): Add limits to grant pruning and enable message
facundomedica Oct 10, 2023
1b98a8e
cleanup: remove getGrant
facundomedica Oct 10, 2023
620d699
cl++
facundomedica Oct 10, 2023
8f452bb
add event, autocli tx and test
facundomedica Oct 12, 2023
b6da60c
add params and authority
facundomedica Oct 12, 2023
ff1d5e1
fix autocli
facundomedica Oct 12, 2023
332c9d5
bump api
facundomedica Oct 12, 2023
572d6ee
go mod tidy
facundomedica Oct 12, 2023
2a52753
go mod tidy
facundomedica Oct 12, 2023
ac0f712
add queryserver
facundomedica Oct 12, 2023
7d4444c
fix legacy build
facundomedica Oct 12, 2023
0bc1152
Revert "fix legacy build"
facundomedica Oct 13, 2023
6ac1e42
Revert "add queryserver"
facundomedica Oct 13, 2023
c67e5ef
Revert "go mod tidy"
facundomedica Oct 13, 2023
be05fc1
Revert "go mod tidy"
facundomedica Oct 13, 2023
d9096c4
Revert "bump api"
facundomedica Oct 13, 2023
5cd5ab9
Revert "fix autocli"
facundomedica Oct 13, 2023
74066e4
re-do autocli
facundomedica Oct 13, 2023
f545db3
Revert "add params and authority"
facundomedica Oct 13, 2023
3159fa8
set default values
facundomedica Oct 13, 2023
74d1576
bump api
facundomedica Oct 13, 2023
5e77b11
bump api
facundomedica Oct 13, 2023
fca13c2
suggestions from julien, only missing benchmark
facundomedica Oct 13, 2023
1314de7
go mod tidy fix test
facundomedica Oct 16, 2023
ec72d0f
lint
facundomedica Oct 16, 2023
630034e
api bump
facundomedica Oct 16, 2023
d9e6c11
tests and comments
facundomedica Oct 16, 2023
2f10574
lint fix
facundomedica Oct 16, 2023
a6c1c6f
correct since comments in feegrant
facundomedica Oct 16, 2023
66230cc
oopsie
facundomedica Oct 16, 2023
2eba354
fix
facundomedica Oct 16, 2023
d84f1a1
ty julien
facundomedica Oct 16, 2023
530bc5d
conflicts
facundomedica Oct 16, 2023
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
958 changes: 920 additions & 38 deletions api/cosmos/feegrant/v1beta1/tx.pulsar.go

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions api/cosmos/feegrant/v1beta1/tx_grpc.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions proto/cosmos/feegrant/v1beta1/tx.proto
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ service Msg {
// RevokeAllowance revokes any fee allowance of granter's account that
// has been granted to the grantee.
rpc RevokeAllowance(MsgRevokeAllowance) returns (MsgRevokeAllowanceResponse);

// PruneAllowances prunes expired fee allowances.
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
rpc PruneAllowances(MsgPruneAllowances) returns (MsgPruneAllowancesResponse);
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
}

// MsgGrantAllowance adds permission for Grantee to spend up to Allowance
Expand Down Expand Up @@ -55,3 +58,14 @@ message MsgRevokeAllowance {

// MsgRevokeAllowanceResponse defines the Msg/RevokeAllowanceResponse response type.
message MsgRevokeAllowanceResponse {}

// MsgPruneAllowances prunes expired fee allowances.
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
message MsgPruneAllowances {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
option (cosmos.msg.v1.signer) = "pruner";

// pruner is the address of the user pruning expired allowances.
string pruner = 1 [(cosmos_proto.scalar) = "cosmos.AddressString"];
}

// MsgPruneAllowancesResponse defines the Msg/PruneAllowancesResponse response type.
message MsgPruneAllowancesResponse {}
1 change: 1 addition & 0 deletions x/feegrant/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* [#18047](https://github.com/cosmos/cosmos-sdk/pull/18047) Added a limit of 50 grants pruned per EndBlock and the method PruneAllowances that prunes 5 expired grants on every run.
* [#14649](https://github.com/cosmos/cosmos-sdk/pull/14649) The `x/feegrant` module is extracted to have a separate go.mod file which allows it to be a standalone module.

### API Breaking Changes
Expand Down
2 changes: 2 additions & 0 deletions x/feegrant/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ const (
EventTypeRevokeFeeGrant = "revoke_feegrant"
EventTypeSetFeeGrant = "set_feegrant"
EventTypeUpdateFeeGrant = "update_feegrant"
EventTypePruneFeeGrant = "prune_feegrant"

AttributeKeyGranter = "granter"
AttributeKeyGrantee = "grantee"
AttributeKeyPruner = "pruner"
)
28 changes: 10 additions & 18 deletions x/feegrant/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (k Keeper) GrantAllowance(ctx context.Context, granter, grantee sdk.AccAddr

// UpdateAllowance updates the existing grant.
func (k Keeper) UpdateAllowance(ctx context.Context, granter, grantee sdk.AccAddress, feeAllowance feegrant.FeeAllowanceI) error {
_, err := k.getGrant(ctx, granter, grantee)
_, err := k.GetAllowance(ctx, granter, grantee)
if err != nil {
return err
}
Expand Down Expand Up @@ -215,16 +215,6 @@ func (k Keeper) GetAllowance(ctx context.Context, granter, grantee sdk.AccAddres
return grant.GetGrant()
}

// getGrant returns entire grant between both accounts
func (k Keeper) getGrant(ctx context.Context, granter, grantee sdk.AccAddress) (*feegrant.Grant, error) {
feegrant, err := k.FeeAllowance.Get(ctx, collections.Join(grantee, granter))
if err != nil {
return nil, err
}

return &feegrant, nil
}

// IterateAllFeeAllowances iterates over all the grants in the store.
// Callback to get all data, returns true to stop, false to keep reading
// Calling this without pagination is very expensive and only designed for export genesis
Expand All @@ -245,12 +235,7 @@ func (k Keeper) IterateAllFeeAllowances(ctx context.Context, cb func(grant feegr

// UseGrantedFees will try to pay the given fee from the granter's account as requested by the grantee
func (k Keeper) UseGrantedFees(ctx context.Context, granter, grantee sdk.AccAddress, fee sdk.Coins, msgs []sdk.Msg) error {
f, err := k.getGrant(ctx, granter, grantee)
if err != nil {
return err
}

grant, err := f.GetGrant()
grant, err := k.GetAllowance(ctx, granter, grantee)
if err != nil {
return err
}
Expand Down Expand Up @@ -334,9 +319,10 @@ func (k Keeper) ExportGenesis(ctx context.Context) (*feegrant.GenesisState, erro
}

// RemoveExpiredAllowances iterates grantsByExpiryQueue and deletes the expired grants.
func (k Keeper) RemoveExpiredAllowances(ctx context.Context) error {
func (k Keeper) RemoveExpiredAllowances(ctx context.Context, limit int) error {
exp := sdk.UnwrapSDKContext(ctx).HeaderInfo().Time
rng := collections.NewPrefixUntilTripleRange[time.Time, sdk.AccAddress, sdk.AccAddress](exp)
count := 0

err := k.FeeAllowanceQueue.Walk(ctx, rng, func(key collections.Triple[time.Time, sdk.AccAddress, sdk.AccAddress], value bool) (stop bool, err error) {
grantee, granter := key.K2(), key.K3()
Expand All @@ -349,6 +335,12 @@ func (k Keeper) RemoveExpiredAllowances(ctx context.Context) error {
return true, err
}

// limit the amount of iterations to avoid taking too much time
count++
if count == limit {
return true, nil
}

return false, nil
})
if err != nil {
Expand Down
11 changes: 5 additions & 6 deletions x/feegrant/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,17 @@ func TestKeeperTestSuite(t *testing.T) {
}

func (suite *KeeperTestSuite) SetupTest() {
suite.addrs = simtestutil.CreateIncrementalAccounts(4)
suite.addrs = simtestutil.CreateIncrementalAccounts(7)
key := storetypes.NewKVStoreKey(feegrant.StoreKey)
testCtx := testutil.DefaultContextWithDB(suite.T(), key, storetypes.NewTransientStoreKey("transient_test"))
encCfg := moduletestutil.MakeTestEncodingConfig(module.AppModuleBasic{})

// setup gomock and initialize some globally expected executions
ctrl := gomock.NewController(suite.T())
suite.accountKeeper = feegranttestutil.NewMockAccountKeeper(ctrl)
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[0]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[0])).AnyTimes()
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[1]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[1])).AnyTimes()
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[2]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[2])).AnyTimes()
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[3]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[3])).AnyTimes()
for i := 0; i < len(suite.addrs); i++ {
suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[i]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[i])).AnyTimes()
}

ac := codecaddress.NewBech32Codec("cosmos")
suite.accountKeeper.EXPECT().AddressCodec().Return(ac).AnyTimes()
Expand Down Expand Up @@ -416,7 +415,7 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
}
err := suite.feegrantKeeper.GrantAllowance(suite.ctx, tc.granter, tc.grantee, tc.allowance)
suite.NoError(err)
err = suite.feegrantKeeper.RemoveExpiredAllowances(tc.ctx)
err = suite.feegrantKeeper.RemoveExpiredAllowances(tc.ctx, 5)
suite.NoError(err)

grant, err := suite.feegrantKeeper.GetAllowance(tc.ctx, tc.granter, tc.grantee)
Expand Down
19 changes: 19 additions & 0 deletions x/feegrant/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/x/feegrant"

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

Expand Down Expand Up @@ -84,3 +85,21 @@ func (k msgServer) RevokeAllowance(ctx context.Context, msg *feegrant.MsgRevokeA

return &feegrant.MsgRevokeAllowanceResponse{}, nil
}

// PruneAllowances removes expired allowances from the store.
func (k msgServer) PruneAllowances(ctx context.Context, req *feegrant.MsgPruneAllowances) (*feegrant.MsgPruneAllowancesResponse, error) {
err := k.RemoveExpiredAllowances(ctx, 5)
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
sdkCtx.EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypePruneFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyPruner, req.Pruner),
),
)

return &feegrant.MsgPruneAllowancesResponse{}, nil
}
51 changes: 51 additions & 0 deletions x/feegrant/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ import (

"github.com/golang/mock/gomock"

"cosmossdk.io/collections"
"cosmossdk.io/core/header"
"cosmossdk.io/x/feegrant"

codecaddress "github.com/cosmos/cosmos-sdk/codec/address"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

Expand Down Expand Up @@ -286,3 +288,52 @@ func (suite *KeeperTestSuite) TestRevokeAllowance() {
})
}
}

func (suite *KeeperTestSuite) TestPruneAllowances() {
ctx := suite.ctx.WithHeaderInfo(header.Info{Time: time.Now()})
oneYear := ctx.HeaderInfo().Time.AddDate(1, 0, 0)

// We create 6 allowances, all expiring in one year
for i := 0; i < 6; i++ {
any, err := codectypes.NewAnyWithValue(&feegrant.BasicAllowance{
SpendLimit: suite.atom,
Expiration: &oneYear,
})
suite.Require().NoError(err)
req := &feegrant.MsgGrantAllowance{
Granter: suite.encodedAddrs[0],
Grantee: suite.encodedAddrs[i+1],
Allowance: any,
}

_, err = suite.msgSrvr.GrantAllowance(ctx, req)
suite.Require().NoError(err)

}

// we have 6 allowances
count := 0
suite.feegrantKeeper.FeeAllowance.Walk(ctx, nil, func(key collections.Pair[types.AccAddress, types.AccAddress], value feegrant.Grant) (stop bool, err error) {
count++
return false, nil
})
suite.Require().Equal(6, count)

// after a year and one day passes, they are all expired
oneYearAndADay := ctx.HeaderInfo().Time.AddDate(1, 0, 1)
ctx = suite.ctx.WithHeaderInfo(header.Info{Time: oneYearAndADay})

// we prune them, but currently only 5 will be pruned
_, err := suite.msgSrvr.PruneAllowances(ctx, &feegrant.MsgPruneAllowances{})
suite.Require().NoError(err)

// we have 1 allowance left
count = 0
suite.feegrantKeeper.FeeAllowance.Walk(ctx, nil, func(key collections.Pair[types.AccAddress, types.AccAddress], value feegrant.Grant) (stop bool, err error) {
count++

return false, nil
})
suite.Require().Equal(1, count)

}
11 changes: 4 additions & 7 deletions x/feegrant/module/abci.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package module

import (
"cosmossdk.io/x/feegrant/keeper"
"context"

sdk "github.com/cosmos/cosmos-sdk/types"
"cosmossdk.io/x/feegrant/keeper"
)

func EndBlocker(ctx sdk.Context, k keeper.Keeper) {
err := k.RemoveExpiredAllowances(ctx)
if err != nil {
panic(err)
}
func EndBlocker(ctx context.Context, k keeper.Keeper) error {
return k.RemoveExpiredAllowances(ctx, 50)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
}
7 changes: 7 additions & 0 deletions x/feegrant/module/autocli.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ You can find the fee-grant of a granter and grantee.`),
{ProtoField: "grantee"},
},
},
{
RpcMethod: "PruneAllowances",
Use: "prune",
Short: "Prune expired allowances",
Long: "Prune a limited amount of expired allowances in order to reduce the size of the store when the number of expired allowances is large.",
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
Example: fmt.Sprintf(`$ %s tx feegrant prune`, version.AppName),
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
},
facundomedica marked this conversation as resolved.
Show resolved Hide resolved
},
EnhanceCustomCommand: true,
},
Expand Down
5 changes: 1 addition & 4 deletions x/feegrant/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
sdkclient "github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
cdctypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
simtypes "github.com/cosmos/cosmos-sdk/types/simulation"
)
Expand Down Expand Up @@ -162,9 +161,7 @@ func (AppModule) ConsensusVersion() uint64 { return 2 }
// EndBlock returns the end blocker for the feegrant module. It returns no validator
// updates.
func (am AppModule) EndBlock(ctx context.Context) error {
c := sdk.UnwrapSDKContext(ctx)
EndBlocker(c, am.keeper)
return nil
return EndBlocker(ctx, am.keeper)
}

func init() {
Expand Down
Loading
Loading