diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml index e06ba1e39c16..bda6a1d32a4e 100644 --- a/.github/workflows/labeler.yml +++ b/.github/workflows/labeler.yml @@ -14,4 +14,5 @@ jobs: steps: - uses: actions/labeler@v4 with: + configuration-path: .github/labeler.yml repo-token: "${{ secrets.GITHUB_TOKEN }}" diff --git a/.github/workflows/lint-pr.yml b/.github/workflows/lint-pr.yml deleted file mode 100644 index d5b4d46b2158..000000000000 --- a/.github/workflows/lint-pr.yml +++ /dev/null @@ -1,22 +0,0 @@ -name: "Lint PR" - -on: - pull_request_target: - types: - - opened - - edited - - synchronize - -permissions: - contents: read - -jobs: - main: - permissions: - pull-requests: read # for amannn/action-semantic-pull-request to analyze PRs - statuses: write # for amannn/action-semantic-pull-request to mark status of analyzed PR - runs-on: ubuntu-latest - steps: - - uses: amannn/action-semantic-pull-request@v5.0.2 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 7e2f8553c9fd..3c3c17cc059e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -264,12 +264,4 @@ jobs: if: env.GIT_DIFF run: | cd simapp - go test -mod=readonly -timeout 30m -tags='app_v1 norace ledger test_ledger_mock rocksdb_build' ./... - - name: sonarcloud - if: env.GIT_DIFF - uses: SonarSource/sonarcloud-github-action@master - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - with: - projectBaseDir: simapp/ + go test -mod=readonly -timeout 30m -tags='app_v1 norace ledger test_ledger_mock rocksdb_build' ./... \ No newline at end of file diff --git a/store/cachekv/search_benchmark_test.go b/store/cachekv/search_benchmark_test.go index 4007c7cda202..ddf4bdf02c15 100644 --- a/store/cachekv/search_benchmark_test.go +++ b/store/cachekv/search_benchmark_test.go @@ -3,8 +3,6 @@ package cachekv import ( "strconv" "testing" - - "github.com/cosmos/cosmos-sdk/store/cachekv/internal" ) func BenchmarkLargeUnsortedMisses(b *testing.B) { @@ -36,9 +34,10 @@ func generateStore() *Store { cache[key] = &cValue{} } - return &Store{ + store := &Store{ cache: cache, unsortedCache: unsorted, - sortedCache: internal.NewBTree(), } + store.resetSortedCache() + return store } diff --git a/store/cachekv/store.go b/store/cachekv/store.go index a8f468979cd9..2cb5a63fa6a4 100644 --- a/store/cachekv/store.go +++ b/store/cachekv/store.go @@ -28,7 +28,7 @@ type Store struct { mtx sync.Mutex cache map[string]*cValue unsortedCache map[string]struct{} - sortedCache internal.BTree // always ascending sorted + sortedCache *internal.BTree // always ascending sorted parent types.KVStore } @@ -39,7 +39,7 @@ func NewStore(parent types.KVStore) *Store { return &Store{ cache: make(map[string]*cValue), unsortedCache: make(map[string]struct{}), - sortedCache: internal.NewBTree(), + sortedCache: nil, parent: parent, } } @@ -93,13 +93,18 @@ func (store *Store) Delete(key []byte) { store.setCacheValue(key, nil, true) } +func (store *Store) resetSortedCache() { + newTree := internal.NewBTree() + store.sortedCache = &newTree +} + // Implements Cachetypes.KVStore. func (store *Store) Write() { store.mtx.Lock() defer store.mtx.Unlock() if len(store.cache) == 0 && len(store.unsortedCache) == 0 { - store.sortedCache = internal.NewBTree() + store.sortedCache = nil return } @@ -140,7 +145,7 @@ func (store *Store) Write() { for key := range store.unsortedCache { delete(store.unsortedCache, key) } - store.sortedCache = internal.NewBTree() + store.sortedCache = nil } // CacheWrap implements CacheWrapper. @@ -169,6 +174,9 @@ func (store *Store) ReverseIterator(start, end []byte) types.Iterator { func (store *Store) iterator(start, end []byte, ascending bool) types.Iterator { store.mtx.Lock() defer store.mtx.Unlock() + if store.sortedCache == nil { + store.resetSortedCache() + } store.dirtyItems(start, end) isoSortedCache := store.sortedCache.Copy() diff --git a/store/cachemulti/store.go b/store/cachemulti/store.go index 86927466a905..a162910fe5a5 100644 --- a/store/cachemulti/store.go +++ b/store/cachemulti/store.go @@ -73,7 +73,7 @@ func NewStore( } func newCacheMultiStoreFromCMS(cms Store) Store { - stores := make(map[types.StoreKey]types.CacheWrapper) + stores := make(map[types.StoreKey]types.CacheWrapper, len(cms.stores)) for k, v := range cms.stores { stores[k] = v } diff --git a/x/feegrant/keeper/keeper.go b/x/feegrant/keeper/keeper.go index bb36f9c7964f..4510218cc6bb 100644 --- a/x/feegrant/keeper/keeper.go +++ b/x/feegrant/keeper/keeper.go @@ -323,17 +323,25 @@ func (k Keeper) addToFeeAllowanceQueue(ctx sdk.Context, grantKey []byte, exp *ti store.Set(feegrant.FeeAllowancePrefixQueue(exp, grantKey), []byte{}) } +// NOTE: backport from v50 // RemoveExpiredAllowances iterates grantsByExpiryQueue and deletes the expired grants. -func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context) { +func (k Keeper) RemoveExpiredAllowances(ctx sdk.Context, limit int) { exp := ctx.BlockTime() store := ctx.KVStore(k.storeKey) iterator := store.Iterator(feegrant.FeeAllowanceQueueKeyPrefix, sdk.InclusiveEndBytes(feegrant.AllowanceByExpTimeKey(&exp))) defer iterator.Close() + count := 0 for ; iterator.Valid(); iterator.Next() { store.Delete(iterator.Key()) granter, grantee := feegrant.ParseAddressesFromFeeAllowanceQueueKey(iterator.Key()) store.Delete(feegrant.FeeAllowanceKey(granter, grantee)) + + // limit the amount of iterations to avoid taking too much time + count++ + if count == limit { + break + } } } diff --git a/x/feegrant/keeper/keeper_test.go b/x/feegrant/keeper/keeper_test.go index 16d10a532cd6..348c6326bdf5 100644 --- a/x/feegrant/keeper/keeper_test.go +++ b/x/feegrant/keeper/keeper_test.go @@ -33,7 +33,7 @@ func TestKeeperTestSuite(t *testing.T) { } func (suite *KeeperTestSuite) SetupTest() { - suite.addrs = simtestutil.CreateIncrementalAccounts(4) + suite.addrs = simtestutil.CreateIncrementalAccounts(20) key := sdk.NewKVStoreKey(feegrant.StoreKey) testCtx := testutil.DefaultContextWithDB(suite.T(), key, sdk.NewTransientStoreKey("transient_test")) encCfg := moduletestutil.MakeTestEncodingConfig(module.AppModuleBasic{}) @@ -41,10 +41,9 @@ func (suite *KeeperTestSuite) SetupTest() { // setup gomock and initialize some globally expected executions ctrl := gomock.NewController(suite.T()) suite.accountKeeper = feegranttestutil.NewMockAccountKeeper(ctrl) - suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[0]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[0])).AnyTimes() - suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[1]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[1])).AnyTimes() - suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[2]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[2])).AnyTimes() - suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[3]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[3])).AnyTimes() + for i := 0; i < len(suite.addrs); i++ { + suite.accountKeeper.EXPECT().GetAccount(gomock.Any(), suite.addrs[i]).Return(authtypes.NewBaseAccountWithAddress(suite.addrs[i])).AnyTimes() + } suite.feegrantKeeper = keeper.NewKeeper(encCfg.Codec, key, suite.accountKeeper) suite.ctx = testCtx.Ctx @@ -447,7 +446,7 @@ func (suite *KeeperTestSuite) TestPruneGrants() { } err := suite.feegrantKeeper.GrantAllowance(suite.ctx, tc.granter, tc.grantee, tc.allowance) suite.NoError(err) - suite.feegrantKeeper.RemoveExpiredAllowances(tc.ctx) + suite.feegrantKeeper.RemoveExpiredAllowances(tc.ctx, 5) grant, err := suite.feegrantKeeper.GetAllowance(tc.ctx, tc.granter, tc.grantee) if tc.expErrMsg != "" { suite.Error(err) @@ -461,3 +460,38 @@ func (suite *KeeperTestSuite) TestPruneGrants() { }) } } + +func (suite *KeeperTestSuite) TestPruneGrantsEdgecases() { + eth := sdk.NewCoins(sdk.NewInt64Coin("eth", 123)) + now := suite.ctx.BlockTime() + oneYearExpiry := now.AddDate(1, 0, 0) + + granter := suite.addrs[1] + grantee1 := suite.addrs[2] + err := suite.feegrantKeeper.GrantAllowance(suite.ctx, granter, grantee1, &feegrant.BasicAllowance{ + SpendLimit: eth, + Expiration: &oneYearExpiry, + }) + suite.NoError(err) + + grantee2 := suite.addrs[3] + err = suite.feegrantKeeper.GrantAllowance(suite.ctx, granter, grantee2, &feegrant.BasicAllowance{ + SpendLimit: eth, + Expiration: &now, + }) + suite.NoError(err) + + // expect 2 active grants + grantsBeforePrune, err := suite.feegrantKeeper.AllowancesByGranter(suite.ctx, &feegrant.QueryAllowancesByGranterRequest{Granter: granter.String()}) + suite.NoError(err) + suite.Len(grantsBeforePrune.Allowances, 2) + + // use blocktime that would result in both grants being expired + expireCtx := suite.ctx.WithBlockTime(now.AddDate(1, 0, 1)) + + // expect 1 grant to be removed due to the imposed limit + suite.feegrantKeeper.RemoveExpiredAllowances(expireCtx, 1) + grantsAfterPrune, err := suite.feegrantKeeper.AllowancesByGranter(suite.ctx, &feegrant.QueryAllowancesByGranterRequest{Granter: granter.String()}) + suite.NoError(err) + suite.Len(grantsAfterPrune.Allowances, 1) +} diff --git a/x/feegrant/module/abci.go b/x/feegrant/module/abci.go index 34c19c528d6a..e6577806080e 100644 --- a/x/feegrant/module/abci.go +++ b/x/feegrant/module/abci.go @@ -6,5 +6,5 @@ import ( ) func EndBlocker(ctx sdk.Context, k keeper.Keeper) { - k.RemoveExpiredAllowances(ctx) + k.RemoveExpiredAllowances(ctx, 200) }