From f36a0b1b238517f7d7b6aa451f3fa4230e43e27e Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 5 Jan 2024 17:33:36 -0600 Subject: [PATCH 1/5] Expand spam filter --- x/incentives/keeper/distribute.go | 43 ++++++++++++++++++------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index f43801f8b2b..b8d37155a16 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -637,24 +637,9 @@ func (k Keeper) distributeInternal( } } else { // This is a standard lock distribution flow that assumes that we have locks associated with the gauge. - if len(locks) == 0 { - return nil, nil - } - - // In this case, remove redundant cases. - // Namely: gauge empty OR gauge coins undistributable. - if remainCoins.Empty() { - ctx.Logger().Debug(fmt.Sprintf("gauge debug, this gauge is empty, why is it being ran %d. Balancer code", gauge.Id)) - err := k.updateGaugePostDistribute(ctx, gauge, totalDistrCoins) - return totalDistrCoins, err - } - - // Remove some spam gauges, is state compatible. - // If they're to pool 1 they can't distr at this small of a quantity. - if remainCoins.Len() == 1 && remainCoins[0].Amount.LTE(osmomath.NewInt(10)) && gauge.DistributeTo.Denom == "gamm/pool/1" && remainCoins[0].Denom != "uosmo" { - ctx.Logger().Debug(fmt.Sprintf("gauge debug, this gauge is perceived spam, skipping %d", gauge.Id)) - err := k.updateGaugePostDistribute(ctx, gauge, totalDistrCoins) - return totalDistrCoins, err + isSpam, totaltotalDistrCoins, err := k.skipSpamGaugeDistribute(ctx, locks, gauge, totalDistrCoins, remainCoins) + if isSpam { + return totaltotalDistrCoins, err } // This is a standard lock distribution flow that assumes that we have locks associated with the gauge. @@ -713,6 +698,28 @@ func (k Keeper) distributeInternal( return totalDistrCoins, err } +func (k Keeper) skipSpamGaugeDistribute(ctx sdk.Context, locks []*lockuptypes.PeriodLock, gauge types.Gauge, totalDistrCoins sdk.Coins, remainCoins sdk.Coins) (bool, sdk.Coins, error) { + if len(locks) == 0 { + return true, nil, nil + } + + // In this case, remove redundant cases. + // Namely: gauge empty OR gauge coins undistributable. + if remainCoins.Empty() { + ctx.Logger().Debug(fmt.Sprintf("gauge debug, this gauge is empty, why is it being ran %d. Balancer code", gauge.Id)) + err := k.updateGaugePostDistribute(ctx, gauge, totalDistrCoins) + return true, totalDistrCoins, err + } + + // Remove some spam gauges that are not worth distributing. + if remainCoins.Len() == 1 && remainCoins[0].Amount.LTE(osmomath.NewInt(100)) && remainCoins[0].Denom != "stake" { + ctx.Logger().Debug(fmt.Sprintf("gauge debug, this gauge is perceived spam, skipping %d", gauge.Id)) + err := k.updateGaugePostDistribute(ctx, gauge, totalDistrCoins) + return true, totalDistrCoins, err + } + return false, totalDistrCoins, nil +} + func checkBigInt(bi *big.Int) { if bi.BitLen() > sdkmath.MaxBitLen { panic("overflow") From 4ab95369cfb85f33eb64efb11d86013208b4577d Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Fri, 5 Jan 2024 17:53:08 -0600 Subject: [PATCH 2/5] Update comment --- x/incentives/keeper/distribute.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/incentives/keeper/distribute.go b/x/incentives/keeper/distribute.go index b8d37155a16..f4ffafe8730 100644 --- a/x/incentives/keeper/distribute.go +++ b/x/incentives/keeper/distribute.go @@ -711,7 +711,7 @@ func (k Keeper) skipSpamGaugeDistribute(ctx sdk.Context, locks []*lockuptypes.Pe return true, totalDistrCoins, err } - // Remove some spam gauges that are not worth distributing. + // Remove some spam gauges that are not worth distributing. (We ignore the denom stake because of tests.) if remainCoins.Len() == 1 && remainCoins[0].Amount.LTE(osmomath.NewInt(100)) && remainCoins[0].Denom != "stake" { ctx.Logger().Debug(fmt.Sprintf("gauge debug, this gauge is perceived spam, skipping %d", gauge.Id)) err := k.updateGaugePostDistribute(ctx, gauge, totalDistrCoins) From 36a6f63c108b859e65e683da3f64ea5daf2a81af Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Sat, 6 Jan 2024 19:13:51 -0700 Subject: [PATCH 3/5] add changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c7ccb47135..33a4933853f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,10 +57,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * [#7106](https://github.com/osmosis-labs/osmosis/pull/7106) Halve the time of log2 calculation (speeds up TWAP code) * [#7093](https://github.com/osmosis-labs/osmosis/pull/7093),[#7100](https://github.com/osmosis-labs/osmosis/pull/7100),[#7172](https://github.com/osmosis-labs/osmosis/pull/7172) Lower CPU overheads of the Osmosis epoch. * [#7203](https://github.com/osmosis-labs/osmosis/pull/7203) Make a maximum number of pools of 100 billion. +* [#7250](https://github.com/osmosis-labs/osmosis/pull/7250) Further filter spam gauges from epoch distribution. ### Bug Fixes -* [#7233](https://github.com/osmosis-labs/osmosis/pull/7233) fix: config overwrite ignores app.toml values +* [#7233](https://github.com/osmosis-labs/osmosis/pull/7233) fix: config overwrite ignores app.toml values * [#7243](https://github.com/osmosis-labs/osmosis/pull/7243) fix: chore: update gov metadata length from 256 to 10200 * [#7246](https://github.com/osmosis-labs/osmosis/pull/7246) fix: config overwrite fails with exit code 1 if wrong permissions From 5cf383a0df80454fad9f4c202f02dc903c9fef3b Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Tue, 9 Jan 2024 13:30:22 -0700 Subject: [PATCH 4/5] add skipSpamGaugeDistribute test --- x/incentives/keeper/distribute_test.go | 90 ++++++++++++++++++++++++++ x/incentives/keeper/export_test.go | 5 ++ 2 files changed, 95 insertions(+) diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index 3f2ec02de7b..7db790f2b9a 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -2406,3 +2406,93 @@ func (s *KeeperTestSuite) TestHandleGroupPostDistribute() { validateLastEpochNonPerpetualPruning(currentGauge.Id, currentGauge.DistributedCoins.Add(defaultCoins...), initialDistributionCoins, s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName))) }) } + +func (s *KeeperTestSuite) TestSkipSpamGaugeDistribute() { + defaultGauge := perpGaugeDesc{ + lockDenom: defaultLPDenom, + lockDuration: defaultLockDuration, + rewardAmount: sdk.Coins{sdk.NewInt64Coin(defaultRewardDenom, 3000)}, + } + + tenCoins := sdk.Coins{sdk.NewInt64Coin(defaultRewardDenom, 10)} + oneKCoins := sdk.Coins{sdk.NewInt64Coin(defaultRewardDenom, 1000)} + twoCoinsOneK := sdk.Coins{sdk.NewInt64Coin(defaultRewardDenom, 1000), sdk.NewInt64Coin("uosmo", 1000)} + tests := []struct { + name string + locks []*lockuptypes.PeriodLock + gauge perpGaugeDesc + totalDistrCoins sdk.Coins + remainCoins sdk.Coins + expectedToSkip bool + expectedTotalDistrCoins sdk.Coins + expectedGaugeUpdated bool + }{ + { + name: "Lock length of 0, should be skipped", + locks: []*lockuptypes.PeriodLock{}, + gauge: defaultGauge, + totalDistrCoins: oneKCoins, + remainCoins: sdk.Coins{}, + expectedToSkip: true, + expectedTotalDistrCoins: nil, + expectedGaugeUpdated: false, + }, + { + name: "Empty remainCoins, should be skipped", + locks: []*lockuptypes.PeriodLock{ + {ID: 1, Owner: string(s.TestAccs[0]), Coins: oneKCoins, Duration: defaultLockDuration}, + }, + gauge: defaultGauge, + totalDistrCoins: oneKCoins, + remainCoins: sdk.Coins{}, + expectedToSkip: true, + expectedTotalDistrCoins: oneKCoins, + expectedGaugeUpdated: true, + }, + { + name: "Remain coins len == 1 and value less than 100 threshold, should be skipped", + locks: []*lockuptypes.PeriodLock{ + {ID: 1, Owner: string(s.TestAccs[0]), Coins: oneKCoins, Duration: defaultLockDuration}, + }, + gauge: defaultGauge, + totalDistrCoins: oneKCoins, + remainCoins: tenCoins, + expectedToSkip: true, + expectedTotalDistrCoins: oneKCoins, + expectedGaugeUpdated: true, + }, + { + name: "Lock length > 0, gauge value greater than 100 threshold, and remain coins len != 1, should not be skipped", + locks: []*lockuptypes.PeriodLock{ + {ID: 1, Owner: string(s.TestAccs[0]), Coins: oneKCoins, Duration: defaultLockDuration}, + }, + gauge: defaultGauge, + totalDistrCoins: oneKCoins, + remainCoins: twoCoinsOneK, + expectedToSkip: false, + expectedTotalDistrCoins: oneKCoins, + expectedGaugeUpdated: false, + }, + } + for _, tc := range tests { + s.SetupTest() + // setup gauge defined in test case + gauges := s.SetupGauges([]perpGaugeDesc{tc.gauge}, defaultLPDenom) + + shouldBeSkipped, distrCoins, err := s.App.IncentivesKeeper.SkipSpamGaugeDistribute(s.Ctx, tc.locks, gauges[0], tc.totalDistrCoins, tc.remainCoins) + s.Require().NoError(err) + s.Require().Equal(tc.expectedToSkip, shouldBeSkipped) + s.Require().Equal(tc.expectedTotalDistrCoins, distrCoins) + + // Retrieve gauge + gauge, err := s.App.IncentivesKeeper.GetGaugeByID(s.Ctx, gauges[0].Id) + s.Require().NoError(err) + + expectedGauge := gauges[0] + if tc.expectedGaugeUpdated { + expectedGauge.FilledEpochs++ + expectedGauge.DistributedCoins = expectedGauge.DistributedCoins.Add(distrCoins...) + } + s.Require().Equal(expectedGauge, *gauge) + } +} diff --git a/x/incentives/keeper/export_test.go b/x/incentives/keeper/export_test.go index b263d47824b..c6884fa1dd9 100644 --- a/x/incentives/keeper/export_test.go +++ b/x/incentives/keeper/export_test.go @@ -7,6 +7,7 @@ import ( "github.com/osmosis-labs/osmosis/osmomath" "github.com/osmosis-labs/osmosis/v21/x/incentives/types" + lockuptypes "github.com/osmosis-labs/osmosis/v21/x/lockup/types" ) var ByGroupQueryCondition = byGroupQueryCondition @@ -97,3 +98,7 @@ func (k Keeper) CreateGroupInternal(ctx sdk.Context, coins sdk.Coins, numEpochPa func (k Keeper) CalculateGroupWeights(ctx sdk.Context, group types.Group) (types.Group, error) { return k.calculateGroupWeights(ctx, group) } + +func (k Keeper) SkipSpamGaugeDistribute(ctx sdk.Context, locks []*lockuptypes.PeriodLock, gauge types.Gauge, totalDistrCoins sdk.Coins, remainCoins sdk.Coins) (bool, sdk.Coins, error) { + return k.skipSpamGaugeDistribute(ctx, locks, gauge, totalDistrCoins, remainCoins) +} From 8da4e45839e3e2107d4086263fac32f4b6234e62 Mon Sep 17 00:00:00 2001 From: Adam Tucker Date: Sat, 10 Feb 2024 16:29:39 -0700 Subject: [PATCH 5/5] lint --- x/incentives/keeper/distribute_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/incentives/keeper/distribute_test.go b/x/incentives/keeper/distribute_test.go index 27c1e3157c0..3de94893066 100644 --- a/x/incentives/keeper/distribute_test.go +++ b/x/incentives/keeper/distribute_test.go @@ -2478,6 +2478,7 @@ func (s *KeeperTestSuite) TestHandleGroupPostDistribute() { validateLastEpochNonPerpetualPruning(currentGauge.Id, currentGauge.DistributedCoins.Add(defaultCoins...), initialDistributionCoins, s.App.BankKeeper.GetAllBalances(s.Ctx, s.App.AccountKeeper.GetModuleAddress(types.ModuleName))) }) } + func (s *KeeperTestSuite) TestGetNoLockGaugeUptime() { defaultPoolId := uint64(1) tests := map[string]struct {