[Uptime Incentives]: Incorporate gov param in distribution logic #7419
Conversation
| s.Require().Error(err) | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
After a couple cleanup attempts, it seems a messed up diff is unavoidable here. The core changes are:
- Removed error branch, as the logic covered by these cases should never be erroring (also there were no error cases)
- Moved the passing logic out of the
elseblock, which caused the indentation-related diff below - Updated uptime checks to be against
tc.expectedUptimeinstead of a static default value
Note: i made the changes for 1 in a separate commit in case we want to revert. I find this version of the tests to be much less verbose & more readable, but on the fence on this.
x/incentives/keeper/distribute.go
Outdated
| // | ||
| // Note that this fallback happens even if the gauge is an internal gauge. This is because | ||
| // we want to be able to unauthorize internal gauges as well in the event that there is an | ||
| // issue found with a previously authorized uptime. |
There was a problem hiding this comment.
A secondary reason is that allowing "supported but unauthorized uptimes" to be valid for internal gauges would require hackily bypassing CL incentive record checks, which currently validate against authorized uptimes. I started with trying this and ended up walking back my impl to this version.
| }{ | ||
| "valid case: one poolId and gaugeId": { |
There was a problem hiding this comment.
Note: diff is a bit messy due to driveby test case renaming/removal of expectErr: falses. Happy to undo either of these if anyone has strong qualms.
Closes: #7371
What is the purpose of the change
This PR adds the gov param from #7418 into distribution logic as described in #7370 and #7371. In short, it sets the incentive record's uptime to the gov param if the gauge is an internal gauge.
The fallback logic for unauthorized uptimes is applied to internal gauges as well, meaning that if an unauthorized uptime is set as the internal uptime (or a previously set uptime is now unauthorized), distributions will proceed with the default uptime of 1ns. The reasons for this are explained in PR comments, but the short version is we want to be able to immediately stop distributions to problematic uptimes if a problem is discovered with a non-default one in the future.
Testing and Verifying
Tests can be found in
distribute_test.goDocumentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)