refactor/test(CL): LP methods with lower min spot price #6323
Conversation
| // TODO: remove this limit upon completion of the refactor in: | ||
| // https://github.com/osmosis-labs/osmosis/issues/5726 | ||
| // Due to an intermediary refactor step where we have | ||
| // full range positions created in the extended full range it | ||
| // sometimes tries to swap to the V2 MinInitializedTick that | ||
| // is not supported yet by the rest of the system. | ||
| if targetTick < types.MinInitializedTick { | ||
| return false, false | ||
| } |
There was a problem hiding this comment.
Note to reviewer: this is an acceptance criteria of #6066
| // TODO: swith DefaultMinCurrentTick to types.MinCurrentTickV2 upon | ||
| // completion of https://github.com/osmosis-labs/osmosis/issues/5726 | ||
| DefaultMinCurrentTick = types.MinCurrentTick |
There was a problem hiding this comment.
Note to reviewer: this is added as a sub-task for #5726
| expectedSpreadRewardGrowthOutsideLower: oneEthCoins, | ||
| expectedSpreadRewardGrowthOutsideLower: cl.EmptyCoins, |
There was a problem hiding this comment.
Note to reviewer: I think this is a test setup error where this should have been empty before
p0mvn
changed the title
|
Please do not merge until #6319 is in |
p0mvn
force-pushed
the
roman/tick-to-sqrt-breaking
branch
from
cc7ab11
to
7db80ff
Compare
| // Note that the min sqrt price must be computed from the min tick since that's | ||
| // the assumption made in LP logic. | ||
| lowerSqrtPrice, _ := math.TickToSqrtPrice(lpTest.lowerTick) | ||
| upperSqrtPrice, _ := math.TickToSqrtPrice(lpTest.upperTick) |
There was a problem hiding this comment.
Noting the circular reliance on internal functions to compute expected values – in the past, bugs related to ticks/price precision tended to show up in these internal functions, so we should mentally flag that low-level edge cases might be missed here
There was a problem hiding this comment.
Followed-up in DM. Will resolve
| @@ -26,6 +26,10 @@ import ( | |||
| ) | |||
|
|
|||
| var ( | |||
| // TODO: switch: | |||
There was a problem hiding this comment.
Might be too late for this but I wonder if it would have been beneficial to have a special TODO for precision stuff to make it easier to be sure we don't miss any of these later (e.g. TODO (precision): so we can search for that when #5726 is done)
There was a problem hiding this comment.
Yeah, that's a good idea!
However, I made sure that every single one is tracked in the epic though so not worried about misses:
#5864
This reverts commit 013cd96.
This reverts commit 013cd96.
* revert 6b3273f * Revert "refactor/test(CL): LP methods with lower min spot price (#6323)" This reverts commit 013cd96. * Revert "test(CL): rewards logic to work on the extended price range (#6328)" This reverts commit 3b5d993. * adds check back to fuzzer * Revert "test(CL): swap to new min tick and back (#6327)" This reverts commit e2ef44f. * remove unused imports
Progress towards: #6060
What is the purpose of the change
Adds
CreatePosition,WithdrawPositionandAddToPositionunit tests per vectors suggested in #6060.Refactors
validateTickRangeIsValidto allow the extended full range. As a result, makes this PR state-breaking.Disables fuzz test suite in the extended range. To be re-enabled in #6066
The test suite is made with the goal of minimizing the changes necessary while validating that positions LPed in the extended range function as intended.
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)