fix/test: swap over contiguously initialized ticks with spacing of one #5582
Conversation
| // computeExpectedValuesForTestOneForZero returns the tick to swap to during estimate computation and amountIn multiplier. | ||
| // It most cases, the tick to swap to is the same as the expected tick to reach after the swap and the multiplier is 1. | ||
| // The only exception is when performing a second swap in the same direction within the same tick. | ||
| // In such a case, we need to run our estimate logic one tick further to ensure that our estimate is non-zero. | ||
| // However, we discount the amountIn by half to ensure that the tick is not crossed. | ||
| computeNextTickToReachAndMultiplier := func(isZeroForOne bool, expectedSwapEndTick int64, shouldStayWithinTheSameTickInCompute bool) (int64, sdk.Dec) { | ||
| if shouldStayWithinTheSameTickInCompute { | ||
| nextTickToReachInCompute := expectedSwapEndTick | ||
| if isZeroForOne { | ||
| nextTickToReachInCompute = nextTickToReachInCompute - 1 | ||
| } else { | ||
| nextTickToReachInCompute = nextTickToReachInCompute + 1 | ||
| } | ||
|
|
||
| return nextTickToReachInCompute, sdk.NewDecWithPrec(5, 1) | ||
| } | ||
|
|
||
| return expectedSwapEndTick, sdk.OneDec() | ||
| } |
There was a problem hiding this comment.
Can you explain a little more about the multiplier here? I later realized we used 50 percent to stay within the same tick, but it was not clear when initially reading the code here.
There was a problem hiding this comment.
You are correct. We use a 50% discount to stay within the same bucket. 50% picked arbitrarily. If we don't discount, the next tick will be crossed as we are running the token in estimation logic between ticks. For some test cases, we specifically want to test staying within the same bucket.
Let me know if there is anything else that I should elaborate on
There was a problem hiding this comment.
Can you add that the 50 percent is arbitrary in the comments to make it easier for other reviewers?
p0mvn
added
V:state/compatible/no_backport
p0mvn
added
V:state/breaking
Closes: #5558, #5571, 5574
What is the purpose of the change
Adding swap test vectors for out given in where contiguous ticks are initialized with spacing of one. Both zero for one and one for zero are covered
This test helped uncover 2 problems that were separated into the following PRs:
This PR also addresses removing the hack described here: #5571
It also addresses #5574 with investigations done in
TestSwapOutGivenIn_PriceLimit_TickCross_ZeroForOneto swap with sqrt price limit at tick boundaries and asserting that ticks are crossed as expected. I was unable to construct a test case that would reproduce the behavior defined in #5574Testing and Verifying
This change adds a test.
Documentation and Release Note
Unreleasedsection ofCHANGELOG.md?Where is the change documented?
x/{module}/README.md)