fix: (x/feegrant) expiration when overwriting an existing grant (cosm…

…os#11845)

## Description
This PR couple of bugs with expiration not being updated when we overwrite an existing grant with different expiry.

**Bug 1**
`grantee`, `granter` passed wrongly to the `getGrant` method here:
https://github.com/cosmos/cosmos-sdk/blob/a187a1c321be6b57d98c194eca0d77febf3f1488/x/feegrant/keeper/keeper.go#L55

`grantee` is passed at `granter`'s place and vice versa
https://github.com/cosmos/cosmos-sdk/blob/a187a1c321be6b57d98c194eca0d77febf3f1488/x/feegrant/keeper/keeper.go#L181-L196

**Bug 2**
The following `if` condition never gets satisfied, `getGrant` returns `err`, `grant` with two possible out comes which are `nil`, `grant` or `err`, `nil` 
https://github.com/cosmos/cosmos-sdk/blob/a187a1c321be6b57d98c194eca0d77febf3f1488/x/feegrant/keeper/keeper.go#L56

and also there is not only one possible error from `getGrant` with `fee-grant not found` and also one another possible error is unmarshal error, when there is an unmarshal error we should return the error.



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

x/feegrant/client/testutil/suite.go

@@ -663,7 +663,7 @@ func (s *IntegrationTestSuite) TestNewCmdRevokeFeegrant() {
 				},
 				commonFlags...,
 			),
-			false, 4, &sdk.TxResponse{},
+			false, 38, &sdk.TxResponse{},
 		},
 		{
 			"Valid revoke",

x/feegrant/keeper/keeper.go

@@ -52,8 +52,15 @@ func (k Keeper) GrantAllowance(ctx sdk.Context, granter, grantee sdk.AccAddress,
 	key := feegrant.FeeAllowanceKey(granter, grantee)
 
 	var oldExp *time.Time
-	existingGrant, err := k.getGrant(ctx, grantee, granter)
-	if err != nil && existingGrant != nil && existingGrant.GetAllowance() != nil {
+	existingGrant, err := k.getGrant(ctx, granter, grantee)
+
+	// If we didn't find any grant, we don't return any error.
+	// All other kinds of errors are returned.
+	if err != nil && !sdkerrors.IsOf(err, sdkerrors.ErrNotFound) {
+		return err
+	}
+
+	if existingGrant != nil && existingGrant.GetAllowance() != nil {
 		grantInfo, err := existingGrant.GetGrant()
 		if err != nil {
 			return err
@@ -184,7 +191,7 @@ func (k Keeper) getGrant(ctx sdk.Context, granter sdk.AccAddress, grantee sdk.Ac
 	key := feegrant.FeeAllowanceKey(granter, grantee)
 	bz := store.Get(key)
 	if len(bz) == 0 {
-		return nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "fee-grant not found")
+		return nil, sdkerrors.ErrNotFound.Wrap("fee-grant not found")
 	}
 
 	var feegrant feegrant.Grant

x/feegrant/keeper/keeper_test.go

@@ -46,6 +46,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
 	// some helpers
 	eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 123))
 	exp := suite.sdkCtx.BlockTime().AddDate(1, 0, 0)
+	exp2 := suite.sdkCtx.BlockTime().AddDate(2, 0, 0)
 	basic := &feegrant.BasicAllowance{
 		SpendLimit: suite.atom,
 		Expiration: &exp,
@@ -56,6 +57,11 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
 		Expiration: &exp,
 	}
 
+	basic3 := &feegrant.BasicAllowance{
+		SpendLimit: eth,
+		Expiration: &exp2,
+	}
+
 	// let's set up some initial state here
 	err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[0], suite.addrs[1], basic)
 	suite.Require().NoError(err)
@@ -85,7 +91,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
 	err = suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[0], suite.addrs[2], basic)
 	suite.Require().NoError(err)
 
-	err = suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[1], suite.addrs[2], basic2)
+	err = suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[1], suite.addrs[2], basic3)
 	suite.Require().NoError(err)
 
 	// end state:
@@ -115,7 +121,7 @@ func (suite *KeeperTestSuite) TestKeeperCrud() {
 		"addr modified": {
 			granter:   suite.addrs[1],
 			grantee:   suite.addrs[2],
-			allowance: basic2,
+			allowance: basic3,
 		},
 	}
 
@@ -265,6 +271,7 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
 	eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 123))
 	now := suite.sdkCtx.BlockTime()
 	oneYearExpiry := now.AddDate(1, 0, 0)
+	oneDay := now.AddDate(0, 0, 1)
 
 	testCases := []struct {
 		name      string
@@ -273,6 +280,8 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
 		grantee   sdk.AccAddress
 		allowance feegrant.FeeAllowanceI
 		expErrMsg string
+		preRun    func()
+		postRun   func()
 	}{
 		{
 			name:    "grant not pruned from state",
@@ -326,12 +335,108 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
 				Expiration: &oneYearExpiry,
 			},
 		},
+		{
+			name:    "grant created with a day expiry & overwritten with no expiry shouldn't be pruned: no error",
+			ctx:     suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
+			granter: suite.addrs[2],
+			grantee: suite.addrs[1],
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: eth,
+			},
+			preRun: func() {
+				// create a grant with a day expiry.
+				allowance := &feegrant.BasicAllowance{
+					SpendLimit: suite.atom,
+					Expiration: &oneDay,
+				}
+				err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
+				suite.NoError(err)
+			},
+			postRun: func() {
+				_, err := suite.msgSrvr.RevokeAllowance(suite.sdkCtx, &feegrant.MsgRevokeAllowance{
+					Granter: suite.addrs[2].String(),
+					Grantee: suite.addrs[1].String(),
+				})
+				suite.NoError(err)
+			},
+		},
+		{
+			name:    "grant created with a day expiry & overwritten with a year expiry shouldn't be pruned: no error",
+			ctx:     suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
+			granter: suite.addrs[2],
+			grantee: suite.addrs[1],
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: eth,
+				Expiration: &oneYearExpiry,
+			},
+			preRun: func() {
+				// create a grant with a day expiry.
+				allowance := &feegrant.BasicAllowance{
+					SpendLimit: suite.atom,
+					Expiration: &oneDay,
+				}
+				err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
+				suite.NoError(err)
+			},
+			postRun: func() {
+				_, err := suite.msgSrvr.RevokeAllowance(suite.sdkCtx, &feegrant.MsgRevokeAllowance{
+					Granter: suite.addrs[2].String(),
+					Grantee: suite.addrs[1].String(),
+				})
+				suite.NoError(err)
+			},
+		},
+		{
+			name:    "grant created with a year expiry & overwritten with a day expiry should be pruned after a day: error",
+			ctx:     suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
+			granter: suite.addrs[2],
+			grantee: suite.addrs[1],
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: eth,
+				Expiration: &oneDay,
+			},
+			preRun: func() {
+				// create a grant with a year expiry.
+				allowance := &feegrant.BasicAllowance{
+					SpendLimit: suite.atom,
+					Expiration: &oneYearExpiry,
+				}
+				err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
+				suite.NoError(err)
+			},
+			postRun:   func() {},
+			expErrMsg: "not found",
+		},
+		{
+			name:    "grant created with no expiry & overwritten with a day expiry should be pruned after a day: error",
+			ctx:     suite.sdkCtx.WithBlockTime(now.AddDate(0, 0, 2)),
+			granter: suite.addrs[2],
+			grantee: suite.addrs[1],
+			allowance: &feegrant.BasicAllowance{
+				SpendLimit: eth,
+				Expiration: &oneDay,
+			},
+			preRun: func() {
+				// create a grant with no expiry.
+				allowance := &feegrant.BasicAllowance{
+					SpendLimit: suite.atom,
+				}
+				err := suite.keeper.GrantAllowance(suite.sdkCtx, suite.addrs[2], suite.addrs[1], allowance)
+				suite.NoError(err)
+			},
+			postRun:   func() {},
+			expErrMsg: "not found",
+		},
 	}
 
 	for _, tc := range testCases {
 		tc := tc
 		suite.Run(tc.name, func() {
-			suite.keeper.GrantAllowance(suite.sdkCtx, tc.granter, tc.grantee, tc.allowance)
+			if tc.preRun != nil {
+				tc.preRun()
+			}
+			err := suite.keeper.GrantAllowance(suite.sdkCtx, tc.granter, tc.grantee, tc.allowance)
+			suite.NoError(err)
 			suite.app.FeeGrantKeeper.RemoveExpiredAllowances(tc.ctx)
 			grant, err := suite.keeper.GetAllowance(tc.ctx, tc.granter, tc.grantee)
 			if tc.expErrMsg != "" {
@@ -340,6 +445,9 @@ func (suite *KeeperTestSuite) TestPruneGrants() {
 			} else {
 				suite.NotNil(grant)
 			}
+			if tc.postRun != nil {
+				tc.postRun()
+			}
 		})
 	}
 }