(Backport) fix(x/authz)!: limit expired authz grant pruning to 200 per block

CHANGELOG.md

@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (store) [#525](https://github.com/osmosis-labs/cosmos-sdk/pull/525) CacheKV speedups
 * (slashing) [#548](https://github.com/osmosis-labs/cosmos-sdk/pull/548) Implement v0.50 slashing bitmap logic
 * (slashing) [#543](https://github.com/osmosis-labs/cosmos-sdk/pull/543) Make slashing not write sign info every block
+* (authz) [#513](https://github.com/osmosis-labs/cosmos-sdk/pull/513) Limit expired authz grant pruning to 200 per block
 * (gov) [#514](https://github.com/osmosis-labs/cosmos-sdk/pull/514) Let gov hooks return an error
 
 ## IAVL v23 v1 Releases

x/authz/keeper/keeper.go

@@ -374,14 +374,21 @@ func (k Keeper) removeFromGrantQueue(ctx sdk.Context, grantKey []byte, granter,
 	return nil
 }
 
+// NOTE: backported from v50
 // DequeueAndDeleteExpiredGrants deletes expired grants from the state and grant queue.
-func (k Keeper) DequeueAndDeleteExpiredGrants(ctx sdk.Context) error {
+func (k Keeper) DequeueAndDeleteExpiredGrants(ctx sdk.Context, limit int) error {
 	store := ctx.KVStore(k.storeKey)
 
 	iterator := store.Iterator(GrantQueuePrefix, sdk.InclusiveEndBytes(GrantQueueTimePrefix(ctx.BlockTime())))
 	defer iterator.Close()
 
+	count := 0
 	for ; iterator.Valid(); iterator.Next() {
+		// limit the amount of iterations to avoid taking too much time
+		if count >= limit {
+			return nil
+		}
+
 		var queueItem authz.GrantQueueItem
 		if err := k.cdc.Unmarshal(iterator.Value(), &queueItem); err != nil {
 			return err
@@ -397,6 +404,8 @@ func (k Keeper) DequeueAndDeleteExpiredGrants(ctx sdk.Context) error {
 		for _, typeURL := range queueItem.MsgTypeUrls {
 			store.Delete(grantStoreKey(grantee, granter, typeURL))
 		}
+
+		count++
 	}
 
 	return nil

x/authz/keeper/keeper_test.go

@@ -370,7 +370,8 @@ func (s *TestSuite) TestDequeueAllGrantsQueue() {
 	require.NoError(err)
 
 	newCtx := s.ctx.WithBlockTime(exp.AddDate(1, 0, 0))
-	err = s.authzKeeper.DequeueAndDeleteExpiredGrants(newCtx)
+	// setting a high limit so all grants are dequeued
+	err = s.authzKeeper.DequeueAndDeleteExpiredGrants(newCtx, 200)
 	require.NoError(err)
 
 	s.T().Log("verify expired grants are pruned from the state")
@@ -391,6 +392,74 @@ func (s *TestSuite) TestDequeueAllGrantsQueue() {
 	require.Len(authzs, 1)
 }
 
+func (s *TestSuite) TestDequeueGrantsQueueEdgecases() {
+	require := s.Require()
+	addrs := s.addrs
+	granter := addrs[0]
+	grantee := addrs[1]
+	grantee1 := addrs[2]
+	exp := s.ctx.BlockTime().AddDate(0, 0, 1)
+	a := banktypes.SendAuthorization{SpendLimit: coins100}
+
+	// create few authorizations
+	err := s.authzKeeper.SaveGrant(s.ctx, grantee, granter, &a, &exp)
+	require.NoError(err)
+
+	err = s.authzKeeper.SaveGrant(s.ctx, grantee1, granter, &a, &exp)
+	require.NoError(err)
+
+	exp2 := exp.AddDate(0, 1, 0)
+	err = s.authzKeeper.SaveGrant(s.ctx, granter, grantee1, &a, &exp2)
+	require.NoError(err)
+
+	exp2 = exp.AddDate(2, 0, 0)
+	err = s.authzKeeper.SaveGrant(s.ctx, granter, grantee, &a, &exp2)
+	require.NoError(err)
+
+	newCtx := s.ctx.WithBlockTime(exp.AddDate(1, 0, 0))
+	err = s.authzKeeper.DequeueAndDeleteExpiredGrants(newCtx, 0)
+	require.NoError(err)
+
+	s.T().Log("verify no pruning happens when limit is 0")
+	authzs, err := s.authzKeeper.GetAuthorizations(newCtx, grantee, granter)
+	require.NoError(err)
+	require.Len(authzs, 1)
+
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, granter, grantee1)
+	require.NoError(err)
+	require.Len(authzs, 1)
+
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, grantee1, granter)
+	require.NoError(err)
+	require.Len(authzs, 1)
+
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, granter, grantee)
+	require.NoError(err)
+	require.Len(authzs, 1)
+
+	// expecting to prune 1 record when limit is 1
+	newCtx = s.ctx.WithBlockTime(exp.AddDate(1, 0, 0))
+	err = s.authzKeeper.DequeueAndDeleteExpiredGrants(newCtx, 1)
+	require.NoError(err)
+
+	s.T().Log("verify 1 record is prunded when limit is 1")
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, grantee, granter)
+	require.NoError(err)
+	require.Len(authzs, 0)
+
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, granter, grantee1)
+	require.NoError(err)
+	require.Len(authzs, 1)
+
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, grantee1, granter)
+	require.NoError(err)
+	require.Len(authzs, 1)
+
+	authzs, err = s.authzKeeper.GetAuthorizations(newCtx, granter, grantee)
+	require.NoError(err)
+	require.Len(authzs, 1)
+}
+
 func (s *TestSuite) TestGetAuthorization() {
 	addr1 := s.addrs[3]
 	addr2 := s.addrs[4]

x/authz/module/abci.go

@@ -8,7 +8,7 @@
 // BeginBlocker is called at the beginning of every block
 func BeginBlocker(ctx sdk.Context, keeper keeper.Keeper) {
 	// delete all the mature grants
-	if err := keeper.DequeueAndDeleteExpiredGrants(ctx); err != nil {
+	if err := keeper.DequeueAndDeleteExpiredGrants(ctx, 200); err != nil {
 		panic(err)
 	}
 }