Potential issue with testPathIndependenceForSwaps test from part 3/4

[aviggiano]

Hello,

I am fuzzing Uniswap v2 and I believe I have found a potential issue with the testPathIndependenceForSwaps test from part 3/4.

When running echidna with testLimit equals to 100k and 4 workers, the test breaks:

swapExactTokensForTokensPathIndependence(uint256): failed!💥
  Call sequence:
    testProvideLiquidityInvariants(7222,148)
    testRemoveLiquidityInvariants(1412263)
    testSwapTokens(878955802143800039497958245)
    testPathIndependenceForSwaps(0)

I believe this is caused by a wrong assumption that swapping back and forth only costs 3% in fees.

    /*
    Swapping x of testToken1 for y token of testToken2 and back should (roughly) give user x of testToken1.
    The following function checks this condition by assessing that the resulting x is no more than 3% from the original x.
    ...
    */
    ...

        assert((x - xOut) * 100 <= 3 * x); // (x - xOut) / x <= 0.03; no more than 3% loss of funds

Instead, each swap takes 3%, so the back-and-forth maximum loss should be 100% - (100%-3%)*(100%-3%) = 5.91%

[technovision99]

The test doesn't concern the fees taken by uniswap, which is 0.30%, not 3%. The test attempts to test the invariant "swapping X for Y and back should leave the user with X tokens" (bar some precision loss, currently set to 3%). So I believe the test is correct, although it's interesting you found an edge case where this doesn't hold. Could you zip and send your corpus? This might have to do with the pool conditions being strange (it seems there's low and imbalanced liquditity judging from the call sequence, but I'm not 100% sure)

[aviggiano]

Thank you for your response, I stand corrected.

About 3%, was this precision loss defined? Just empirically?

About the corpus, I actually changed a bunch of stuff on the tests, so maybe this won't work out of the box. I'll try to repro on this repo and will send it here if I'm able to do it.

[technovision99]

Yeah the precision loss is defined empirically. I'm not sure if there's any specification that could detail the potential precision loss, but I think 3% is a generous bound given that only a 0.3% fee is taken from each swap. I also think this is a good number to use because it's generally the recommended slippage tolerance