You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
returnfmt.Errorf("'no lock' gauge must have duration set to 0, was (%d)", m.DistributeTo.Duration)
}
Thus, external gauges are executed using the 0 duration in their key, while internal gauges are executed using the gauge duration (set to 1 epoch) as their key. The additional 0-duration link stored in state for internal gauges goes unused.
Note that this should be functionally indistinguishable from if we had stored gauge -> pool links in state with gauge duration instead of hard coding 0, since for external gauges the duration is always 0 anyways and for internal gauges the hard coded link goes unused anyways.
However, once we relax the 0 duration requirement for external gauges, this hard coded link will matter. Thus, we should ensure that it is updated by the time we remove the validate basic check.
Suggested Design
Remove the following check from ValidateBasic to allow for non-zero durations to be used on CL gauges:
This should already be addressed in #7369, but if it isn’t yet, an invalid or unauthorized gauge duration should fall back to the default uptime of 1ns in this line (instead of directly using the gauge’s duration):
This should make it so all existing CL gauges can be properly read without a state migration and automatically fall back to the default uptime of 1ns (as they currently are).
Note that while existing NoLock gauges have technically invalid durations of 0 seconds, the fact that the gauge duration and the duration in state are consistent means that the approach above should handle this cleanly.
Testing Strategy
Creating a gauge with 0 duration is likely not possible to do since the validation is baked into CreateGauge, but it might be worthwhile to attempt to overwrite the gauge and link in state with 0 durations after it is created and then run distribution logic.
In other words, this functionality can be covered by:
Creating a gauge
Overwriting the gauge in state with an identical gauge that has 0 duration (using setGauge)
Overwriting the link in state with a 0 duration (using SetPoolGaugeIdNoLock)
Running epoch and checking that the gauge was properly distributed with default (1ns) uptime
Note that this is somewhat hacky and might cause unexpected issues while implementing.
Acceptance Criteria
Changes in suggested design are all incorporated and distribution of 0 duration gauges are tested
The text was updated successfully, but these errors were encountered:
Background
Gauge distribution logic for CL gauges currently leans on a gauge -> pool link to find which gauge:
osmosis/x/incentives/keeper/distribute.go
Lines 591 to 592 in 7c81b90
This implementation, however, is exceptionally hacky. While the reads are all based on gauge duration, the writes are as follows:
This currently works because in
ValidateBasic
, we ensure that the gauge duration for externally created gauges are zero:osmosis/x/incentives/types/msgs.go
Lines 76 to 78 in 7c81b90
Thus, external gauges are executed using the 0 duration in their key, while internal gauges are executed using the gauge duration (set to 1 epoch) as their key. The additional 0-duration link stored in state for internal gauges goes unused.
Note that this should be functionally indistinguishable from if we had stored gauge -> pool links in state with gauge duration instead of hard coding 0, since for external gauges the duration is always 0 anyways and for internal gauges the hard coded link goes unused anyways.
However, once we relax the 0 duration requirement for external gauges, this hard coded link will matter. Thus, we should ensure that it is updated by the time we remove the validate basic check.
Suggested Design
Remove the following check from
ValidateBasic
to allow for non-zero durations to be used on CL gauges:osmosis/x/incentives/types/msgs.go
Lines 76 to 78 in 7c81b90
Then, ensure that the hard coded value here uses gauge duration (this will likely be hit in another PR by the time this issue is addressed:
osmosis/x/pool-incentives/keeper/keeper.go
Line 176 in 7c81b90
This should already be addressed in #7369, but if it isn’t yet, an invalid or unauthorized gauge duration should fall back to the default uptime of 1ns in this line (instead of directly using the gauge’s duration):
osmosis/x/incentives/keeper/distribute.go
Lines 629 to 630 in 7c81b90
This should make it so all existing CL gauges can be properly read without a state migration and automatically fall back to the default uptime of 1ns (as they currently are).
Note that while existing NoLock gauges have technically invalid durations of 0 seconds, the fact that the gauge duration and the duration in state are consistent means that the approach above should handle this cleanly.
Testing Strategy
Creating a gauge with 0 duration is likely not possible to do since the validation is baked into
CreateGauge
, but it might be worthwhile to attempt to overwrite the gauge and link in state with 0 durations after it is created and then run distribution logic.In other words, this functionality can be covered by:
setGauge
)SetPoolGaugeIdNoLock
)Note that this is somewhat hacky and might cause unexpected issues while implementing.
Acceptance Criteria
The text was updated successfully, but these errors were encountered: