diff --git a/x/feegrant/CHANGELOG.md b/x/feegrant/CHANGELOG.md index 995dde2bf2bd..22d484a015a7 100644 --- a/x/feegrant/CHANGELOG.md +++ b/x/feegrant/CHANGELOG.md @@ -46,3 +46,4 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [#15347](https://github.com/cosmos/cosmos-sdk/pull/15347) `ValidateBasic` is treated as a no op now with with acceptance of RFC001 * [#17869](https://github.com/cosmos/cosmos-sdk/pull/17869) `NewGrant`, `NewMsgGrantAllowance` & `NewMsgRevokeAllowance` takes strings instead of `sdk.AccAddress` * [#16535](https://github.com/cosmos/cosmos-sdk/pull/16535) Use collections for `FeeAllowance`, `FeeAllowanceQueue`. +* [#18815](https://github.com/cosmos/cosmos-sdk/pull/18815) Add the implementation of the `UpdatePeriodReset` interface to update the value of the `PeriodReset` field. \ No newline at end of file diff --git a/x/feegrant/basic_fee.go b/x/feegrant/basic_fee.go index a43929f3227c..e9241ba1ae52 100644 --- a/x/feegrant/basic_fee.go +++ b/x/feegrant/basic_fee.go @@ -62,3 +62,6 @@ func (a BasicAllowance) ValidateBasic() error { func (a BasicAllowance) ExpiresAt() (*time.Time, error) { return a.Expiration, nil } + +// UpdatePeriodReset BasicAllowance does not update "PeriodReset" +func (a BasicAllowance) UpdatePeriodReset(validTime time.Time) error { return nil } diff --git a/x/feegrant/basic_fee_test.go b/x/feegrant/basic_fee_test.go index 837e1951e66c..6eb141d3676e 100644 --- a/x/feegrant/basic_fee_test.go +++ b/x/feegrant/basic_fee_test.go @@ -130,7 +130,10 @@ func TestBasicFeeValidAllow(t *testing.T) { for name, stc := range cases { tc := stc // to make scopelint happy t.Run(name, func(t *testing.T) { - err := tc.allowance.ValidateBasic() + err := tc.allowance.UpdatePeriodReset(tc.blockTime) + require.NoError(t, err) + + err = tc.allowance.ValidateBasic() require.NoError(t, err) ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: tc.blockTime}) diff --git a/x/feegrant/client/cli/tx.go b/x/feegrant/client/cli/tx.go index 99561be03625..a2ab385a27af 100644 --- a/x/feegrant/client/cli/tx.go +++ b/x/feegrant/client/cli/tx.go @@ -151,7 +151,6 @@ Examples: periodic := feegrant.PeriodicAllowance{ Basic: basic, Period: getPeriod(periodClock), - PeriodReset: getPeriodReset(periodClock), PeriodSpendLimit: periodLimit, PeriodCanSpend: periodLimit, } diff --git a/x/feegrant/fees.go b/x/feegrant/fees.go index fcaa8fdf9fcc..d229512f9383 100644 --- a/x/feegrant/fees.go +++ b/x/feegrant/fees.go @@ -7,7 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" ) -// FeeAllowance implementations are tied to a given fee delegator and delegatee, +// FeeAllowanceI implementations are tied to a given fee delegator and delegatee, // and are used to enforce feegrant limits. type FeeAllowanceI interface { // Accept can use fee payment requested as well as timestamp of the current block @@ -28,4 +28,7 @@ type FeeAllowanceI interface { // ExpiresAt returns the expiry time of the allowance. ExpiresAt() (*time.Time, error) + + // UpdatePeriodReset update "PeriodReset" value by valid time + UpdatePeriodReset(validTime time.Time) error } diff --git a/x/feegrant/filtered_fee.go b/x/feegrant/filtered_fee.go index c6ed2fed40b4..75cb208e8bd7 100644 --- a/x/feegrant/filtered_fee.go +++ b/x/feegrant/filtered_fee.go @@ -30,7 +30,7 @@ func (a *AllowedMsgAllowance) UnpackInterfaces(unpacker types.AnyUnpacker) error return unpacker.UnpackAny(a.Allowance, &allowance) } -// NewAllowedMsgFeeAllowance creates new filtered fee allowance. +// NewAllowedMsgAllowance creates new filtered fee allowance. func NewAllowedMsgAllowance(allowance FeeAllowanceI, allowedMsgs []string) (*AllowedMsgAllowance, error) { msg, ok := allowance.(proto.Message) if !ok { @@ -136,3 +136,12 @@ func (a *AllowedMsgAllowance) ExpiresAt() (*time.Time, error) { } return allowance.ExpiresAt() } + +// UpdatePeriodReset update "PeriodReset" of the AllowedMsgAllowance. +func (a *AllowedMsgAllowance) UpdatePeriodReset(validTime time.Time) error { + allowance, err := a.GetAllowance() + if err != nil { + return err + } + return allowance.UpdatePeriodReset(validTime) +} diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index 1a2366a4e668..08e8f4510fec 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -106,6 +106,11 @@ func (k Keeper) GrantAllowance(ctx context.Context, granter, grantee sdk.AccAddr return err } + err = feeAllowance.UpdatePeriodReset(sdkCtx.HeaderInfo().Time) + if err != nil { + return err + } + grant, err := feegrant.NewGrant(granterStr, granteeStr, feeAllowance) if err != nil { return err diff --git a/x/feegrant/keeper/msg_server_test.go b/x/feegrant/keeper/msg_server_test.go index ecda3d89c157..2f5532717f0c 100644 --- a/x/feegrant/keeper/msg_server_test.go +++ b/x/feegrant/keeper/msg_server_test.go @@ -160,6 +160,28 @@ func (suite *KeeperTestSuite) TestGrantAllowance() { false, "", }, + { + "valid: with period reset", + func() *feegrant.MsgGrantAllowance { + any, err := codectypes.NewAnyWithValue(&feegrant.PeriodicAllowance{ + Basic: feegrant.BasicAllowance{ + SpendLimit: suite.atom, + Expiration: &oneYear, + }, + Period: time.Hour, + PeriodSpendLimit: suite.atom, + PeriodReset: oneYear, + }) + suite.Require().NoError(err) + return &feegrant.MsgGrantAllowance{ + Granter: suite.encodedAddrs[1], + Grantee: suite.encodedAddrs[2], + Allowance: any, + } + }, + false, + "", + }, { "error: fee allowance exists", func() *feegrant.MsgGrantAllowance { diff --git a/x/feegrant/periodic_fee.go b/x/feegrant/periodic_fee.go index 1b12a63ef961..2b623a5d680e 100644 --- a/x/feegrant/periodic_fee.go +++ b/x/feegrant/periodic_fee.go @@ -70,9 +70,9 @@ func (a *PeriodicAllowance) tryResetPeriod(blockTime time.Time) { // If we are within the period, step from expiration (eg. if you always do one tx per day, it will always reset the same time) // If we are more then one period out (eg. no activity in a week), reset is one period from this time - a.PeriodReset = a.PeriodReset.Add(a.Period) + _ = a.UpdatePeriodReset(a.PeriodReset) if blockTime.After(a.PeriodReset) { - a.PeriodReset = blockTime.Add(a.Period) + _ = a.UpdatePeriodReset(blockTime) } } @@ -113,3 +113,9 @@ func (a PeriodicAllowance) ValidateBasic() error { func (a PeriodicAllowance) ExpiresAt() (*time.Time, error) { return a.Basic.ExpiresAt() } + +// UpdatePeriodReset update "PeriodReset" of the PeriodicAllowance. +func (a *PeriodicAllowance) UpdatePeriodReset(validTime time.Time) error { + a.PeriodReset = validTime.Add(a.Period) + return nil +} diff --git a/x/feegrant/periodic_fee_test.go b/x/feegrant/periodic_fee_test.go index 1b7f942f72fc..f63ac9ef52b8 100644 --- a/x/feegrant/periodic_fee_test.go +++ b/x/feegrant/periodic_fee_test.go @@ -34,15 +34,16 @@ func TestPeriodicFeeValidAllow(t *testing.T) { tenMinutes := time.Duration(10) * time.Minute cases := map[string]struct { - allow feegrant.PeriodicAllowance - fee sdk.Coins - blockTime time.Time - valid bool // all other checks are ignored if valid=false - accept bool - remove bool - remains sdk.Coins - remainsPeriod sdk.Coins - periodReset time.Time + allow feegrant.PeriodicAllowance + fee sdk.Coins + blockTime time.Time + valid bool // all other checks are ignored if valid=false + accept bool + remove bool + remains sdk.Coins + remainsPeriod sdk.Coins + periodReset time.Time + updatePeriodReset bool }{ "empty": { allow: feegrant.PeriodicAllowance{}, @@ -185,6 +186,20 @@ func TestPeriodicFeeValidAllow(t *testing.T) { accept: false, remove: true, }, + "test update PeriodReset ": { + allow: feegrant.PeriodicAllowance{ + Period: tenMinutes, + PeriodSpendLimit: smallAtom, + PeriodReset: now.Add(30 * time.Minute), + }, + blockTime: now, + valid: true, + accept: true, + remove: false, + remainsPeriod: emptyCoins, + periodReset: now.Add(10 * time.Minute), + updatePeriodReset: true, + }, } for name, stc := range cases { @@ -197,6 +212,11 @@ func TestPeriodicFeeValidAllow(t *testing.T) { } require.NoError(t, err) + if tc.updatePeriodReset { + err = tc.allow.UpdatePeriodReset(tc.blockTime) + require.NoError(t, err) + } + ctx := testCtx.Ctx.WithHeaderInfo(header.Info{Time: tc.blockTime}) // now try to deduct remove, err := tc.allow.Accept(ctx, tc.fee, []sdk.Msg{})