fix: feegrant grant period not resetting (#9450)

* fix grant period reset * add period reset test * consolidate condition Co-authored-by: ryanchrypto <[email protected]> Co-authored-by: Tyler <[email protected]> Co-authored-by: Amaury <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
cosmos · Jun 7, 2021 · cab40b4 · cab40b4
1 parent a55b6ed
commit cab40b4
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 8 deletions.
diff --git a/x/feegrant/periodic_fee.go b/x/feegrant/periodic_fee.go
@@ -49,16 +49,17 @@ func (a *PeriodicAllowance) Accept(ctx sdk.Context, fee sdk.Coins, _ []sdk.Msg)
 
 // tryResetPeriod will check if the PeriodReset has been hit. If not, it is a no-op.
 // If we hit the reset period, it will top up the PeriodCanSpend amount to
-// min(PeriodicSpendLimit, a.Basic.SpendLimit) so it is never more than the maximum allowed.
+// min(PeriodSpendLimit, Basic.SpendLimit) so it is never more than the maximum allowed.
 // It will also update the PeriodReset. If we are within one Period, it will update from the
 // last PeriodReset (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 the execution of this method
 func (a *PeriodicAllowance) tryResetPeriod(blockTime time.Time) {
 	if blockTime.Before(a.PeriodReset) {
 		return
 	}
-	// set CanSpend to the lesser of PeriodSpendLimit and the TotalLimit
-	if _, isNeg := a.Basic.SpendLimit.SafeSub(a.PeriodSpendLimit); isNeg {
+
+	// set PeriodCanSpend to the lesser of Basic.SpendLimit and PeriodSpendLimit
+	if _, isNeg := a.Basic.SpendLimit.SafeSub(a.PeriodSpendLimit); isNeg && !a.Basic.SpendLimit.Empty() {
 		a.PeriodCanSpend = a.Basic.SpendLimit
 	} else {
 		a.PeriodCanSpend = a.PeriodSpendLimit

diff --git a/x/feegrant/periodic_fee_test.go b/x/feegrant/periodic_fee_test.go
@@ -31,11 +31,10 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 	tenMinutes := time.Duration(10) * time.Minute
 
 	cases := map[string]struct {
-		allow feegrant.PeriodicAllowance
-		// all other checks are ignored if valid=false
+		allow         feegrant.PeriodicAllowance
 		fee           sdk.Coins
 		blockTime     time.Time
-		valid         bool
+		valid         bool // all other checks are ignored if valid=false
 		accept        bool
 		remove        bool
 		remains       sdk.Coins
@@ -115,7 +114,7 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			remove:        false,
 			remainsPeriod: nil,
 			remains:       smallAtom,
-			periodReset:   oneHour.Add(10 * time.Minute), // one step from last reset, not now
+			periodReset:   oneHour.Add(tenMinutes), // one step from last reset, not now
 		},
 		"step limited by global allowance": {
 			allow: feegrant.PeriodicAllowance{
@@ -134,7 +133,20 @@ func TestPeriodicFeeValidAllow(t *testing.T) {
 			remove:        false,
 			remainsPeriod: smallAtom.Sub(oneAtom),
 			remains:       smallAtom.Sub(oneAtom),
-			periodReset:   oneHour.Add(10 * time.Minute), // one step from last reset, not now
+			periodReset:   oneHour.Add(tenMinutes), // one step from last reset, not now
+		},
+		"period reset no spend limit": {
+			allow: feegrant.PeriodicAllowance{
+				Period:           tenMinutes,
+				PeriodReset:      now,
+				PeriodSpendLimit: atom,
+			},
+			valid:       true,
+			fee:         atom,
+			blockTime:   oneHour,
+			accept:      true,
+			remove:      false,
+			periodReset: oneHour.Add(tenMinutes), // one step from last reset, not now
 		},
 		"expired": {
 			allow: feegrant.PeriodicAllowance{