Skip to content

Commit

Permalink
refactor(feegrant): migrate to use env var (#19450)
Browse files Browse the repository at this point in the history
  • Loading branch information
tac0turtle authored Feb 20, 2024
1 parent b050ec2 commit 52106a6
Show file tree
Hide file tree
Showing 14 changed files with 77 additions and 77 deletions.
2 changes: 1 addition & 1 deletion simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ func NewSimApp(
appCodec, legacyAmino, app.StakingKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

app.FeeGrantKeeper = feegrantkeeper.NewKeeper(appCodec, runtime.NewKVStoreService(keys[feegrant.StoreKey]), app.AuthKeeper)
app.FeeGrantKeeper = feegrantkeeper.NewKeeper(runtime.NewEnvironment(runtime.NewKVStoreService(keys[feegrant.StoreKey]), logger), appCodec, app.AuthKeeper)

// register the staking hooks
// NOTE: stakingKeeper above is passed by reference, so that it will contain these hooks
Expand Down
4 changes: 4 additions & 0 deletions x/feegrant/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Ref: https://keepachangelog.com/en/1.0.0/

## [Unreleased]

### API Breaking Changes

* [#19450](https://github.com/cosmos/cosmos-sdk/pull/19450) Migrate module to use `appmodule.Environment` instead of passing individual services.

### Consens Breaking Changes

* [#19188](https://github.com/cosmos/cosmos-sdk/pull/19188) Remove creation of `BaseAccount` when sending a message to an account that does not exist
Expand Down
3 changes: 2 additions & 1 deletion x/feegrant/basic_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ var _ FeeAllowanceI = (*BasicAllowance)(nil)
// If remove is true (regardless of the error), the FeeAllowance will be deleted from storage
// (eg. when it is used up). (See call to RevokeAllowance in Keeper.UseGrantedFees)
func (a *BasicAllowance) Accept(ctx context.Context, fee sdk.Coins, _ []sdk.Msg) (bool, error) {
if a.Expiration != nil && a.Expiration.Before(sdk.UnwrapSDKContext(ctx).HeaderInfo().Time) {
sdkCtx := sdk.UnwrapSDKContext(ctx)
if a.Expiration != nil && a.Expiration.Before(sdkCtx.HeaderInfo().Time) {
return true, errorsmod.Wrap(ErrFeeLimitExpired, "basic allowance")
}

Expand Down
13 changes: 7 additions & 6 deletions x/feegrant/filtered_fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (a *AllowedMsgAllowance) SetAllowance(allowance FeeAllowanceI) error {

// Accept method checks for the filtered messages has valid expiry
func (a *AllowedMsgAllowance) Accept(ctx context.Context, fee sdk.Coins, msgs []sdk.Msg) (bool, error) {
if !a.allMsgTypesAllowed(sdk.UnwrapSDKContext(ctx), msgs) {
if !a.allMsgTypesAllowed(ctx, msgs) {
return false, errorsmod.Wrap(ErrMessageNotAllowed, "message does not exist in allowed messages")
}

Expand All @@ -88,21 +88,22 @@ func (a *AllowedMsgAllowance) Accept(ctx context.Context, fee sdk.Coins, msgs []
return remove, err
}

func (a *AllowedMsgAllowance) allowedMsgsToMap(ctx sdk.Context) map[string]bool {
func (a *AllowedMsgAllowance) allowedMsgsToMap(ctx context.Context) map[string]bool {
msgsMap := make(map[string]bool, len(a.AllowedMessages))
sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, msg := range a.AllowedMessages {
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
msgsMap[msg] = true
}

return msgsMap
}

func (a *AllowedMsgAllowance) allMsgTypesAllowed(ctx sdk.Context, msgs []sdk.Msg) bool {
func (a *AllowedMsgAllowance) allMsgTypesAllowed(ctx context.Context, msgs []sdk.Msg) bool {
msgsMap := a.allowedMsgsToMap(ctx)

sdkCtx := sdk.UnwrapSDKContext(ctx)
for _, msg := range msgs {
ctx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
sdkCtx.GasMeter().ConsumeGas(gasCostPerIteration, "check msg")
if !msgsMap[sdk.MsgTypeURL(msg)] {
return false
}
Expand Down
3 changes: 2 additions & 1 deletion x/feegrant/keeper/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/golang/mock/gomock"
"gotest.tools/v3/assert"

"cosmossdk.io/log"
"cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
authtypes "cosmossdk.io/x/auth/types"
Expand Down Expand Up @@ -50,7 +51,7 @@ func initFixture(t *testing.T) *genesisFixture {

return &genesisFixture{
ctx: testCtx.Ctx,
feegrantKeeper: keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), accountKeeper),
feegrantKeeper: keeper.NewKeeper(runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), encCfg.Codec, accountKeeper),
accountKeeper: accountKeeper,
}
}
Expand Down
81 changes: 35 additions & 46 deletions x/feegrant/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import (
"time"

"cosmossdk.io/collections"
"cosmossdk.io/core/store"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/core/event"
errorsmod "cosmossdk.io/errors"
"cosmossdk.io/log"
"cosmossdk.io/x/auth/ante"
Expand All @@ -20,10 +21,10 @@ import (
// Keeper manages state of all fee grants, as well as calculating approval.
// It must have a codec with all available allowances registered.
type Keeper struct {
cdc codec.BinaryCodec
storeService store.KVStoreService
authKeeper feegrant.AccountKeeper
Schema collections.Schema
cdc codec.BinaryCodec
environment appmodule.Environment
authKeeper feegrant.AccountKeeper
Schema collections.Schema
// FeeAllowance key: grantee+granter | value: Grant
FeeAllowance collections.Map[collections.Pair[sdk.AccAddress, sdk.AccAddress], feegrant.Grant]
// FeeAllowanceQueue key: expiration time+grantee+granter | value: bool
Expand All @@ -33,13 +34,13 @@ type Keeper struct {
var _ ante.FeegrantKeeper = &Keeper{}

// NewKeeper creates a feegrant Keeper
func NewKeeper(cdc codec.BinaryCodec, storeService store.KVStoreService, ak feegrant.AccountKeeper) Keeper {
sb := collections.NewSchemaBuilder(storeService)
func NewKeeper(env appmodule.Environment, cdc codec.BinaryCodec, ak feegrant.AccountKeeper) Keeper {
sb := collections.NewSchemaBuilder(env.KVStoreService)

return Keeper{
cdc: cdc,
storeService: storeService,
authKeeper: ak,
cdc: cdc,
environment: env,
authKeeper: ak,
FeeAllowance: collections.NewMap(
sb,
feegrant.FeeAllowanceKeyPrefix,
Expand Down Expand Up @@ -75,8 +76,8 @@ func (k Keeper) GrantAllowance(ctx context.Context, granter, grantee sdk.AccAddr
}

// expiration shouldn't be in the past.
sdkCtx := sdk.UnwrapSDKContext(ctx)
now := sdkCtx.HeaderInfo().Time

now := k.environment.HeaderService.GetHeaderInfo(ctx).Time
if exp != nil && exp.Before(now) {
return errorsmod.Wrap(sdkerrors.ErrInvalidRequest, "expiration is before current block time")
}
Expand Down Expand Up @@ -116,15 +117,11 @@ func (k Keeper) GrantAllowance(ctx context.Context, granter, grantee sdk.AccAddr
return err
}

sdkCtx.EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypeSetFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyGranter, grant.Granter),
sdk.NewAttribute(feegrant.AttributeKeyGrantee, grant.Grantee),
),
return k.environment.EventService.EventManager(ctx).EmitKV(
feegrant.EventTypeSetFeeGrant,
event.NewAttribute(feegrant.AttributeKeyGranter, grant.Granter),
event.NewAttribute(feegrant.AttributeKeyGrantee, grant.Grantee),
)

return nil
}

// UpdateAllowance updates the existing grant.
Expand Down Expand Up @@ -152,15 +149,11 @@ func (k Keeper) UpdateAllowance(ctx context.Context, granter, grantee sdk.AccAdd
return err
}

sdk.UnwrapSDKContext(ctx).EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypeUpdateFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyGranter, grant.Granter),
sdk.NewAttribute(feegrant.AttributeKeyGrantee, grant.Grantee),
),
return k.environment.EventService.EventManager(ctx).EmitKV(
feegrant.EventTypeUpdateFeeGrant,
event.NewAttribute(feegrant.AttributeKeyGranter, grant.Granter),
event.NewAttribute(feegrant.AttributeKeyGrantee, grant.Grantee),
)

return nil
}

// revokeAllowance removes an existing grant
Expand Down Expand Up @@ -194,14 +187,11 @@ func (k Keeper) revokeAllowance(ctx context.Context, granter, grantee sdk.AccAdd
return err
}

sdk.UnwrapSDKContext(ctx).EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypeRevokeFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyGranter, granterStr),
sdk.NewAttribute(feegrant.AttributeKeyGrantee, granteeStr),
),
return k.environment.EventService.EventManager(ctx).EmitKV(
feegrant.EventTypeRevokeFeeGrant,
event.NewAttribute(feegrant.AttributeKeyGranter, granterStr),
event.NewAttribute(feegrant.AttributeKeyGrantee, granteeStr),
)
return nil
}

// GetAllowance returns the allowance between the granter and grantee.
Expand Down Expand Up @@ -245,26 +235,25 @@ func (k Keeper) UseGrantedFees(ctx context.Context, granter, grantee sdk.AccAddr
if remove && err == nil {
// Ignoring the `revokeFeeAllowance` error, because the user has enough grants to perform this transaction.
_ = k.revokeAllowance(ctx, granter, grantee)
emitUseGrantEvent(ctx, granterStr, granteeStr)

return nil
return k.emitUseGrantEvent(ctx, granterStr, granteeStr)
}
if err != nil {
return err
}
emitUseGrantEvent(ctx, granterStr, granteeStr)
if err := k.emitUseGrantEvent(ctx, granterStr, granteeStr); err != nil {
return err
}

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

func emitUseGrantEvent(ctx context.Context, granter, grantee string) {
sdk.UnwrapSDKContext(ctx).EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypeUseFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyGranter, granter),
sdk.NewAttribute(feegrant.AttributeKeyGrantee, grantee),
),
func (k *Keeper) emitUseGrantEvent(ctx context.Context, granter, grantee string) error {
return k.environment.EventService.EventManager(ctx).EmitKV(
feegrant.EventTypeUseFeeGrant,
event.NewAttribute(feegrant.AttributeKeyGranter, granter),
event.NewAttribute(feegrant.AttributeKeyGrantee, grantee),
)
}

Expand Down Expand Up @@ -309,7 +298,7 @@ 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, limit int) error {
exp := sdk.UnwrapSDKContext(ctx).HeaderInfo().Time
exp := k.environment.HeaderService.GetHeaderInfo(ctx).Time
rng := collections.NewPrefixUntilTripleRange[time.Time, sdk.AccAddress, sdk.AccAddress](exp)
count := 0

Expand Down
3 changes: 2 additions & 1 deletion x/feegrant/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/suite"

"cosmossdk.io/core/header"
"cosmossdk.io/log"
sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
authtypes "cosmossdk.io/x/auth/types"
Expand Down Expand Up @@ -60,7 +61,7 @@ func (suite *KeeperTestSuite) SetupTest() {
suite.encodedAddrs = append(suite.encodedAddrs, str)
}

suite.feegrantKeeper = keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), suite.accountKeeper)
suite.feegrantKeeper = keeper.NewKeeper(runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger()), encCfg.Codec, suite.accountKeeper)
suite.ctx = testCtx.Ctx
suite.msgSrvr = keeper.NewMsgServerImpl(suite.feegrantKeeper)
suite.atom = sdk.NewCoins(sdk.NewCoin("atom", sdkmath.NewInt(555)))
Expand Down
2 changes: 1 addition & 1 deletion x/feegrant/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,5 @@ func NewMigrator(keeper Keeper) Migrator {

// Migrate1to2 migrates from version 1 to 2.
func (m Migrator) Migrate1to2(ctx context.Context) error {
return v2.MigrateStore(ctx, m.keeper.storeService, m.keeper.cdc)
return v2.MigrateStore(ctx, m.keeper.environment, m.keeper.cdc)
}
15 changes: 7 additions & 8 deletions x/feegrant/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ import (
"context"
"strings"

"cosmossdk.io/core/event"
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 @@ -94,13 +94,12 @@ func (k msgServer) PruneAllowances(ctx context.Context, req *feegrant.MsgPruneAl
return nil, err
}

sdkCtx := sdk.UnwrapSDKContext(ctx)
sdkCtx.EventManager().EmitEvent(
sdk.NewEvent(
feegrant.EventTypePruneFeeGrant,
sdk.NewAttribute(feegrant.AttributeKeyPruner, req.Pruner),
),
)
if err := k.environment.EventService.EventManager(ctx).EmitKV(
feegrant.EventTypePruneFeeGrant,
event.NewAttribute(feegrant.AttributeKeyPruner, req.Pruner),
); err != nil {
return nil, err
}

return &feegrant.MsgPruneAllowancesResponse{}, nil
}
12 changes: 6 additions & 6 deletions x/feegrant/migrations/v2/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ package v2
import (
"context"

"cosmossdk.io/core/appmodule"
"cosmossdk.io/core/store"
"cosmossdk.io/store/prefix"
"cosmossdk.io/x/feegrant"

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

func addAllowancesByExpTimeQueue(ctx context.Context, store store.KVStore, cdc codec.BinaryCodec) error {
func addAllowancesByExpTimeQueue(ctx context.Context, env appmodule.Environment, store store.KVStore, cdc codec.BinaryCodec) error {
prefixStore := prefix.NewStore(runtime.KVStoreAdapter(store), FeeAllowanceKeyPrefix)
iterator := prefixStore.Iterator(nil, nil)
defer iterator.Close()
Expand All @@ -37,7 +37,7 @@ func addAllowancesByExpTimeQueue(ctx context.Context, store store.KVStore, cdc c
if exp != nil {
// store key is not changed in 0.46
key := iterator.Key()
if exp.Before(types.UnwrapSDKContext(ctx).HeaderInfo().Time) {
if exp.Before(env.HeaderService.GetHeaderInfo(ctx).Time) {
prefixStore.Delete(key)
} else {
grantByExpTimeQueueKey := FeeAllowancePrefixQueue(exp, key)
Expand All @@ -52,7 +52,7 @@ func addAllowancesByExpTimeQueue(ctx context.Context, store store.KVStore, cdc c
return nil
}

func MigrateStore(ctx context.Context, storeService store.KVStoreService, cdc codec.BinaryCodec) error {
store := storeService.OpenKVStore(ctx)
return addAllowancesByExpTimeQueue(ctx, store, cdc)
func MigrateStore(ctx context.Context, env appmodule.Environment, cdc codec.BinaryCodec) error {
store := env.KVStoreService.OpenKVStore(ctx)
return addAllowancesByExpTimeQueue(ctx, env, store, cdc)
}
3 changes: 2 additions & 1 deletion x/feegrant/migrations/v2/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/core/header"
"cosmossdk.io/log"
sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
"cosmossdk.io/x/feegrant"
Expand Down Expand Up @@ -87,7 +88,7 @@ func TestMigration(t *testing.T) {
}

ctx = ctx.WithHeaderInfo(header.Info{Time: now.Add(30 * time.Hour)})
require.NoError(t, v2.MigrateStore(ctx, runtime.NewKVStoreService(feegrantKey), cdc))
require.NoError(t, v2.MigrateStore(ctx, runtime.NewEnvironment(runtime.NewKVStoreService(feegrantKey), log.NewNopLogger()), cdc))
store = ctx.KVStore(feegrantKey)

require.NotNil(t, store.Get(v2.FeeAllowanceKey(granter1, grantee1)))
Expand Down
5 changes: 4 additions & 1 deletion x/feegrant/module/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/require"

"cosmossdk.io/core/header"
"cosmossdk.io/log"
sdkmath "cosmossdk.io/math"
storetypes "cosmossdk.io/store/types"
authtypes "cosmossdk.io/x/auth/types"
Expand Down Expand Up @@ -47,7 +48,9 @@ func TestFeegrantPruning(t *testing.T) {
ac := address.NewBech32Codec("cosmos")
accountKeeper.EXPECT().AddressCodec().Return(ac).AnyTimes()

feegrantKeeper := keeper.NewKeeper(encCfg.Codec, runtime.NewKVStoreService(key), accountKeeper)
env := runtime.NewEnvironment(runtime.NewKVStoreService(key), log.NewNopLogger())

feegrantKeeper := keeper.NewKeeper(env, encCfg.Codec, accountKeeper)

err := feegrantKeeper.GrantAllowance(
testCtx.Ctx,
Expand Down
5 changes: 2 additions & 3 deletions x/feegrant/module/depinject.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package module
import (
modulev1 "cosmossdk.io/api/cosmos/feegrant/module/v1"
"cosmossdk.io/core/appmodule"
"cosmossdk.io/core/store"
"cosmossdk.io/depinject"
"cosmossdk.io/depinject/appconfig"
"cosmossdk.io/x/feegrant"
Expand All @@ -30,15 +29,15 @@ func init() {
type FeegrantInputs struct {
depinject.In

StoreService store.KVStoreService
Environment appmodule.Environment
Cdc codec.Codec
AccountKeeper feegrant.AccountKeeper
BankKeeper feegrant.BankKeeper
Registry cdctypes.InterfaceRegistry
}

func ProvideModule(in FeegrantInputs) (keeper.Keeper, appmodule.AppModule) {
k := keeper.NewKeeper(in.Cdc, in.StoreService, in.AccountKeeper)
k := keeper.NewKeeper(in.Environment, in.Cdc, in.AccountKeeper)
m := NewAppModule(in.Cdc, in.AccountKeeper, in.BankKeeper, k, in.Registry)
return k, m
}
Expand Down
Loading

0 comments on commit 52106a6

Please sign in to comment.