feat!: implement expired subspace fee grant deletion (#1198)

## Description

This PR implements expired subspace fee grant deletion mentioned in
[ADR-017](https://github.com/desmos-labs/desmos/blob/master/docs/architecture/adr-017-subspace-fee-grant.md#store-1).

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review. -->

---

### 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...

- [x] included the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR
Targeting](https://github.com/desmos-labs/desmos/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building
modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration
[tests](https://github.com/desmos-labs/desmos/blob/master/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
- [x] 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)

---------

Co-authored-by: Riccardo <riccardo.montagnin@gmail.com>

.changeset/entries/110685c056cf123bfab42140330553503f10e3ec69b13d2a6edb7c57c99a13d7.yaml

@@ -0,0 +1,6 @@
+type: feat
+module: x/subspaces
+pull_request: 1198
+description: Implement expired subspace fee grant deletion
+backward_compatible: true
+date: 2023-07-14T08:54:35.868279313Z

x/subspaces/abci.go

@@ -0,0 +1,12 @@
+package subspaces
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+
+	"github.com/desmos-labs/desmos/v5/x/subspaces/keeper"
+)
+
+// BeginBlocker is called every block and takes care of removing expired allowances
+func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) {
+	keeper.RemoveExpiredAllowances(ctx, ctx.BlockTime())
+}

x/subspaces/abci_test.go

@@ -0,0 +1,123 @@
+package subspaces_test
+
+import (
+	"testing"
+	"time"
+
+	db "github.com/cometbft/cometbft-db"
+	"github.com/cometbft/cometbft/libs/log"
+	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
+	"github.com/cosmos/cosmos-sdk/store"
+	storetypes "github.com/cosmos/cosmos-sdk/store/types"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/feegrant"
+	"github.com/stretchr/testify/require"
+
+	"github.com/desmos-labs/desmos/v5/app"
+	"github.com/desmos-labs/desmos/v5/x/subspaces"
+	"github.com/desmos-labs/desmos/v5/x/subspaces/keeper"
+	"github.com/desmos-labs/desmos/v5/x/subspaces/types"
+)
+
+func TestBeginBlocker(t *testing.T) {
+	// Define store keys
+	keys := sdk.NewMemoryStoreKeys(
+		types.StoreKey,
+	)
+
+	// Create an in-memory db
+	memDB := db.NewMemDB()
+	ms := store.NewCommitMultiStore(memDB)
+	for _, key := range keys {
+		ms.MountStoreWithDB(key, storetypes.StoreTypeIAVL, memDB)
+	}
+
+	err := ms.LoadLatestVersion()
+	require.NoError(t, err)
+
+	ctx := sdk.NewContext(ms, tmproto.Header{ChainID: "test-chain"}, false, log.NewNopLogger())
+	cdc, _ := app.MakeCodecs()
+
+	keeper := keeper.NewKeeper(cdc, keys[types.StoreKey], nil, nil, "authority")
+
+	testCases := []struct {
+		name     string
+		setupCtx func(ctx sdk.Context) sdk.Context
+		store    func(ctx sdk.Context)
+		check    func(ctx sdk.Context)
+	}{
+		{
+			name: "allowance is not expired before time",
+			setupCtx: func(ctx sdk.Context) sdk.Context {
+				return ctx.WithBlockTime(time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC))
+			},
+			store: func(ctx sdk.Context) {
+				// Just 1 nanosecond after time
+				expiration := time.Date(2020, 1, 1, 12, 00, 00, 001, time.UTC)
+
+				keeper.SaveGrant(ctx, types.NewGrant(
+					1,
+					"cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53",
+					types.NewGroupGrantee(1),
+					&feegrant.BasicAllowance{
+						SpendLimit: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(1))),
+						Expiration: &expiration,
+					},
+				))
+			},
+			check: func(ctx sdk.Context) {
+				kvStore := ctx.KVStore(keys[types.StoreKey])
+				expiration := time.Date(2020, 1, 1, 12, 00, 00, 001, time.UTC)
+				key := types.GroupAllowanceKey(1, 1)
+				require.True(t, kvStore.Has(key))
+				require.True(t, kvStore.Has(types.ExpiringAllowanceKey(&expiration, key)))
+			},
+		},
+		{
+			name: "allowance is expired after time",
+			setupCtx: func(ctx sdk.Context) sdk.Context {
+				return ctx.WithBlockTime(time.Date(2020, 1, 1, 12, 00, 00, 001, time.UTC))
+			},
+			store: func(ctx sdk.Context) {
+				// Just 1 nanosecond before time
+				expiration := time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC)
+
+				keeper.SaveGrant(ctx, types.NewGrant(
+					1,
+					"cosmos1x5pjlvufs4znnhhkwe8v4tw3kz30f3lxgwza53",
+					types.NewGroupGrantee(1),
+					&feegrant.BasicAllowance{
+						SpendLimit: sdk.NewCoins(sdk.NewCoin("test", sdk.NewInt(1))),
+						Expiration: &expiration,
+					},
+				))
+			},
+			check: func(ctx sdk.Context) {
+				kvStore := ctx.KVStore(keys[types.StoreKey])
+				expiration := time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC)
+				key := types.GroupAllowanceKey(1, 1)
+				require.False(t, kvStore.Has(key))
+				require.False(t, kvStore.Has(types.ExpiringAllowanceKey(&expiration, key)))
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			ctx, _ := ctx.CacheContext()
+			if tc.setupCtx != nil {
+				ctx = tc.setupCtx(ctx)
+			}
+			if tc.store != nil {
+				tc.store(ctx)
+			}
+
+			subspaces.BeginBlocker(ctx, keeper)
+
+			if tc.check != nil {
+				tc.check(ctx)
+			}
+		})
+	}
+}

x/subspaces/keeper/common_test.go

@@ -72,3 +72,18 @@ func (suite *KeeperTestSuite) SetupTest() {
 	// Define keeper
 	suite.k = keeper.NewKeeper(suite.cdc, suite.storeKey, suite.ak, suite.authzKeeper, authtypes.NewModuleAddress("gov").String())
 }
+
+func (suite *KeeperTestSuite) getAllGrantsInExpiringQueue(ctx sdk.Context) []types.Grant {
+	store := ctx.KVStore(suite.storeKey)
+	iterator := sdk.KVStorePrefixIterator(store, types.ExpiringAllowanceQueuePrefix)
+	defer iterator.Close()
+
+	var grants []types.Grant
+	for ; iterator.Valid(); iterator.Next() {
+		var grant types.Grant
+		suite.cdc.MustUnmarshal(store.Get(types.ParseAllowanceKeyFromExpiringKey(iterator.Key())), &grant)
+		grants = append(grants, grant)
+	}
+
+	return grants
+}

x/subspaces/keeper/feegrant.go

@@ -2,6 +2,7 @@ package keeper
 
 import (
 	"fmt"
+	"time"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 
@@ -11,8 +12,14 @@ import (
 // SaveGrant saves the given grant inside the current context
 func (k Keeper) SaveGrant(ctx sdk.Context, grant types.Grant) {
 	store := ctx.KVStore(k.storeKey)
-	store.Set(getGrantKey(grant), k.cdc.MustMarshal(&grant))
 
+	grantKey := getGrantKey(grant)
+
+	// Save allowance grant to expiration queue
+	k.saveAllowanceToExpiringQueue(ctx, grant.GetExpiration(), grantKey)
+
+	// Save allowance grant
+	store.Set(grantKey, k.cdc.MustMarshal(&grant))
 	if grantee, isUserGrantee := grant.Grantee.GetCachedValue().(*types.UserGrantee); isUserGrantee {
 		k.createAccountIfNotExists(ctx, grantee.User)
 	}
@@ -41,6 +48,11 @@ func (k Keeper) HasUserGrant(ctx sdk.Context, subspaceID uint64, grantee string)
 func (k Keeper) DeleteUserGrant(ctx sdk.Context, subspaceID uint64, grantee string) {
 	store := ctx.KVStore(k.storeKey)
 	key := types.UserAllowanceKey(subspaceID, grantee)
+
+	// Remove the allowance from the list of expiring allowances
+	k.removeAllowanceFromExpiringQueue(ctx, key)
+
+	// Delete allowance
 	store.Delete(key)
 }
 
@@ -68,6 +80,11 @@ func (k Keeper) HasGroupGrant(ctx sdk.Context, subspaceID uint64, groupID uint32
 func (k Keeper) DeleteGroupGrant(ctx sdk.Context, subspaceID uint64, groupID uint32) {
 	store := ctx.KVStore(k.storeKey)
 	key := types.GroupAllowanceKey(subspaceID, groupID)
+
+	// Remove the allowance from the list of expiring allowances
+	k.removeAllowanceFromExpiringQueue(ctx, key)
+
+	// Delete allowance
 	store.Delete(key)
 }
 
@@ -87,6 +104,51 @@ func (k Keeper) GetGroupGrant(ctx sdk.Context, subspaceID uint64, groupID uint32
 
 // --------------------------------------------------------------------------------------------------------------------
 
+// saveAllowanceToExpiringQueue saves the allowance into expiring queue
+func (k Keeper) saveAllowanceToExpiringQueue(ctx sdk.Context, expiration *time.Time, grantKey []byte) {
+	// Make sure we remove the grant from the expiring queue to properly handle expiration date updates and avoid duplicated keys
+	k.removeAllowanceFromExpiringQueue(ctx, grantKey)
+
+	store := ctx.KVStore(k.storeKey)
+	if expiration != nil {
+		store.Set(types.ExpiringAllowanceKey(expiration, grantKey), []byte{0x1})
+	}
+}
+
+// removeAllowanceFromExpiringQueue removes allowance from expiring queue
+func (k Keeper) removeAllowanceFromExpiringQueue(ctx sdk.Context, grantKey []byte) {
+	store := ctx.KVStore(k.storeKey)
+
+	// Do nothing if grant does not exist
+	if !store.Has(grantKey) {
+		return
+	}
+
+	// Get existing grant
+	var grant types.Grant
+	k.cdc.MustUnmarshal(store.Get(grantKey), &grant)
+
+	// Delete grant from expiration queue
+	expiration := grant.GetExpiration()
+	if expiration != nil {
+		store.Delete(types.ExpiringAllowanceKey(expiration, grantKey))
+	}
+}
+
+// RemoveExpiredAllowances deletes the expired allowances
+func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context, expiration time.Time) {
+	store := ctx.KVStore(k.storeKey)
+	iterator := store.Iterator(types.ExpiringAllowanceQueuePrefix, types.ExpiringAllowanceTimePrefix(&expiration))
+	defer iterator.Close()
+
+	for ; iterator.Valid(); iterator.Next() {
+		store.Delete(iterator.Key())
+		store.Delete(types.ParseAllowanceKeyFromExpiringKey(iterator.Key()))
+	}
+}
+
+// --------------------------------------------------------------------------------------------------------------------
+
 // UseGrantedFees will try to pay the given fees from the treasury account as requested by the grantee.
 // If no valid allowance exists, then returns false to show the fees will not be paid in this phase.
 func (k Keeper) UseGrantedFees(ctx sdk.Context, subspaceID uint64, grantee sdk.AccAddress, fees sdk.Coins, msgs []sdk.Msg) bool {