[Uptime Incentives]: Add authorized uptime validation for new NoLock gauges #7376
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: "", | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
|
|
@@ -520,7 +521,8 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this assumes the gauge is internal | ||
| Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), | ||
| Duration: s.App.IncentivesKeeper.GetEpochInfo(s.Ctx).Duration, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
|
|
@@ -533,7 +535,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 +546,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| name: "fail to create no lock gauge with balancer pool", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| Duration: defaultNoLockDuration, | ||
| }, | ||
| poolId: balancerPoolId, | ||
|
|
||
|
|
@@ -552,6 +556,7 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| name: "fail to create no lock gauge with non-existent pool", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| Duration: defaultNoLockDuration, | ||
| }, | ||
| poolId: invalidPool, | ||
|
|
||
|
|
@@ -561,9 +566,46 @@ func (s *KeeperTestSuite) TestCreateGauge_NoLockGauges() { | |
| name: "fail to create no lock gauge with zero pool id", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| Duration: defaultNoLockDuration, | ||
| }, | ||
| 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: "", | ||
| // 1h is a supported uptime that is not authorized | ||
| Duration: time.Hour, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
| expectErr: true, | ||
| }, | ||
| { | ||
| name: "fail to create external no lock gauge due to entirely invalid uptime", | ||
| distrTo: lockuptypes.QueryCondition{ | ||
| LockQueryType: lockuptypes.NoLock, | ||
| // Note: this assumes the gauge is external | ||
| Denom: "", | ||
| // 2ns is an uptime that isn't supported at all (i.e. can't even be authorized) | ||
| 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 | ||
| Denom: types.NoLockInternalGaugeDenom(concentratedPoolId), | ||
| Duration: time.Nanosecond, | ||
| }, | ||
| poolId: concentratedPoolId, | ||
|
|
||
| expectErr: true, | ||
| }, | ||
| } | ||
|
|
@@ -589,10 +631,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 +812,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
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right – since 1 nanosecond is an authorized uptime by default, the test cases with that duration should be covering the non-failing branches