Skip to content

Commit

Permalink
fix(x/authz)!: limit expired authz grant pruning to 200 per block (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
MSalopek authored Feb 1, 2024
1 parent f6bf231 commit 42a86ea
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 3 deletions.
11 changes: 10 additions & 1 deletion x/authz/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
71 changes: 70 additions & 1 deletion x/authz/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion x/authz/module/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
// 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)
}
}

0 comments on commit 42a86ea

Please sign in to comment.