Section 5 - New bug: TSwapPool functions getPriceOfOneWethInPoolTokens() and getPriceOfOnePoolTokenInWeth() return incorrect values

[paprikrumplikas]

Hi,

As the title says, the return values of TSwapPool::getPriceOfOneWethInPoolTokens and TSwapPool::getPriceOfOnePoolTokenInWeth are incorrect. They both return the amount of output tokens after fees, which is not the same as the price of 1 output token in input tokens.

(As compared to a fee-less protocol, if there are fees, the amount of output tokens should be lower, while the price should be not lower but higher.)

Incorrect:

function getPriceOfOneWethInPoolTokens() external view returns (uint256) {
        return getOutputAmountBasedOnInput(
            1e18, i_wethToken.balanceOf(address(this)), i_poolToken.balanceOf(address(this)));
    }

Correct:

 function getPriceOfOneWethInPoolTokens() external view returns (uint256) {       
        uint256 precision = 1e18;
        uint256 amountOfPoolTokensReceivedForOneWeth = getOutputAmountBasedOnInput(
            1e18, i_wethToken.balanceOf(address(this)), i_poolToken.balanceOf(address(this)));
            
        uint256 priceOfOneWethInPoolTokens = (1e18 * precision) / amountOfPoolTokensReceivedForOneWeth;

     return priceOfOneWethInPoolTokens;
    }

The same applies to getPriceOfOnePoolTokenInWeth.

[paprikrumplikas]

Test this by inserting the following piece of code in TSwapPool.t.sol:

/**
     * @notice In scenarios where inputAmount is close to the minOutputAmount, even a small loss of precision can lead
     * to the outputAmount falling below minOutputAmount, triggering the TSwapPool__OutputTooLow error.
     */
    function test_incorrectPriceValueReturnedByGetPriceOfOneWethInPoolTokens() public {
        uint256 precision = 1 ether;

        // we need more liquidity in the pool, so granting additional money for the provider
        weth.mint(liquidityProvider, 800e18);
        poolToken.mint(liquidityProvider, 800e18);

        // providing liquidity to the pool
        uint256 initialLiquidity = 1000e18;

        vm.startPrank(liquidityProvider);
        weth.approve(address(pool), initialLiquidity);
        poolToken.approve(address(pool), initialLiquidity);

        pool.deposit({
            wethToDeposit: initialLiquidity,
            minimumLiquidityTokensToMint: 0,
            maximumPoolTokensToDeposit: initialLiquidity,
            deadline: uint64(block.timestamp)
        });
        vm.stopPrank();

        uint256 incorrectPriceOfOneWeth = pool.getPriceOfOneWethInPoolTokens();
        uint256 correctPriceOfOneWeth = (1e18 * precision) / incorrectPriceOfOneWeth;

        console.log("Incorrect price: ", incorrectPriceOfOneWeth); // 987_158_034_397_061_298 = 9.87*10**17
        console.log("Correct price: ", correctPriceOfOneWeth);

        address userSuccess = makeAddr("userSuccess");
        address userFail = makeAddr("userFail");

        // userFail attempts to buy 1 weth with a balance of pool tokens that equals the incorrect price of 1 weth
        poolToken.mint(userFail, incorrectPriceOfOneWeth);
        vm.startPrank(userFail);
        poolToken.approve(address(pool), type(uint256).max);
        // expect a revert (specifically, TSwapPool__OutputTooLow)
        vm.expectRevert();
        // using swapExactOutput() would be more appropriate here, but that one has a huge bug
        pool.swapExactInput({
            inputToken: poolToken,
            inputAmount: incorrectPriceOfOneWeth,
            outputToken: weth,
            minOutputAmount: 99999e13, // due to precision loss, we cant really expect to get 1 full weth (1000e15), we
                // can only approximate
            deadline: uint64(block.timestamp)
        });
        vm.stopPrank();

        // userSuccess attempts to buy 1 weth with a balance of pool tokens that equals the correct price of 1 weth
        poolToken.mint(userSuccess, correctPriceOfOneWeth);
        vm.startPrank(userSuccess);
        poolToken.approve(address(pool), type(uint256).max);
        // using swapExactOutput() would be more appropriate here, but that one has a huge bug
        pool.swapExactInput({
            inputToken: poolToken,
            inputAmount: correctPriceOfOneWeth,
            outputToken: weth,
            minOutputAmount: 99999e13, // due to precision loss, we cant really expect to get 1 full weth (1000e15), we
                // can only approximate
            deadline: uint64(block.timestamp)
        });
        vm.stopPrank();

        assert(weth.balanceOf(userSuccess) > 999e15); // has nearly 1 full weth
        assertEq(poolToken.balanceOf(userSuccess), 0); // spent all his poolToken
    }

[PatrickAlphaC]

Well done :)

[akhilmanga]

Hi, @paprikrumplikas , I understood nothing, and how can we write tests like you? I'm unable to write the tests. How can I connect with you (social media?). Explain this bug, @PatrickAlphaC

[PatrickAlphaC]

@akhilmanga we teach how to write these in the foundry full course! Have you taken that course yet?

[akhilmanga]

I completed that course. But I did not spend time on writing tests on my own.

[farismln]

you should try to write it as it would be faster to understand the test itself @akhilmanga