[Uptime Incentives]: Add authorized uptime validation for new NoLock gauges #7376
+98
−21
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -507,7 +507,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this assumes the gauge is external | ||
| Denom: "", | ||
|
There was a problem hiding this comment. Can we also add non-failing cases for the three branches that has been added to nolock createGauge? There was a problem hiding this comment. The non-failing case should be covered by existing tests, but will re review to make sure I'm not missing anything There was a problem hiding this comment. Sg! On first site of existing tests, test cases did not seem obvious in which case touches which branch. Please feel free to merge once you have re-checked! There was a problem hiding this comment. Right – since 1 nanosecond is an authorized uptime by default, the test cases with that duration should be covering the non-failing branches |
||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
|
|
@@ -520,7 +521,10 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this assumes the gauge is internal | ||
| // We intentionally do not set a gauge duration as it should make no | ||
| // difference for internal gauges. | ||
| Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), | ||
| Duration: s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
|
|
@@ -533,7 +537,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this is invalid for NoLock gauges | ||
| Denom: "uosmo", | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
|
|
@@ -543,6 +548,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| name: "fail to create no lock gauge with balancer pool", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: balancerPoolId, | ||
|
|
||
|
|
@@ -552,6 +558,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| name: "fail to create no lock gauge with non-existent pool", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: invalidPool, | ||
|
|
||
|
|
@@ -561,9 +568,35 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| name: "fail to create no lock gauge with zero pool id", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: zeroPoolId, | ||
|
|
||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "fail to create external no lock gauge due to unauthorized uptime", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this assumes the gauge is external | ||
| Denom: "", | ||
| Duration: 2 * time.Nanosecond, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "fail to create an internal gauge with an unexpected duration", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this assumes the gauge is internal | ||
| // We intentionally do not set a gauge duration as it should make no | ||
| // difference for internal gauges. | ||
| Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
| expectErr: true, | ||
| }, | ||
| } | ||
|
|
@@ -589,10 +622,6 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
|
|
||
| s.Require().Equal(tc.expectedGaugeId, gaugeId) | ||
|
|
||
|
There was a problem hiding this comment. Highlighting this for review @p0mvn. The key-related change in this PR means that the "general linking" between gauges and pools is now based on duration, which means that this check is no longer necessary. It also technically means that we can skip the "internal gauge link" component of the internal gauge setup, but that logic is not well abstracted so it would require meaningful additional work to save on the duplicate write that would be happening here. |
||
| // Confirm that the general pool id to gauge id link is set. | ||
| gaugeIds, err := s.App.PoolIncentivesKeeper.GetNoLockGaugeIdsFromPool(s.Ctx, tc.poolId) | ||
| s.Require().NoError(err) | ||
|
|
@@ -774,7 +803,7 @@ func (s *KeeperTestSuite) createGaugeNoRestrictions(isPerpetual bool, coins sdk. | |
| } | ||
|
|
||
| if poolID != 0 { | ||
| s.App.PoolIncentivesKeeper.SetPoolGaugeIdNoLock(s.Ctx, poolID, nextGaugeID, distrTo.Duration) | ||
| } | ||
|
|
||
| err := s.App.IncentivesKeeper.SetGauge(s.Ctx, &gauge) | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,7 +28,7 @@ func (k Keeper) InitGenesis(ctx sdk.Context, genState *types.GenesisState) { | |
| } | ||
| if genState.ConcentratedPoolToNoLockGauges != nil { | ||
| for _, record := range genState.ConcentratedPoolToNoLockGauges.PoolToGauge { | ||
| k.SetPoolGaugeIdNoLock(ctx, record.PoolId, record.GaugeId, record.Duration) | ||
|
There was a problem hiding this comment. Are we planning for any migration PR after this? 👀 There was a problem hiding this comment. We shouldn't need to - please see #7375 (if you see any problems with the reasoning in that issue please flag them!) |
||
| } | ||
| } | ||
| } | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewer: CL gauges with unauthorized uptimes can no longer be created due to the new validation logic, so existing tests are updated to use 1ns which is authorized by default. See #7375 for discussion on backwards compatibility with gauges currently live on mainnet with unauthorized uptimes.