refactor(x/feegrant): Change the periodReset assignment time (#18815)

Co-authored-by: Tim <[email protected]>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>

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.

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 }

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})

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,
 				}

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
 }

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)
+}

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

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 {

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
+}

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{})