From 152e37dcb2432fdf4d87cf57a11c17db4677fa3e Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 14 Dec 2021 17:50:22 +0530 Subject: [PATCH 01/26] WIP feegrant pruning --- x/feegrant/keeper/keeper.go | 12 ++++++++++++ x/feegrant/key.go | 14 +++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 5d4d79fabc4..197603b386a 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -49,6 +49,11 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, store := ctx.KVStore(k.storeKey) key := feegrant.FeeAllowanceKey(granter, grantee) + + existedGrant, err := k.getGrant(ctx, grantee, granter) + if err != nil && existedGrant.GetAllowance() != nil { + // k.removeFromGrantKey(ctx, feegrant.FeeAllowancePrefixQueue(feeAllowance)) + } grant, err := feegrant.NewGrant(granter, grantee, feeAllowance) if err != nil { return err @@ -228,3 +233,10 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (*feegrant.GenesisState, error) { Allowances: grants, }, err } + +func (k Keeper) removeFromGrantKey(ctx sdk.Context, key []byte) error { + store := ctx.KVStore(k.storeKey) + store.Delete(key) + + return nil +} diff --git a/x/feegrant/key.go b/x/feegrant/key.go index cfc885d9a7a..5e8b916589a 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -1,6 +1,8 @@ package feegrant import ( + time "time" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/types/address" ) @@ -21,7 +23,8 @@ const ( var ( // FeeAllowanceKeyPrefix is the set of the kvstore for fee allowance data - FeeAllowanceKeyPrefix = []byte{0x00} + FeeAllowanceKeyPrefix = []byte{0x00} + FeeAllowanceQueuePrefix = []byte{0x01} ) // FeeAllowanceKey is the canonical key to store a grant from granter to grantee @@ -34,3 +37,12 @@ func FeeAllowanceKey(granter sdk.AccAddress, grantee sdk.AccAddress) []byte { func FeeAllowancePrefixByGrantee(grantee sdk.AccAddress) []byte { return append(FeeAllowanceKeyPrefix, address.MustLengthPrefix(grantee.Bytes())...) } + +func FeeAllowancePrefixQueue(exp time.Time, allowanceKey []byte) []byte { + allowanceByExpTimeKey := AllowanceByExpTimeKey(exp) + return append(allowanceByExpTimeKey, allowanceKey[1:]...) +} + +func AllowanceByExpTimeKey(exp time.Time) []byte { + return append(FeeAllowanceQueuePrefix, sdk.FormatTimeBytes(exp)...) +} From b631f0c57d0baa714de272eda2555d65dff0ca0a Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 16 Dec 2021 17:46:02 +0530 Subject: [PATCH 02/26] add queue for feegrant keys with expiry time --- x/feegrant/basic_fee.go | 6 ++++++ x/feegrant/fees.go | 5 +++++ x/feegrant/filtered_fee.go | 10 ++++++++++ x/feegrant/keeper/keeper.go | 29 ++++++++++++++++++++++++----- x/feegrant/key.go | 4 ++-- x/feegrant/periodic_fee.go | 4 ++++ 6 files changed, 51 insertions(+), 7 deletions(-) diff --git a/x/feegrant/basic_fee.go b/x/feegrant/basic_fee.go index 85ba8ab2564..6c0e809d997 100644 --- a/x/feegrant/basic_fee.go +++ b/x/feegrant/basic_fee.go @@ -1,6 +1,8 @@ package feegrant import ( + time "time" + sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) @@ -52,3 +54,7 @@ func (a BasicAllowance) ValidateBasic() error { return nil } + +func (a BasicAllowance) ExpTime() (*time.Time, error) { + return a.Expiration, nil +} diff --git a/x/feegrant/fees.go b/x/feegrant/fees.go index bd8c682608f..31f67d03e02 100644 --- a/x/feegrant/fees.go +++ b/x/feegrant/fees.go @@ -1,6 +1,8 @@ package feegrant import ( + "time" + sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -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 + + // ExpTime returns the expiry time of the allowance. + ExpTime() (*time.Time, error) } diff --git a/x/feegrant/filtered_fee.go b/x/feegrant/filtered_fee.go index d842e40852a..c9e2f1ebb34 100644 --- a/x/feegrant/filtered_fee.go +++ b/x/feegrant/filtered_fee.go @@ -1,6 +1,8 @@ package feegrant import ( + "time" + "github.com/gogo/protobuf/proto" "github.com/cosmos/cosmos-sdk/codec/types" @@ -103,3 +105,11 @@ func (a *AllowedMsgAllowance) ValidateBasic() error { return allowance.ValidateBasic() } + +func (a *AllowedMsgAllowance) ExpTime() (*time.Time, error) { + allowance, err := a.GetAllowance() + if err != nil { + return nil, err + } + return allowance.ExpTime() +} diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 197603b386a..86d493e5e4c 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -2,6 +2,7 @@ package keeper import ( "fmt" + "time" storetypes "github.com/cosmos/cosmos-sdk/store/types" "github.com/tendermint/tendermint/libs/log" @@ -50,10 +51,24 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, store := ctx.KVStore(k.storeKey) key := feegrant.FeeAllowanceKey(granter, grantee) - existedGrant, err := k.getGrant(ctx, grantee, granter) - if err != nil && existedGrant.GetAllowance() != nil { - // k.removeFromGrantKey(ctx, feegrant.FeeAllowancePrefixQueue(feeAllowance)) + var exp *time.Time + existingGrant, err := k.getGrant(ctx, grantee, granter) + if err != nil && existingGrant.GetAllowance() != nil { + grant1, err := existingGrant.GetGrant() + if err != nil { + return err + } + + exp, err = grant1.ExpTime() + if err != nil { + return err + } + + if exp != nil { + k.removeFromGrantQueue(ctx, feegrant.FeeAllowancePrefixQueue(exp, key)) + } } + grant, err := feegrant.NewGrant(granter, grantee, feeAllowance) if err != nil { return err @@ -65,6 +80,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, } store.Set(key, bz) + k.insertAllowanceKey(ctx, key, exp) ctx.EventManager().EmitEvent( sdk.NewEvent( @@ -234,9 +250,12 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (*feegrant.GenesisState, error) { }, err } -func (k Keeper) removeFromGrantKey(ctx sdk.Context, key []byte) error { +func (k Keeper) removeFromGrantQueue(ctx sdk.Context, key []byte) { store := ctx.KVStore(k.storeKey) store.Delete(key) +} - return nil +func (k Keeper) insertAllowanceKey(ctx sdk.Context, grantKey []byte, exp *time.Time) { + store := ctx.KVStore(k.storeKey) + store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), grantKey) } diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 5e8b916589a..772a8857fab 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -38,8 +38,8 @@ func FeeAllowancePrefixByGrantee(grantee sdk.AccAddress) []byte { return append(FeeAllowanceKeyPrefix, address.MustLengthPrefix(grantee.Bytes())...) } -func FeeAllowancePrefixQueue(exp time.Time, allowanceKey []byte) []byte { - allowanceByExpTimeKey := AllowanceByExpTimeKey(exp) +func FeeAllowancePrefixQueue(exp *time.Time, allowanceKey []byte) []byte { + allowanceByExpTimeKey := AllowanceByExpTimeKey(*exp) return append(allowanceByExpTimeKey, allowanceKey[1:]...) } diff --git a/x/feegrant/periodic_fee.go b/x/feegrant/periodic_fee.go index c6fbc94b6ea..e5ae56a347a 100644 --- a/x/feegrant/periodic_fee.go +++ b/x/feegrant/periodic_fee.go @@ -105,3 +105,7 @@ func (a PeriodicAllowance) ValidateBasic() error { return nil } + +func (a PeriodicAllowance) ExpTime() (*time.Time, error) { + return a.Basic.ExpTime() +} From 88eb828f4953612d0d9984e60685768917436ed5 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 16 Dec 2021 21:54:01 +0530 Subject: [PATCH 03/26] add endblocker --- x/feegrant/keeper/keeper.go | 12 ++++++++++++ x/feegrant/key.go | 6 +++--- x/feegrant/module/abci.go | 11 +++++++++++ x/feegrant/module/module.go | 3 ++- 4 files changed, 28 insertions(+), 4 deletions(-) create mode 100644 x/feegrant/module/abci.go diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 86d493e5e4c..71bf7834d7e 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -259,3 +259,15 @@ func (k Keeper) insertAllowanceKey(ctx sdk.Context, grantKey []byte, exp *time.T store := ctx.KVStore(k.storeKey) store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), grantKey) } + +// RemoveExpiredAllowances iterates grantsByExpiryQueue and deletes the expired grants. +func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context, exp *time.Time) { + store := ctx.KVStore(k.storeKey) + iterator := store.Iterator(feegrant.FeeAllowanceQueuePrefix, sdk.FormatTimeBytes(*exp)) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + store.Delete(iterator.Key()) + store.Delete(iterator.Value()) + } +} diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 772a8857fab..4a13d721391 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -39,10 +39,10 @@ func FeeAllowancePrefixByGrantee(grantee sdk.AccAddress) []byte { } func FeeAllowancePrefixQueue(exp *time.Time, allowanceKey []byte) []byte { - allowanceByExpTimeKey := AllowanceByExpTimeKey(*exp) + allowanceByExpTimeKey := AllowanceByExpTimeKey(exp) return append(allowanceByExpTimeKey, allowanceKey[1:]...) } -func AllowanceByExpTimeKey(exp time.Time) []byte { - return append(FeeAllowanceQueuePrefix, sdk.FormatTimeBytes(exp)...) +func AllowanceByExpTimeKey(exp *time.Time) []byte { + return append(FeeAllowanceQueuePrefix, sdk.FormatTimeBytes(*exp)...) } diff --git a/x/feegrant/module/abci.go b/x/feegrant/module/abci.go new file mode 100644 index 00000000000..ab72ed2f52c --- /dev/null +++ b/x/feegrant/module/abci.go @@ -0,0 +1,11 @@ +package module + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/feegrant/keeper" +) + +func EndBlocker(ctx sdk.Context, k keeper.Keeper) { + exp := ctx.BlockTime() + k.RemoveExpiredAllowances(ctx, &exp) +} diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index 44107bc733d..ce99cf12038 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -180,7 +180,8 @@ func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} // EndBlock returns the end blocker for the feegrant module. It returns no validator // updates. -func (AppModule) EndBlock(_ sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { +func (am AppModule) EndBlock(ctx sdk.Context, _ abci.RequestEndBlock) []abci.ValidatorUpdate { + EndBlocker(ctx, am.keeper) return []abci.ValidatorUpdate{} } From 39ef3b9e3135fef9821cc0dcc60b2f6486f747e9 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 21 Dec 2021 15:56:32 +0530 Subject: [PATCH 04/26] add tests --- x/feegrant/keeper/keeper.go | 13 ++++-- x/feegrant/keeper/keeper_test.go | 72 ++++++++++++++++++++++++++++++++ x/feegrant/module/abci.go | 3 +- 3 files changed, 83 insertions(+), 5 deletions(-) diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 71bf7834d7e..f10e83e11a3 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -80,7 +80,13 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, } store.Set(key, bz) - k.insertAllowanceKey(ctx, key, exp) + expiration, err := feeAllowance.ExpTime() + + if err != nil { + return err + } else if expiration != nil { + k.insertAllowanceKey(ctx, key, expiration) + } ctx.EventManager().EmitEvent( sdk.NewEvent( @@ -261,9 +267,10 @@ func (k Keeper) insertAllowanceKey(ctx sdk.Context, grantKey []byte, exp *time.T } // RemoveExpiredAllowances iterates grantsByExpiryQueue and deletes the expired grants. -func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context, exp *time.Time) { +func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context) { + exp := ctx.BlockTime() store := ctx.KVStore(k.storeKey) - iterator := store.Iterator(feegrant.FeeAllowanceQueuePrefix, sdk.FormatTimeBytes(*exp)) + iterator := store.Iterator(feegrant.FeeAllowanceQueuePrefix, sdk.InclusiveEndBytes(feegrant.AllowanceByExpTimeKey(&exp))) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { diff --git a/x/feegrant/keeper/keeper_test.go b/x/feegrant/keeper/keeper_test.go index 8397f264fb9..ee9e0157e34 100644 --- a/x/feegrant/keeper/keeper_test.go +++ b/x/feegrant/keeper/keeper_test.go @@ -257,5 +257,77 @@ 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, + }, + }, + } + + 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) + } + }) + } } diff --git a/x/feegrant/module/abci.go b/x/feegrant/module/abci.go index ab72ed2f52c..34c19c528d6 100644 --- a/x/feegrant/module/abci.go +++ b/x/feegrant/module/abci.go @@ -6,6 +6,5 @@ import ( ) func EndBlocker(ctx sdk.Context, k keeper.Keeper) { - exp := ctx.BlockTime() - k.RemoveExpiredAllowances(ctx, &exp) + k.RemoveExpiredAllowances(ctx) } From 4c72059d4b67eabd648492614f84cc6b4134da67 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 21 Dec 2021 17:47:42 +0530 Subject: [PATCH 05/26] changes --- x/feegrant/events.go | 1 + x/feegrant/keeper/keeper.go | 35 ++++++++++++++++++++- x/feegrant/keeper/migrations/v44/keys.go | 39 ++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 x/feegrant/keeper/migrations/v44/keys.go diff --git a/x/feegrant/events.go b/x/feegrant/events.go index a4470b82704..67aafafb4a3 100644 --- a/x/feegrant/events.go +++ b/x/feegrant/events.go @@ -5,6 +5,7 @@ const ( EventTypeUseFeeGrant = "use_feegrant" EventTypeRevokeFeeGrant = "revoke_feegrant" EventTypeSetFeeGrant = "set_feegrant" + EventTypeUpdateFeeGrant = "update_feegrant" AttributeKeyGranter = "granter" AttributeKeyGrantee = "grantee" diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index f10e83e11a3..921b71b965f 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -99,6 +99,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) @@ -204,7 +237,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) { diff --git a/x/feegrant/keeper/migrations/v44/keys.go b/x/feegrant/keeper/migrations/v44/keys.go new file mode 100644 index 00000000000..7bb537adeee --- /dev/null +++ b/x/feegrant/keeper/migrations/v44/keys.go @@ -0,0 +1,39 @@ +package v044 + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" + "github.com/cosmos/cosmos-sdk/types/kv" +) + +var ( + GrantPrefix = []byte{0x01} +) + +func GrantStoreKey(grantee sdk.AccAddress, granter sdk.AccAddress) []byte { + granter = address.MustLengthPrefix(granter) + grantee = address.MustLengthPrefix(grantee) + + l := 1 + len(grantee) + len(granter) + var key = make([]byte, l) + copy(key, GrantPrefix) + copy(key[1:], granter) + copy(key[1+len(granter):], grantee) + + return key +} + +// ParseGrantKey - split granter, grantee address and msg type from the authorization key +func ParseGrantKey(key []byte) (granterAddr, granteeAddr sdk.AccAddress) { + // key is of format: + // + kv.AssertKeyAtLeastLength(key, 2) + granterAddrLen := key[0] + kv.AssertKeyAtLeastLength(key, int(2+granterAddrLen)) + granterAddr = sdk.AccAddress(key[1 : 1+granterAddrLen]) + granteeAddrLen := int(key[1+granterAddrLen]) + kv.AssertKeyAtLeastLength(key, 3+int(granterAddrLen+byte(granteeAddrLen))) + granteeAddr = sdk.AccAddress(key[2+granterAddrLen : 2+granterAddrLen+byte(granteeAddrLen)]) + + return granterAddr, granteeAddr +} From a67604afa8f1e90ee08e744138eb7ef0f176ced4 Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 22 Dec 2021 16:16:10 +0530 Subject: [PATCH 06/26] added tests for migration --- x/feegrant/keeper/keeper.go | 2 +- x/feegrant/keeper/keeper_test.go | 10 +++ x/feegrant/keeper/migrations.go | 21 ++++++ x/feegrant/keeper/migrations/v44/keys.go | 39 ----------- x/feegrant/key.go | 9 ++- x/feegrant/migrations/v045/keys.go | 5 ++ x/feegrant/migrations/v045/store.go | 50 +++++++++++++++ x/feegrant/migrations/v045/store_test.go | 82 ++++++++++++++++++++++++ x/feegrant/module/module.go | 7 +- 9 files changed, 181 insertions(+), 44 deletions(-) create mode 100644 x/feegrant/keeper/migrations.go delete mode 100644 x/feegrant/keeper/migrations/v44/keys.go create mode 100644 x/feegrant/migrations/v045/keys.go create mode 100644 x/feegrant/migrations/v045/store.go create mode 100644 x/feegrant/migrations/v045/store_test.go diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 921b71b965f..51961a2e77b 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -303,7 +303,7 @@ func (k Keeper) insertAllowanceKey(ctx sdk.Context, grantKey []byte, exp *time.T func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context) { exp := ctx.BlockTime() store := ctx.KVStore(k.storeKey) - iterator := store.Iterator(feegrant.FeeAllowanceQueuePrefix, sdk.InclusiveEndBytes(feegrant.AllowanceByExpTimeKey(&exp))) + iterator := store.Iterator(feegrant.FeeAllowanceQueueKeyPrefix, sdk.InclusiveEndBytes(feegrant.AllowanceByExpTimeKey(&exp))) defer iterator.Close() for ; iterator.Valid(); iterator.Next() { diff --git a/x/feegrant/keeper/keeper_test.go b/x/feegrant/keeper/keeper_test.go index ee9e0157e34..153cf892943 100644 --- a/x/feegrant/keeper/keeper_test.go +++ b/x/feegrant/keeper/keeper_test.go @@ -314,6 +314,16 @@ func (suite *KeeperTestSuite) TestPruneGrants() { 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 { diff --git a/x/feegrant/keeper/migrations.go b/x/feegrant/keeper/migrations.go new file mode 100644 index 00000000000..6b39efaa43d --- /dev/null +++ b/x/feegrant/keeper/migrations.go @@ -0,0 +1,21 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + v045 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v045" +) + +// 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 v045.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) +} diff --git a/x/feegrant/keeper/migrations/v44/keys.go b/x/feegrant/keeper/migrations/v44/keys.go deleted file mode 100644 index 7bb537adeee..00000000000 --- a/x/feegrant/keeper/migrations/v44/keys.go +++ /dev/null @@ -1,39 +0,0 @@ -package v044 - -import ( - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/types/address" - "github.com/cosmos/cosmos-sdk/types/kv" -) - -var ( - GrantPrefix = []byte{0x01} -) - -func GrantStoreKey(grantee sdk.AccAddress, granter sdk.AccAddress) []byte { - granter = address.MustLengthPrefix(granter) - grantee = address.MustLengthPrefix(grantee) - - l := 1 + len(grantee) + len(granter) - var key = make([]byte, l) - copy(key, GrantPrefix) - copy(key[1:], granter) - copy(key[1+len(granter):], grantee) - - return key -} - -// ParseGrantKey - split granter, grantee address and msg type from the authorization key -func ParseGrantKey(key []byte) (granterAddr, granteeAddr sdk.AccAddress) { - // key is of format: - // - kv.AssertKeyAtLeastLength(key, 2) - granterAddrLen := key[0] - kv.AssertKeyAtLeastLength(key, int(2+granterAddrLen)) - granterAddr = sdk.AccAddress(key[1 : 1+granterAddrLen]) - granteeAddrLen := int(key[1+granterAddrLen]) - kv.AssertKeyAtLeastLength(key, 3+int(granterAddrLen+byte(granteeAddrLen))) - granteeAddr = sdk.AccAddress(key[2+granterAddrLen : 2+granterAddrLen+byte(granteeAddrLen)]) - - return granterAddr, granteeAddr -} diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 4a13d721391..5a32be24632 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -23,8 +23,10 @@ const ( var ( // FeeAllowanceKeyPrefix is the set of the kvstore for fee allowance data - FeeAllowanceKeyPrefix = []byte{0x00} - FeeAllowanceQueuePrefix = []byte{0x01} + FeeAllowanceKeyPrefix = []byte{0x00} + + // FeeAllowanceQueueKeyPrefix is the set of the kvstore for fee allowance keys data + FeeAllowanceQueueKeyPrefix = []byte{0x01} ) // FeeAllowanceKey is the canonical key to store a grant from granter to grantee @@ -38,11 +40,12 @@ func FeeAllowancePrefixByGrantee(grantee sdk.AccAddress) []byte { return append(FeeAllowanceKeyPrefix, address.MustLengthPrefix(grantee.Bytes())...) } +// FeeAllowancePrefixQueue is the canonical key to store grant key. func FeeAllowancePrefixQueue(exp *time.Time, allowanceKey []byte) []byte { allowanceByExpTimeKey := AllowanceByExpTimeKey(exp) return append(allowanceByExpTimeKey, allowanceKey[1:]...) } func AllowanceByExpTimeKey(exp *time.Time) []byte { - return append(FeeAllowanceQueuePrefix, sdk.FormatTimeBytes(*exp)...) + return append(FeeAllowanceQueueKeyPrefix, sdk.FormatTimeBytes(*exp)...) } diff --git a/x/feegrant/migrations/v045/keys.go b/x/feegrant/migrations/v045/keys.go new file mode 100644 index 00000000000..9b32780fd32 --- /dev/null +++ b/x/feegrant/migrations/v045/keys.go @@ -0,0 +1,5 @@ +package v045 + +var ( + ModuleName = "feegrant" +) diff --git a/x/feegrant/migrations/v045/store.go b/x/feegrant/migrations/v045/store.go new file mode 100644 index 00000000000..dbe8bdb3d9b --- /dev/null +++ b/x/feegrant/migrations/v045/store.go @@ -0,0 +1,50 @@ +package v045 + +import ( + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/store/prefix" + storetypes "github.com/cosmos/cosmos-sdk/store/types" + "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/feegrant" +) + +func addAllowancesByExpTimeQueue(ctx types.Context, store storetypes.KVStore, cdc codec.BinaryCodec) error { + prefixStore := prefix.NewStore(store, feegrant.FeeAllowanceKeyPrefix) + iterator := prefixStore.Iterator(nil, nil) + defer iterator.Close() + + for ; iterator.Valid(); iterator.Next() { + var grant feegrant.Grant + bz := iterator.Value() + if err := cdc.Unmarshal(bz, &grant); err != nil { + return err + } + + grantInfo, err := grant.GetGrant() + if err != nil { + return err + } + + exp, err := grantInfo.ExpTime() + if err != nil { + return err + } + + if exp != nil { + key := iterator.Key() + if exp.Before(ctx.BlockTime()) { + prefixStore.Delete(key) + } else { + grantByExpTimeQueueKey := feegrant.FeeAllowancePrefixQueue(exp, key[1:]) + store.Set(grantByExpTimeQueueKey, key) + } + } + } + + return nil +} + +func MigrateStore(ctx types.Context, storeKey storetypes.StoreKey, cdc codec.BinaryCodec) error { + store := ctx.KVStore(storeKey) + return addAllowancesByExpTimeQueue(ctx, store, cdc) +} diff --git a/x/feegrant/migrations/v045/store_test.go b/x/feegrant/migrations/v045/store_test.go new file mode 100644 index 00000000000..987db247595 --- /dev/null +++ b/x/feegrant/migrations/v045/store_test.go @@ -0,0 +1,82 @@ +package v045_test + +import ( + "testing" + "time" + + "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519" + "github.com/cosmos/cosmos-sdk/simapp" + "github.com/cosmos/cosmos-sdk/testutil" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/feegrant" + v045 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v045" + "github.com/stretchr/testify/require" +) + +func TestMigration(t *testing.T) { + encCfg := simapp.MakeTestEncodingConfig() + cdc := encCfg.Codec + feegrantKey := sdk.NewKVStoreKey(v045.ModuleName) + ctx := testutil.DefaultContext(feegrantKey, sdk.NewTransientStoreKey("transient_test")) + granter1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) + grantee1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) + granter2 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) + grantee2 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) + + spendLimit := sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))) + now := ctx.BlockTime() + oneDay := ctx.BlockTime().AddDate(0, 0, 1) + + grants := []struct { + granter sdk.AccAddress + grantee sdk.AccAddress + spendLimit sdk.Coins + expiration *time.Time + }{ + { + granter: granter1, + grantee: grantee1, + spendLimit: spendLimit, + expiration: &oneDay, + }, + { + granter: granter2, + grantee: grantee2, + spendLimit: spendLimit, + expiration: &now, + }, + { + granter: granter1, + grantee: grantee2, + spendLimit: spendLimit, + }, + { + granter: granter2, + grantee: grantee1, + expiration: &now, + }, + } + + store := ctx.KVStore(feegrantKey) + for _, grant := range grants { + newGrant, err := feegrant.NewGrant(grant.granter, grant.grantee, &feegrant.BasicAllowance{ + SpendLimit: grant.spendLimit, + Expiration: grant.expiration, + }) + require.NoError(t, err) + + bz, err := cdc.Marshal(&newGrant) + require.NoError(t, err) + + store.Set(feegrant.FeeAllowanceKey(grant.granter, grant.grantee), bz) + } + + ctx = ctx.WithBlockTime(ctx.BlockTime().Add(1 * time.Hour)) + require.NoError(t, v045.MigrateStore(ctx, feegrantKey, cdc)) + store = ctx.KVStore(feegrantKey) + + require.NotNil(t, store.Get(feegrant.FeeAllowanceKey(granter1, grantee1))) + require.Nil(t, store.Get(feegrant.FeeAllowanceKey(granter2, grantee2))) + require.NotNil(t, store.Get(feegrant.FeeAllowanceKey(granter1, grantee2))) + require.Nil(t, store.Get(feegrant.FeeAllowanceKey(granter2, grantee1))) +} diff --git a/x/feegrant/module/module.go b/x/feegrant/module/module.go index ce99cf12038..b44c6c29594 100644 --- a/x/feegrant/module/module.go +++ b/x/feegrant/module/module.go @@ -48,6 +48,11 @@ func (AppModuleBasic) Name() string { func (am AppModule) RegisterServices(cfg module.Configurator) { feegrant.RegisterMsgServer(cfg.MsgServer(), keeper.NewMsgServerImpl(am.keeper)) feegrant.RegisterQueryServer(cfg.QueryServer(), am.keeper) + m := keeper.NewMigrator(am.keeper) + err := cfg.RegisterMigration(feegrant.ModuleName, 1, m.Migrate1to2) + if err != nil { + panic(err) + } } // RegisterLegacyAminoCodec registers the feegrant module's types for the given codec. @@ -173,7 +178,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // ConsensusVersion implements AppModule/ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 1 } +func (AppModule) ConsensusVersion() uint64 { return 2 } // BeginBlock returns the begin blocker for the feegrant module. func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} From e2304da2810fac5d7813a09af34daa873122c92b Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 22 Dec 2021 16:44:12 +0530 Subject: [PATCH 07/26] add abci tests --- x/feegrant/module/abci_test.go | 79 ++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 x/feegrant/module/abci_test.go diff --git a/x/feegrant/module/abci_test.go b/x/feegrant/module/abci_test.go new file mode 100644 index 00000000000..4c8aa03d410 --- /dev/null +++ b/x/feegrant/module/abci_test.go @@ -0,0 +1,79 @@ +package module_test + +import ( + "testing" + + "github.com/cosmos/cosmos-sdk/baseapp" + "github.com/cosmos/cosmos-sdk/simapp" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/feegrant" + "github.com/cosmos/cosmos-sdk/x/feegrant/module" + "github.com/stretchr/testify/require" + abci "github.com/tendermint/tendermint/abci/types" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" +) + +func TestFeegrantPruning(t *testing.T) { + app := simapp.Setup(t, false) + + ctx := app.BaseApp.NewContext(false, tmproto.Header{}) + addrs := simapp.AddTestAddrs(app, ctx, 4, sdk.NewInt(1000)) + granter1 := addrs[0] + granter2 := addrs[1] + granter3 := addrs[2] + grantee := addrs[3] + spendLimit := sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))) + now := ctx.BlockTime() + oneDay := now.AddDate(0, 0, 1) + + header := tmproto.Header{Height: app.LastBlockHeight() + 1} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + app.FeeGrantKeeper.GrantAllowance( + ctx, + granter1, + grantee, + &feegrant.BasicAllowance{ + Expiration: &now, + }, + ) + app.FeeGrantKeeper.GrantAllowance( + ctx, + granter2, + grantee, + &feegrant.BasicAllowance{ + SpendLimit: spendLimit, + }, + ) + app.FeeGrantKeeper.GrantAllowance( + ctx, + granter3, + grantee, + &feegrant.BasicAllowance{ + Expiration: &oneDay, + }, + ) + + queryHelper := baseapp.NewQueryServerTestHelper(ctx, app.InterfaceRegistry()) + feegrant.RegisterQueryServer(queryHelper, app.FeeGrantKeeper) + queryClient := feegrant.NewQueryClient(queryHelper) + + module.EndBlocker(ctx, app.FeeGrantKeeper) + + res, err := queryClient.Allowances(ctx.Context(), &feegrant.QueryAllowancesRequest{ + Grantee: grantee.String(), + }) + require.NoError(t, err) + require.NotNil(t, res) + require.Len(t, res.Allowances, 3) + + ctx = ctx.WithBlockTime(now.AddDate(0, 0, 2)) + module.EndBlocker(ctx, app.FeeGrantKeeper) + + res, err = queryClient.Allowances(ctx.Context(), &feegrant.QueryAllowancesRequest{ + Grantee: grantee.String(), + }) + require.NoError(t, err) + require.NotNil(t, res) + require.Len(t, res.Allowances, 1) +} From 5c07e7aa036fbee6682db28bf611b9af7561b407 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 23 Dec 2021 13:02:22 +0530 Subject: [PATCH 08/26] review changes --- x/feegrant/basic_fee.go | 2 +- x/feegrant/fees.go | 4 ++-- x/feegrant/filtered_fee.go | 4 ++-- x/feegrant/keeper/keeper.go | 4 ++-- x/feegrant/migrations/v045/store.go | 2 +- x/feegrant/periodic_fee.go | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/x/feegrant/basic_fee.go b/x/feegrant/basic_fee.go index 6c0e809d997..3fc7796c7b6 100644 --- a/x/feegrant/basic_fee.go +++ b/x/feegrant/basic_fee.go @@ -55,6 +55,6 @@ func (a BasicAllowance) ValidateBasic() error { return nil } -func (a BasicAllowance) ExpTime() (*time.Time, error) { +func (a BasicAllowance) Expiry() (*time.Time, error) { return a.Expiration, nil } diff --git a/x/feegrant/fees.go b/x/feegrant/fees.go index 31f67d03e02..ea89fd23561 100644 --- a/x/feegrant/fees.go +++ b/x/feegrant/fees.go @@ -25,6 +25,6 @@ type FeeAllowanceI interface { // Don't allow negative amounts, or negative periods for example. ValidateBasic() error - // ExpTime returns the expiry time of the allowance. - ExpTime() (*time.Time, error) + // Expiry returns the expiry time of the allowance. + Expiry() (*time.Time, error) } diff --git a/x/feegrant/filtered_fee.go b/x/feegrant/filtered_fee.go index c9e2f1ebb34..3c1fcd0c82f 100644 --- a/x/feegrant/filtered_fee.go +++ b/x/feegrant/filtered_fee.go @@ -106,10 +106,10 @@ func (a *AllowedMsgAllowance) ValidateBasic() error { return allowance.ValidateBasic() } -func (a *AllowedMsgAllowance) ExpTime() (*time.Time, error) { +func (a *AllowedMsgAllowance) Expiry() (*time.Time, error) { allowance, err := a.GetAllowance() if err != nil { return nil, err } - return allowance.ExpTime() + return allowance.Expiry() } diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 51961a2e77b..055c96bfab7 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -59,7 +59,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, return err } - exp, err = grant1.ExpTime() + exp, err = grant1.Expiry() if err != nil { return err } @@ -80,7 +80,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, } store.Set(key, bz) - expiration, err := feeAllowance.ExpTime() + expiration, err := feeAllowance.Expiry() if err != nil { return err diff --git a/x/feegrant/migrations/v045/store.go b/x/feegrant/migrations/v045/store.go index dbe8bdb3d9b..6a4d57e119d 100644 --- a/x/feegrant/migrations/v045/store.go +++ b/x/feegrant/migrations/v045/store.go @@ -25,7 +25,7 @@ func addAllowancesByExpTimeQueue(ctx types.Context, store storetypes.KVStore, cd return err } - exp, err := grantInfo.ExpTime() + exp, err := grantInfo.Expiry() if err != nil { return err } diff --git a/x/feegrant/periodic_fee.go b/x/feegrant/periodic_fee.go index e5ae56a347a..b7f51447474 100644 --- a/x/feegrant/periodic_fee.go +++ b/x/feegrant/periodic_fee.go @@ -106,6 +106,6 @@ func (a PeriodicAllowance) ValidateBasic() error { return nil } -func (a PeriodicAllowance) ExpTime() (*time.Time, error) { - return a.Basic.ExpTime() +func (a PeriodicAllowance) Expiry() (*time.Time, error) { + return a.Basic.Expiry() } From 0d6c50a176725f2327224c55c4a20407822843d4 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 23 Dec 2021 15:19:02 +0530 Subject: [PATCH 09/26] review changes --- x/feegrant/basic_fee.go | 2 +- x/feegrant/fees.go | 4 ++-- x/feegrant/filtered_fee.go | 4 ++-- x/feegrant/keeper/keeper.go | 4 ++-- x/feegrant/migrations/v045/store.go | 2 +- x/feegrant/periodic_fee.go | 4 ++-- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/x/feegrant/basic_fee.go b/x/feegrant/basic_fee.go index 3fc7796c7b6..703b509b3a6 100644 --- a/x/feegrant/basic_fee.go +++ b/x/feegrant/basic_fee.go @@ -55,6 +55,6 @@ func (a BasicAllowance) ValidateBasic() error { return nil } -func (a BasicAllowance) Expiry() (*time.Time, error) { +func (a BasicAllowance) ExpiresAt() (*time.Time, error) { return a.Expiration, nil } diff --git a/x/feegrant/fees.go b/x/feegrant/fees.go index ea89fd23561..9b2c0333868 100644 --- a/x/feegrant/fees.go +++ b/x/feegrant/fees.go @@ -25,6 +25,6 @@ type FeeAllowanceI interface { // Don't allow negative amounts, or negative periods for example. ValidateBasic() error - // Expiry returns the expiry time of the allowance. - Expiry() (*time.Time, error) + // ExpiresAt returns the expiry time of the allowance. + ExpiresAt() (*time.Time, error) } diff --git a/x/feegrant/filtered_fee.go b/x/feegrant/filtered_fee.go index 3c1fcd0c82f..00b4fd67298 100644 --- a/x/feegrant/filtered_fee.go +++ b/x/feegrant/filtered_fee.go @@ -106,10 +106,10 @@ func (a *AllowedMsgAllowance) ValidateBasic() error { return allowance.ValidateBasic() } -func (a *AllowedMsgAllowance) Expiry() (*time.Time, error) { +func (a *AllowedMsgAllowance) ExpiresAt() (*time.Time, error) { allowance, err := a.GetAllowance() if err != nil { return nil, err } - return allowance.Expiry() + return allowance.ExpiresAt() } diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 055c96bfab7..38c0b0bc9f5 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -59,7 +59,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, return err } - exp, err = grant1.Expiry() + exp, err = grant1.ExpiresAt() if err != nil { return err } @@ -80,7 +80,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, } store.Set(key, bz) - expiration, err := feeAllowance.Expiry() + expiration, err := feeAllowance.ExpiresAt() if err != nil { return err diff --git a/x/feegrant/migrations/v045/store.go b/x/feegrant/migrations/v045/store.go index 6a4d57e119d..1801ae35975 100644 --- a/x/feegrant/migrations/v045/store.go +++ b/x/feegrant/migrations/v045/store.go @@ -25,7 +25,7 @@ func addAllowancesByExpTimeQueue(ctx types.Context, store storetypes.KVStore, cd return err } - exp, err := grantInfo.Expiry() + exp, err := grantInfo.ExpiresAt() if err != nil { return err } diff --git a/x/feegrant/periodic_fee.go b/x/feegrant/periodic_fee.go index b7f51447474..43229247ff0 100644 --- a/x/feegrant/periodic_fee.go +++ b/x/feegrant/periodic_fee.go @@ -106,6 +106,6 @@ func (a PeriodicAllowance) ValidateBasic() error { return nil } -func (a PeriodicAllowance) Expiry() (*time.Time, error) { - return a.Basic.Expiry() +func (a PeriodicAllowance) ExpiresAt() (*time.Time, error) { + return a.Basic.ExpiresAt() } From 9e287d833ead741e7653c2ae3a1b72ad7e297293 Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 5 Jan 2022 16:16:04 +0530 Subject: [PATCH 10/26] remove value storage --- x/feegrant/keeper/keeper.go | 18 +++++++++++------- x/feegrant/migrations/v045/store.go | 4 ++-- x/feegrant/migrations/v045/store_test.go | 11 ++++++----- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 38c0b0bc9f5..e9eeb58af97 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -53,7 +53,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, var exp *time.Time existingGrant, err := k.getGrant(ctx, grantee, granter) - if err != nil && existingGrant.GetAllowance() != nil { + if err != nil && existingGrant != nil && existingGrant.GetAllowance() != nil { grant1, err := existingGrant.GetGrant() if err != nil { return err @@ -65,7 +65,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, } if exp != nil { - k.removeFromGrantQueue(ctx, feegrant.FeeAllowancePrefixQueue(exp, key)) + k.removeFromGrantQueue(ctx, exp, key) } } @@ -85,7 +85,7 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, if err != nil { return err } else if expiration != nil { - k.insertAllowanceKey(ctx, key, expiration) + k.addToFeeAllowanceQueue(ctx, key, expiration) } ctx.EventManager().EmitEvent( @@ -289,14 +289,15 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) (*feegrant.GenesisState, error) { }, err } -func (k Keeper) removeFromGrantQueue(ctx sdk.Context, key []byte) { +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) insertAllowanceKey(ctx sdk.Context, grantKey []byte, exp *time.Time) { +func (k Keeper) addToFeeAllowanceQueue(ctx sdk.Context, grantKey []byte, exp *time.Time) { store := ctx.KVStore(k.storeKey) - store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), grantKey) + store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), []byte{}) } // RemoveExpiredAllowances iterates grantsByExpiryQueue and deletes the expired grants. @@ -308,6 +309,9 @@ func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context) { for ; iterator.Valid(); iterator.Next() { store.Delete(iterator.Key()) - store.Delete(iterator.Value()) + expLen := len(sdk.FormatTimeBytes(time.Now())) + + // 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:]...)) } } diff --git a/x/feegrant/migrations/v045/store.go b/x/feegrant/migrations/v045/store.go index 1801ae35975..133952b28fa 100644 --- a/x/feegrant/migrations/v045/store.go +++ b/x/feegrant/migrations/v045/store.go @@ -35,8 +35,8 @@ func addAllowancesByExpTimeQueue(ctx types.Context, store storetypes.KVStore, cd if exp.Before(ctx.BlockTime()) { prefixStore.Delete(key) } else { - grantByExpTimeQueueKey := feegrant.FeeAllowancePrefixQueue(exp, key[1:]) - store.Set(grantByExpTimeQueueKey, key) + grantByExpTimeQueueKey := feegrant.FeeAllowancePrefixQueue(exp, key) + store.Set(grantByExpTimeQueueKey, []byte{}) } } } diff --git a/x/feegrant/migrations/v045/store_test.go b/x/feegrant/migrations/v045/store_test.go index 987db247595..d176039f289 100644 --- a/x/feegrant/migrations/v045/store_test.go +++ b/x/feegrant/migrations/v045/store_test.go @@ -25,7 +25,8 @@ func TestMigration(t *testing.T) { spendLimit := sdk.NewCoins(sdk.NewCoin("stake", sdk.NewInt(1000))) now := ctx.BlockTime() - oneDay := ctx.BlockTime().AddDate(0, 0, 1) + oneDay := now.AddDate(0, 0, 1) + twoDays := now.AddDate(0, 0, 2) grants := []struct { granter sdk.AccAddress @@ -37,13 +38,13 @@ func TestMigration(t *testing.T) { granter: granter1, grantee: grantee1, spendLimit: spendLimit, - expiration: &oneDay, + expiration: &twoDays, }, { granter: granter2, grantee: grantee2, spendLimit: spendLimit, - expiration: &now, + expiration: &oneDay, }, { granter: granter1, @@ -53,7 +54,7 @@ func TestMigration(t *testing.T) { { granter: granter2, grantee: grantee1, - expiration: &now, + expiration: &oneDay, }, } @@ -71,7 +72,7 @@ func TestMigration(t *testing.T) { store.Set(feegrant.FeeAllowanceKey(grant.granter, grant.grantee), bz) } - ctx = ctx.WithBlockTime(ctx.BlockTime().Add(1 * time.Hour)) + ctx = ctx.WithBlockTime(now.Add(30 * time.Hour)) require.NoError(t, v045.MigrateStore(ctx, feegrantKey, cdc)) store = ctx.KVStore(feegrantKey) From 36dbffcc03e9806c84aef5d3fec583d7b2a20917 Mon Sep 17 00:00:00 2001 From: atheesh Date: Wed, 19 Jan 2022 16:36:31 +0530 Subject: [PATCH 11/26] review changes --- x/feegrant/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index e9eeb58af97..e961be912e4 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -309,7 +309,7 @@ func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context) { for ; iterator.Valid(); iterator.Next() { store.Delete(iterator.Key()) - expLen := len(sdk.FormatTimeBytes(time.Now())) + 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:]...)) From 9fb03f8e46b899f24834b57d52f5e73375eed608 Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 21 Jan 2022 15:23:21 +0530 Subject: [PATCH 12/26] address review changes --- x/feegrant/keeper/keeper.go | 39 ++++++++++++++++++++++------------ x/feegrant/key.go | 20 +++++++++++++++-- x/feegrant/spec/01_concepts.md | 4 ++++ x/feegrant/spec/02_state.md | 8 +++++++ 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index e961be912e4..408273f91af 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -51,22 +51,42 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, store := ctx.KVStore(k.storeKey) key := feegrant.FeeAllowanceKey(granter, grantee) - var exp *time.Time + var oldExp *time.Time existingGrant, err := k.getGrant(ctx, grantee, granter) if err != nil && existingGrant != nil && existingGrant.GetAllowance() != nil { - grant1, err := existingGrant.GetGrant() + grantInfo, err := existingGrant.GetGrant() if err != nil { return err } - exp, err = grant1.ExpiresAt() + oldExp, err = grantInfo.ExpiresAt() if err != nil { return err } + } - if exp != nil { - k.removeFromGrantQueue(ctx, exp, key) - } + 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) @@ -80,13 +100,6 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress, } store.Set(key, bz) - expiration, err := feeAllowance.ExpiresAt() - - if err != nil { - return err - } else if expiration != nil { - k.addToFeeAllowanceQueue(ctx, key, expiration) - } ctx.EventManager().EmitEvent( sdk.NewEvent( diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 5a32be24632..bc5ff3fc344 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -23,29 +23,45 @@ const ( var ( // FeeAllowanceKeyPrefix is the set of the kvstore for fee allowance data + // - 0x00: allowance FeeAllowanceKeyPrefix = []byte{0x00} // FeeAllowanceQueueKeyPrefix is the set of the kvstore for fee allowance keys data + // - 0x01: FeeAllowanceQueueKeyPrefix = []byte{0x01} ) // FeeAllowanceKey is the canonical key to store a grant from granter to grantee // We store by grantee first to allow searching by everyone who granted to you +// +// Key format: +// - <0x00> func FeeAllowanceKey(granter sdk.AccAddress, grantee sdk.AccAddress) []byte { return append(FeeAllowancePrefixByGrantee(grantee), address.MustLengthPrefix(granter.Bytes())...) } // FeeAllowancePrefixByGrantee returns a prefix to scan for all grants to this given address. +// +// Key format: +// - <0x00> func FeeAllowancePrefixByGrantee(grantee sdk.AccAddress) []byte { return append(FeeAllowanceKeyPrefix, address.MustLengthPrefix(grantee.Bytes())...) } // FeeAllowancePrefixQueue is the canonical key to store grant key. -func FeeAllowancePrefixQueue(exp *time.Time, allowanceKey []byte) []byte { +// +// Key format: +// - <0x01> +func FeeAllowancePrefixQueue(exp *time.Time, key []byte) []byte { allowanceByExpTimeKey := AllowanceByExpTimeKey(exp) - return append(allowanceByExpTimeKey, allowanceKey[1:]...) + return append(allowanceByExpTimeKey, key...) } +// AllowanceByExpTimeKey returns a key with `FeeAllowanceQueueKeyPrefix`, expiry +// +// Key format: +// - <0x01> func AllowanceByExpTimeKey(exp *time.Time) []byte { + // no need of appending len(exp_bytes) here, `FormatTimeBytes` gives const length everytime. return append(FeeAllowanceQueueKeyPrefix, sdk.FormatTimeBytes(*exp)...) } diff --git a/x/feegrant/spec/01_concepts.md b/x/feegrant/spec/01_concepts.md index 6b22d46b68c..40b3cc8aa04 100644 --- a/x/feegrant/spec/01_concepts.md +++ b/x/feegrant/spec/01_concepts.md @@ -76,3 +76,7 @@ Fees are deducted from grants in the `x/auth` ante handler. To learn more about In order to prevent DoS attacks, using a filtered `x/feegrant` incurs gas. The SDK must assure that the `grantee`'s transactions all conform to the filter set by the `granter`. The SDK does this by iterating over the allowed messages in the filter and charging 10 gas per filtered message. The SDK will then iterate over the messages being sent by the `grantee` to ensure the messages adhere to the filter, also charging 10 gas per message. The SDK will stop iterating and fail the transaction if it finds a message that does not conform to the filter. **WARNING**: The gas is charged against the granted allowance. Ensure your messages conform to the filter, if any, before sending transactions using your allowance. + +## Pruning + +A queue in the state maintained with the prefix of expiration of the grants and checks them with the current block time for every block to prune. \ No newline at end of file diff --git a/x/feegrant/spec/02_state.md b/x/feegrant/spec/02_state.md index 5b12cd2d0ce..47106e00681 100644 --- a/x/feegrant/spec/02_state.md +++ b/x/feegrant/spec/02_state.md @@ -13,3 +13,11 @@ Fee allowance grants are stored in the state as follows: - Grant: `0x00 | grantee_addr_len (1 byte) | grantee_addr_bytes | granter_addr_len (1 byte) | granter_addr_bytes -> ProtocolBuffer(Grant)` +++ https://github.com/cosmos/cosmos-sdk/blob/691032b8be0f7539ec99f8882caecefc51f33d1f/x/feegrant/feegrant.pb.go#L221-L229 + +## FeeAllowanceQueue + +Fee Allowances queue items are identified by combining the `FeeAllowancePrefixQueue` (i.e., 0x01), `expiration`, `grantee` (the account address of fee allowance grantee), `granter` (the account address of fee allowance granter). Endblocker checks `FeeAllowanceQueue` state for the expired grants and prunes them from `FeeAllowance` if there are any found. + +Fee allowance queue keys are stored in the state as follows: + +- Grant: `0x01 | expiration_bytes | grantee_addr_len (1 byte) | grantee_addr_bytes | granter_addr_len (1 byte) | granter_addr_bytes -> EmptyBytes` \ No newline at end of file From 248aced422eef39bba596efe7610be8d964259a9 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 25 Jan 2022 10:51:23 +0530 Subject: [PATCH 13/26] fix tests --- x/feegrant/keeper/keeper_test.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/x/feegrant/keeper/keeper_test.go b/x/feegrant/keeper/keeper_test.go index 153cf892943..933d85c7cef 100644 --- a/x/feegrant/keeper/keeper_test.go +++ b/x/feegrant/keeper/keeper_test.go @@ -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) @@ -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() { From eb9e5ec60ae2c8aa330eed7848a455e5b9ea71c3 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 25 Jan 2022 16:49:27 +0530 Subject: [PATCH 14/26] add log --- errors/abci.go | 1 + 1 file changed, 1 insertion(+) diff --git a/errors/abci.go b/errors/abci.go index afd535e198c..069b4122305 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,6 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { + fmt.Println("err>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", err.Error()) return errPanicWithMsg } if abciCode(err) == internalABCICode { From 45e819e03b7d37c6b78d91975d3c1b9acb461eb2 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 25 Jan 2022 18:08:53 +0530 Subject: [PATCH 15/26] add log --- errors/abci.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index 069b4122305..33c8130ebe2 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,8 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { - fmt.Println("err>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", err.Error()) - return errPanicWithMsg + return Wrapf(ErrPanic, "panic message redacted to hide potentially sensitive system info "+err.Error()) } if abciCode(err) == internalABCICode { return errInternal From 5bf473874b9f4aa2e47ea92a83902ba72bcfd976 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 25 Jan 2022 18:18:38 +0530 Subject: [PATCH 16/26] revert --- errors/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/abci.go b/errors/abci.go index 33c8130ebe2..afd535e198c 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,7 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { - return Wrapf(ErrPanic, "panic message redacted to hide potentially sensitive system info "+err.Error()) + return errPanicWithMsg } if abciCode(err) == internalABCICode { return errInternal From da084025e1ee85a4412344a7e20eb9af19becc7b Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 25 Jan 2022 18:24:18 +0530 Subject: [PATCH 17/26] add error --- errors/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/abci.go b/errors/abci.go index afd535e198c..9b248b0b074 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,7 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { - return errPanicWithMsg + return err } if abciCode(err) == internalABCICode { return errInternal From 52cb90fc0adaec341e97180920205454e446f303 Mon Sep 17 00:00:00 2001 From: atheesh Date: Tue, 25 Jan 2022 21:53:57 +0530 Subject: [PATCH 18/26] revert --- errors/abci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors/abci.go b/errors/abci.go index 9b248b0b074..afd535e198c 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,7 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { - return err + return errPanicWithMsg } if abciCode(err) == internalABCICode { return errInternal From 3cda0992ad3dc5a2094acff477872494bddd3937 Mon Sep 17 00:00:00 2001 From: aleem1314 Date: Thu, 27 Jan 2022 10:49:35 +0530 Subject: [PATCH 19/26] chore: add logs --- errors/abci.go | 2 +- errors/abci_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index afd535e198c..9b248b0b074 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,7 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { - return errPanicWithMsg + return err } if abciCode(err) == internalABCICode { return errInternal diff --git a/errors/abci_test.go b/errors/abci_test.go index b9370bd7d60..5d47d762218 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -169,10 +169,10 @@ func (s *abciTestSuite) TestRedact() { untouched bool // if true we expect the same error after redact changed error // if untouched == false, expect this error }{ - "panic looses message": { - err: Wrap(ErrPanic, "some secret stack trace"), - changed: errPanicWithMsg, - }, + // "panic looses message": { + // err: Wrap(ErrPanic, "some secret stack trace"), + // changed: errPanicWithMsg, + // }, "sdk errors untouched": { err: Wrap(ErrUnauthorized, "cannot drop db"), untouched: true, From 324dfa5ce20056e9c157ced06032317eb3e67130 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 27 Jan 2022 16:27:01 +0530 Subject: [PATCH 20/26] fix tests --- x/feegrant/keeper/grpc_query.go | 3 ++- x/feegrant/key.go | 21 +++++++++++++-------- x/feegrant/key_test.go | 2 +- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/x/feegrant/keeper/grpc_query.go b/x/feegrant/keeper/grpc_query.go index c439d0b7352..0a228c9dcf1 100644 --- a/x/feegrant/keeper/grpc_query.go +++ b/x/feegrant/keeper/grpc_query.go @@ -110,7 +110,8 @@ 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) diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 9b35774658c..99b03dde062 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -1,6 +1,7 @@ package feegrant import ( + "fmt" time "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -38,6 +39,9 @@ var ( // Key format: // - <0x00> func FeeAllowanceKey(granter sdk.AccAddress, grantee sdk.AccAddress) []byte { + fmt.Println("granter", address.MustLengthPrefix(granter.Bytes())) + fmt.Println("grantee", address.MustLengthPrefix(grantee.Bytes())) + fmt.Println("-----------------------------------------------------") return append(FeeAllowancePrefixByGrantee(grantee), address.MustLengthPrefix(granter.Bytes())...) } @@ -68,16 +72,17 @@ func AllowanceByExpTimeKey(exp *time.Time) []byte { } // ParseAddressesFromFeeAllowanceKey exrtacts and returns the granter, grantee from the given key. +// Note: do not send the key with store prefix, remove the store prefix (first byte) while sending. func ParseAddressesFromFeeAllowanceKey(key []byte) (granter, grantee sdk.AccAddress) { // key is of format: - // 0x00 - kv.AssertKeyAtLeastLength(key, 2) - granteeAddrLen := key[1] // remove prefix key - kv.AssertKeyAtLeastLength(key, int(2+granteeAddrLen)) - grantee = sdk.AccAddress(key[2 : 2+granteeAddrLen]) - granterAddrLen := int(key[2+granteeAddrLen]) - kv.AssertKeyAtLeastLength(key, 3+int(granteeAddrLen+byte(granterAddrLen))) - granter = sdk.AccAddress(key[3+granterAddrLen : 3+granteeAddrLen+byte(granterAddrLen)]) + // + kv.AssertKeyAtLeastLength(key, 1) + granteeAddrLen := key[0] // remove prefix key + kv.AssertKeyAtLeastLength(key, 1+int(granteeAddrLen)) + grantee = sdk.AccAddress(key[1 : 1+int(granteeAddrLen)]) + granterAddrLen := int(key[1+granteeAddrLen]) + kv.AssertKeyAtLeastLength(key, 2+int(granteeAddrLen)+int(granterAddrLen)) + granter = sdk.AccAddress(key[2+granterAddrLen : 2+int(granteeAddrLen)+int(granterAddrLen)]) return granter, grantee } diff --git a/x/feegrant/key_test.go b/x/feegrant/key_test.go index 75a7964bec3..01972cc2643 100644 --- a/x/feegrant/key_test.go +++ b/x/feegrant/key_test.go @@ -19,7 +19,7 @@ func TestMarshalAndUnmarshalFeegrantKey(t *testing.T) { require.Len(t, key, len(grantee.Bytes())+len(granter.Bytes())+3) require.Equal(t, feegrant.FeeAllowancePrefixByGrantee(grantee), key[:len(grantee.Bytes())+2]) - g1, g2 := feegrant.ParseAddressesFromFeeAllowanceKey(key) + g1, g2 := feegrant.ParseAddressesFromFeeAllowanceKey(key[1:]) require.Equal(t, granter, g1) require.Equal(t, grantee, g2) } From 7091b40524b2c03ff5a743f94cb2f8413f6b2637 Mon Sep 17 00:00:00 2001 From: atheesh Date: Thu, 27 Jan 2022 16:30:19 +0530 Subject: [PATCH 21/26] revert --- errors/abci.go | 2 +- errors/abci_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/errors/abci.go b/errors/abci.go index 9b248b0b074..afd535e198c 100644 --- a/errors/abci.go +++ b/errors/abci.go @@ -119,7 +119,7 @@ var errPanicWithMsg = Wrapf(ErrPanic, "panic message redacted to hide potentiall // simply returned. func Redact(err error) error { if ErrPanic.Is(err) { - return err + return errPanicWithMsg } if abciCode(err) == internalABCICode { return errInternal diff --git a/errors/abci_test.go b/errors/abci_test.go index 5d47d762218..b9370bd7d60 100644 --- a/errors/abci_test.go +++ b/errors/abci_test.go @@ -169,10 +169,10 @@ func (s *abciTestSuite) TestRedact() { untouched bool // if true we expect the same error after redact changed error // if untouched == false, expect this error }{ - // "panic looses message": { - // err: Wrap(ErrPanic, "some secret stack trace"), - // changed: errPanicWithMsg, - // }, + "panic looses message": { + err: Wrap(ErrPanic, "some secret stack trace"), + changed: errPanicWithMsg, + }, "sdk errors untouched": { err: Wrap(ErrUnauthorized, "cannot drop db"), untouched: true, From d8d626aab3639eafb283cf41e4ccb1e0140bcf90 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 31 Jan 2022 21:53:01 +0530 Subject: [PATCH 22/26] review changes --- x/feegrant/keeper/migrations.go | 4 +- x/feegrant/key.go | 4 -- x/feegrant/migrations/v045/keys.go | 5 -- x/feegrant/migrations/v046/keys.go | 47 +++++++++++++++++++ x/feegrant/migrations/{v045 => v046}/store.go | 7 +-- .../migrations/{v045 => v046}/store_test.go | 18 +++---- 6 files changed, 62 insertions(+), 23 deletions(-) delete mode 100644 x/feegrant/migrations/v045/keys.go create mode 100644 x/feegrant/migrations/v046/keys.go rename x/feegrant/migrations/{v045 => v046}/store.go (86%) rename x/feegrant/migrations/{v045 => v046}/store_test.go (76%) diff --git a/x/feegrant/keeper/migrations.go b/x/feegrant/keeper/migrations.go index 6b39efaa43d..67c3e489cf0 100644 --- a/x/feegrant/keeper/migrations.go +++ b/x/feegrant/keeper/migrations.go @@ -2,7 +2,7 @@ package keeper import ( sdk "github.com/cosmos/cosmos-sdk/types" - v045 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v045" + v046 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v046" ) // Migrator is a struct for handling in-place store migrations. @@ -17,5 +17,5 @@ func NewMigrator(keeper Keeper) Migrator { // Migrate1to2 migrates from version 1 to 2. func (m Migrator) Migrate1to2(ctx sdk.Context) error { - return v045.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) + return v046.MigrateStore(ctx, m.keeper.storeKey, m.keeper.cdc) } diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 99b03dde062..41284f017a4 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -1,7 +1,6 @@ package feegrant import ( - "fmt" time "time" sdk "github.com/cosmos/cosmos-sdk/types" @@ -39,9 +38,6 @@ var ( // Key format: // - <0x00> func FeeAllowanceKey(granter sdk.AccAddress, grantee sdk.AccAddress) []byte { - fmt.Println("granter", address.MustLengthPrefix(granter.Bytes())) - fmt.Println("grantee", address.MustLengthPrefix(grantee.Bytes())) - fmt.Println("-----------------------------------------------------") return append(FeeAllowancePrefixByGrantee(grantee), address.MustLengthPrefix(granter.Bytes())...) } diff --git a/x/feegrant/migrations/v045/keys.go b/x/feegrant/migrations/v045/keys.go deleted file mode 100644 index 9b32780fd32..00000000000 --- a/x/feegrant/migrations/v045/keys.go +++ /dev/null @@ -1,5 +0,0 @@ -package v045 - -var ( - ModuleName = "feegrant" -) diff --git a/x/feegrant/migrations/v046/keys.go b/x/feegrant/migrations/v046/keys.go new file mode 100644 index 00000000000..818472e6ca6 --- /dev/null +++ b/x/feegrant/migrations/v046/keys.go @@ -0,0 +1,47 @@ +package v046 + +import ( + "time" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/types/address" +) + +var ( + ModuleName = "feegrant" + + // FeeAllowanceKeyPrefix is the set of the kvstore for fee allowance data + // - 0x00: allowance + FeeAllowanceKeyPrefix = []byte{0x00} + + // FeeAllowanceQueueKeyPrefix is the set of the kvstore for fee allowance keys data + // - 0x01: + FeeAllowanceQueueKeyPrefix = []byte{0x01} +) + +// FeeAllowancePrefixQueue is the canonical key to store grant key. +// +// Key format: +// - <0x01> +func FeeAllowancePrefixQueue(exp *time.Time, key []byte) []byte { + // no need of appending len(exp_bytes) here, `FormatTimeBytes` gives const length everytime. + allowanceByExpTimeKey := append(FeeAllowanceQueueKeyPrefix, sdk.FormatTimeBytes(*exp)...) + return append(allowanceByExpTimeKey, key...) +} + +// FeeAllowanceKey is the canonical key to store a grant from granter to grantee +// We store by grantee first to allow searching by everyone who granted to you +// +// Key format: +// - <0x00> +func FeeAllowanceKey(granter sdk.AccAddress, grantee sdk.AccAddress) []byte { + return append(FeeAllowancePrefixByGrantee(grantee), address.MustLengthPrefix(granter.Bytes())...) +} + +// FeeAllowancePrefixByGrantee returns a prefix to scan for all grants to this given address. +// +// Key format: +// - <0x00> +func FeeAllowancePrefixByGrantee(grantee sdk.AccAddress) []byte { + return append(FeeAllowanceKeyPrefix, address.MustLengthPrefix(grantee.Bytes())...) +} diff --git a/x/feegrant/migrations/v045/store.go b/x/feegrant/migrations/v046/store.go similarity index 86% rename from x/feegrant/migrations/v045/store.go rename to x/feegrant/migrations/v046/store.go index 133952b28fa..f3d49312caa 100644 --- a/x/feegrant/migrations/v045/store.go +++ b/x/feegrant/migrations/v046/store.go @@ -1,4 +1,4 @@ -package v045 +package v046 import ( "github.com/cosmos/cosmos-sdk/codec" @@ -9,7 +9,7 @@ import ( ) func addAllowancesByExpTimeQueue(ctx types.Context, store storetypes.KVStore, cdc codec.BinaryCodec) error { - prefixStore := prefix.NewStore(store, feegrant.FeeAllowanceKeyPrefix) + prefixStore := prefix.NewStore(store, FeeAllowanceKeyPrefix) iterator := prefixStore.Iterator(nil, nil) defer iterator.Close() @@ -31,11 +31,12 @@ func addAllowancesByExpTimeQueue(ctx types.Context, store storetypes.KVStore, cd } if exp != nil { + // store key is not changed in 0.46 key := iterator.Key() if exp.Before(ctx.BlockTime()) { prefixStore.Delete(key) } else { - grantByExpTimeQueueKey := feegrant.FeeAllowancePrefixQueue(exp, key) + grantByExpTimeQueueKey := FeeAllowancePrefixQueue(exp, key) store.Set(grantByExpTimeQueueKey, []byte{}) } } diff --git a/x/feegrant/migrations/v045/store_test.go b/x/feegrant/migrations/v046/store_test.go similarity index 76% rename from x/feegrant/migrations/v045/store_test.go rename to x/feegrant/migrations/v046/store_test.go index d176039f289..5d7cb5d9d07 100644 --- a/x/feegrant/migrations/v045/store_test.go +++ b/x/feegrant/migrations/v046/store_test.go @@ -1,4 +1,4 @@ -package v045_test +package v046_test import ( "testing" @@ -9,14 +9,14 @@ import ( "github.com/cosmos/cosmos-sdk/testutil" sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/feegrant" - v045 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v045" + v046 "github.com/cosmos/cosmos-sdk/x/feegrant/migrations/v046" "github.com/stretchr/testify/require" ) func TestMigration(t *testing.T) { encCfg := simapp.MakeTestEncodingConfig() cdc := encCfg.Codec - feegrantKey := sdk.NewKVStoreKey(v045.ModuleName) + feegrantKey := sdk.NewKVStoreKey(v046.ModuleName) ctx := testutil.DefaultContext(feegrantKey, sdk.NewTransientStoreKey("transient_test")) granter1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) grantee1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) @@ -69,15 +69,15 @@ func TestMigration(t *testing.T) { bz, err := cdc.Marshal(&newGrant) require.NoError(t, err) - store.Set(feegrant.FeeAllowanceKey(grant.granter, grant.grantee), bz) + store.Set(v046.FeeAllowanceKey(grant.granter, grant.grantee), bz) } ctx = ctx.WithBlockTime(now.Add(30 * time.Hour)) - require.NoError(t, v045.MigrateStore(ctx, feegrantKey, cdc)) + require.NoError(t, v046.MigrateStore(ctx, feegrantKey, cdc)) store = ctx.KVStore(feegrantKey) - require.NotNil(t, store.Get(feegrant.FeeAllowanceKey(granter1, grantee1))) - require.Nil(t, store.Get(feegrant.FeeAllowanceKey(granter2, grantee2))) - require.NotNil(t, store.Get(feegrant.FeeAllowanceKey(granter1, grantee2))) - require.Nil(t, store.Get(feegrant.FeeAllowanceKey(granter2, grantee1))) + require.NotNil(t, store.Get(v046.FeeAllowanceKey(granter1, grantee1))) + require.Nil(t, store.Get(v046.FeeAllowanceKey(granter2, grantee2))) + require.NotNil(t, store.Get(v046.FeeAllowanceKey(granter1, grantee2))) + require.Nil(t, store.Get(v046.FeeAllowanceKey(granter2, grantee1))) } From d5876296f834b549cfdb0f5fe4c4b6dd97519f13 Mon Sep 17 00:00:00 2001 From: atheeshp <59333759+atheeshp@users.noreply.github.com> Date: Fri, 4 Feb 2022 11:33:39 +0530 Subject: [PATCH 23/26] Update x/feegrant/spec/01_concepts.md Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com> --- x/feegrant/spec/01_concepts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/feegrant/spec/01_concepts.md b/x/feegrant/spec/01_concepts.md index 40b3cc8aa04..16af8279202 100644 --- a/x/feegrant/spec/01_concepts.md +++ b/x/feegrant/spec/01_concepts.md @@ -79,4 +79,4 @@ In order to prevent DoS attacks, using a filtered `x/feegrant` incurs gas. The S ## Pruning -A queue in the state maintained with the prefix of expiration of the grants and checks them with the current block time for every block to prune. \ No newline at end of file +A queue in the state maintained with the prefix of expiration of the grants and checks them on EndBlock with the current block time for every block to prune. \ No newline at end of file From 1133b59b9fdb17f6229121ee316b770cb0f88a7d Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 4 Feb 2022 11:38:42 +0530 Subject: [PATCH 24/26] review changes --- x/feegrant/keeper/grpc_query.go | 3 ++- x/feegrant/key.go | 16 ++++++++-------- x/feegrant/key_test.go | 2 +- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/x/feegrant/keeper/grpc_query.go b/x/feegrant/keeper/grpc_query.go index 0a228c9dcf1..027eba648aa 100644 --- a/x/feegrant/keeper/grpc_query.go +++ b/x/feegrant/keeper/grpc_query.go @@ -114,7 +114,8 @@ func (q Keeper) AllowancesByGranter(c context.Context, req *feegrant.QueryAllowa 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 } diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 41284f017a4..8033599fd28 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -71,14 +71,14 @@ func AllowanceByExpTimeKey(exp *time.Time) []byte { // Note: do not send the key with store prefix, remove the store prefix (first byte) while sending. func ParseAddressesFromFeeAllowanceKey(key []byte) (granter, grantee sdk.AccAddress) { // key is of format: - // - kv.AssertKeyAtLeastLength(key, 1) - granteeAddrLen := key[0] // remove prefix key - kv.AssertKeyAtLeastLength(key, 1+int(granteeAddrLen)) - grantee = sdk.AccAddress(key[1 : 1+int(granteeAddrLen)]) - granterAddrLen := int(key[1+granteeAddrLen]) - kv.AssertKeyAtLeastLength(key, 2+int(granteeAddrLen)+int(granterAddrLen)) - granter = sdk.AccAddress(key[2+granterAddrLen : 2+int(granteeAddrLen)+int(granterAddrLen)]) + // 0x00 + kv.AssertKeyAtLeastLength(key, 2) + granteeAddrLen := key[1] // remove prefix key + kv.AssertKeyAtLeastLength(key, 2+int(granteeAddrLen)) + grantee = sdk.AccAddress(key[2 : 2+int(granteeAddrLen)]) + granterAddrLen := int(key[2+granteeAddrLen]) + kv.AssertKeyAtLeastLength(key, 3+int(granteeAddrLen)+int(granterAddrLen)) + granter = sdk.AccAddress(key[3+granterAddrLen : 3+int(granteeAddrLen)+int(granterAddrLen)]) return granter, grantee } diff --git a/x/feegrant/key_test.go b/x/feegrant/key_test.go index 01972cc2643..75a7964bec3 100644 --- a/x/feegrant/key_test.go +++ b/x/feegrant/key_test.go @@ -19,7 +19,7 @@ func TestMarshalAndUnmarshalFeegrantKey(t *testing.T) { require.Len(t, key, len(grantee.Bytes())+len(granter.Bytes())+3) require.Equal(t, feegrant.FeeAllowancePrefixByGrantee(grantee), key[:len(grantee.Bytes())+2]) - g1, g2 := feegrant.ParseAddressesFromFeeAllowanceKey(key[1:]) + g1, g2 := feegrant.ParseAddressesFromFeeAllowanceKey(key) require.Equal(t, granter, g1) require.Equal(t, grantee, g2) } From e9e1be7c5a502cc3abe7d21e2f6fbb25feffd5aa Mon Sep 17 00:00:00 2001 From: atheesh Date: Fri, 4 Feb 2022 11:52:04 +0530 Subject: [PATCH 25/26] update comment --- x/feegrant/key.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/feegrant/key.go b/x/feegrant/key.go index 8033599fd28..3d2fdb56336 100644 --- a/x/feegrant/key.go +++ b/x/feegrant/key.go @@ -68,7 +68,6 @@ func AllowanceByExpTimeKey(exp *time.Time) []byte { } // ParseAddressesFromFeeAllowanceKey exrtacts and returns the granter, grantee from the given key. -// Note: do not send the key with store prefix, remove the store prefix (first byte) while sending. func ParseAddressesFromFeeAllowanceKey(key []byte) (granter, grantee sdk.AccAddress) { // key is of format: // 0x00 From 4504b50c9226b7a5e455460e0ad73eb14ed9c611 Mon Sep 17 00:00:00 2001 From: atheesh Date: Mon, 7 Feb 2022 14:03:27 +0530 Subject: [PATCH 26/26] update change log --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index deaaa63f598..c5ea2594f9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -200,6 +200,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