From cab40b4c7a007f17ac902590a1789ebdf66a0987 Mon Sep 17 00:00:00 2001
From: Ryan Christoffersen <12519942+ryanchristo@users.noreply.github.com>
Date: Mon, 7 Jun 2021 08:02:49 -0700
Subject: [PATCH] fix: feegrant grant period not resetting (#9450)

* fix grant period reset

* add period reset test

* consolidate condition

Co-authored-by: ryanchrypto <12519942+ryanchrypto@users.noreply.github.com>
Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
---
 x/feegrant/periodic_fee.go      |  7 ++++---
 x/feegrant/periodic_fee_test.go | 22 +++++++++++++++++-----
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/x/feegrant/periodic_fee.go b/x/feegrant/periodic_fee.go
index 63c9e7485..c6fbc94b6 100644
--- a/x/feegrant/periodic_fee.go
+++ b/x/feegrant/periodic_fee.go
@@ -49,7 +49,7 @@ 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
@@ -57,8 +57,9 @@ 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
index a09459cb7..f09640b5f 100644
--- 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{