# 2023-02-08 Test calcPriceFromAddX Is Inverse Of calcXFromPriceDelta
* when `sqrtP` ~= `MIN_SQRT_RATIO`, price can't really be pushed down any further so calcXFromPriceDelta isnt accurate at all. the x returned seems to almost always be 0, even if we are trying to add x size that is orders of magnitude larger than the existing liquidity.
* when running `swapXForYGivenX` with such a low pool price, the swap will revert with the `TickIndexImpl:SqrtMax` reason. This error comes up if the absVal of the tick the price is being pushed into exceeds the max tick (|MIN_TICK | = MAX_TICK )
* This is making creating a fuzz test for a little difficult: it's hard to determine a heuristically sound cutoff for max and min prices for the start price, liquidity and x
* _Working Solution:_
* `x < (1 << 38) `liq + x will always be within what we can fit in a uint128
* `liq < 340282366920938463463374607431768211455` max uint128
* `MIN_SQRT_RATIO * 1e9 < sqrtP && sqrtP < MAX_SQRT_RATIO` give a little bit of room for the price to be pushed down to prevent the tick overflow error mentioned above
* `x>1` x just has to be positive and not 0
* `sqrtP > 0` for whatever reason I had to add this even though `MIN_SQRT_RATIO` is 4295128739 which is greater than 0 but whatever
* maximum acceptable delta between x and expX is 1e12. that's just the lowest that I got the tests to pass. 1e12 eth is worth $0.0016 rn so it doesn't seem to be too big of a deal. Also these large deltas only occur at the extremes of the price range (the closer to `MIN_SQRT_RATIO`, the higher the delta), so in practice if this happens the real-world value loss would be almost immeasurablably low (correct me if I'm wrong here in my thinking). The user eats this difference rather that the protocol, because the expected X calculated from the delta of the fuzzed price value passed in and the fuzzed x is always less than the fuzzed x passed in:
```
Price newSP = SwapMath.calcNewPriceFromAddX(liq, Price.wrap(sqrtP), x);
uint256 expX = SwapMath.calcXFromPriceDelta(newSP, Price.wrap(sqrtP), liq, true);
assertTrue(expX <= x) //passes
assertAlmostEq(x, expX, 1e12); //passes
```
meaning that the code uses more x up than it should for a given price delta (by a small amount)
# 2/9 (and beyond) additional notes
For high prices (even with moderate x size) we overflow in calcNewPriceFromAddX() when we multiply x by the price and try and store it in a uint256. (~line 38):
```
uint256 xrp = x * rp; //overflows
```
This overflows with values as low as x = 871509787656907713528983453696 (~8e29 or 800,000,000,000 in ether) multipled with the price 275240518046957061693889572882779184052551286784 (~2e47) or about 2e29 ether (200,000,000,000,000,000,000,000,000,000). Typing this out now makes me realize how big of an edge case this is. If someone tries to trade the sun and the earth we get an overflow
# test CalcPriceFromSubX() Is Inverse Of CalcXFromPriceDelta()
High prices are causing similar issues in this test as low prices were in the last one.
## {Suspected} Problem with `calcNewPriceFromSubX`: removing x is pushing price down (for large sqrtPrice), should push price up
I suspect this is doe to an overflow due to how in examples the start price is super high, and the end price is very very low, making me think it's wrapping around.
__Example:__
`sqrtP: 300476407847917070821640784539769403464067957526`
`liq: 9918`
`x: 7631`
`newSP: 2040` - low af
Lines ~69 - 80 in Math.sol:
```
uint160 rp = Price.unwrap(oldSqrtPrice);
uint256 xrp = x * rp;
if ((xrp / x) == rp) { // If we don't overflow
uint256 shortDenom;
unchecked {
shortDenom = liqX96 - xrp;
}
if (shortDenom > liqX96) { // Check the denom hasn't underflowed
// This might go over so we have to safe cast.
return Price.wrap(FullMath.mulDivRoundingUp(liqX96, rp, shortDenom).toUint160());
}
}
```
`x*rp = 2292935468287455167439940826822980317834302583880906`
`liqX96 = 785784915816473700252768899432448`
`shortDenom (liqX96 - xrp) = 115792089237316195423570982715752439565814818011484653032950966426379445191478`
In this case, shortDenom is greater than liqX96, so we underflow.
This seems to only be an issue when the price gets to around a million quadrillion (1e30) and the liquidity is low (like a thousand). X is less than the liquidity. In practice I dont think this would be an issue but perhaps warrants a revert in the code or something (i havent noticed any reverts that would catch this but I could have missed it)
Also going to have to come back to this test, getting an error saying that the `vm.assume` cheatcode rejected too many inputs (65536 allowed). Fuzzing tests to remove x needs more input restrictions since there is a finite amount of x to be removed to the pool (bounded by the liquidity). Additionally the price extremes need to be restricted. Not sure if it's practical to fuzz test this one or I need to rethink how I'm doing it
# Test CalcPriceFromAddY Is Inverse Of CalcYFromPriceDelta
Used the same vm.assume() restrictions as CalcPriceFromAddX() test and tests pass. Also, the expected Y from the price delta is always <= the y the user is trying to spend. This also occurs at the low extreme of the price range. User eats whatever tiny difference there is
# Test CalcPriceFromSubY Is Inverse Of CalcYFromPriceDelta
This test passes with the following restrictions:
```
vm.assume(MIN_FUZZ_PRICE < sqrtP && sqrtP < MAX_FUZZ_PRICE);
vm.assume(y > 0);
vm.assume(liq > y);
```
*Fuzz prices are restricted to the middle 2/3 of the price range (1/6 taken off each side)
# 2023-02-10 Continued from yesterday
When the sqrtPrice gets up to around (decimals added to aid practical reasoning): 243,574,450,580,868,350,547,878,842,033.998137066699935673
* corresponding price (non-sqrt): 5.932851297577188e58
* y amount requested: 80429494640
* y amount calculated from price delta (amount to be sent to user): 79429494639
* excess amount overspent: 1e9 (0.000000001)
* value of assets overspent: 5.932851297577189e49
* These prices are impractically high
I was initally incorrect when anaylizing the practical implications for the difference between the y and expected y caluclated from the delta. The y calculated from the delta being lower actually means that less y is being calculated to be sent back, as the price delta is smaller. This also means that less x is being used. These larger differences (still a very small token amount, around a billionth of an eth) occur at unrealisticly high price levels.
## Continuation of test CalcPriceFromSubX() Is Inverse Of CalcXFromPriceDelta():
The issues stemming from this test seem to all be possibly related to the issue of x * sqrtP being a large margin greater than the liquidity. When subtracted (as mentioned above) we get an overflow that behaves differently than what is expected(or i misunderstand the comments, which is very likely). In practice, this wouldn't happen since the x provided would cause for more y than there exists liquid in the pool to be sent to the user.