Lopjf
Lopjf

Reputation: 43

Uniswap addLiquidityETH with large amount of ETH as parameter. What are the risks?

I am creating a smart contract that applies fees on uniswap buys / sells. Portion of these fees are then added back to the Liquidity Pool(LP) and split among 3 recipients.

To do so and keep all fees dynamic I came up with the algorithm below. However I am unsure how safe that is since:

I like this approach for its ability to split all available ETH, even eventual ETH sent to the contract.

However the uniswap documentation specifies: "Always adds assets at the ideal ratio, according to the price when the transaction is executed.". But I am unsure why and what could be the consequences of such a code.

Here is the code snippet.

    function _distributeFee() internal nonReentrant {
        swapTokensForETH(totalDevAmount + totalMarketingAmount + totalOwnerAmount + (totalLiquidity / 2));

        uint256 swapEthBalance = address(this).balance;

        if (totalLiquidity > 0) {
            addLiquidity((totalLiquidity / 2), swapEthBalance);
            totalLiquidity = 0;
        }

        swapEthBalance = address(this).balance;
        
        // send fees to their recipients
        uint256 devFeeETHAmount = swapEthBalance / (devFeePercent + marketingFeePercent + ownerFeePercent) * (devFeePercent);
        uint256 marketingFeeETHAmount = swapEthBalance / (devFeePercent + marketingFeePercent + ownerFeePercent) * (marketingFeePercent);
        uint256 ownerFeeETHAmount = swapEthBalance / (devFeePercent + marketingFeePercent + ownerFeePercent) * (ownerFeePercent);
        payable(devFeeRecipient).transfer(devFeeETHAmount);
        payable(marketingFeeRecipient).transfer(marketingFeeETHAmount);
        payable(ownerFeeRecipient).transfer(ownerFeeETHAmount);
    }

    function swapTokensForETH(uint256 tokenAmount) private {
        address[] memory path = new address[](2);
        path[0] = address(this);
        path[1] = uniswapV2Router.WETH();
        _approve(address(this), address(uniswapV2Router), tokenAmount);
        uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens(
            tokenAmount,
            0, 
            path,
            address(this),
            block.timestamp
        );
    }

    function addLiquidity(uint256 tokenAmount, uint256 ethAmount) private {
        // approve token transfer to cover all possible scenarios
        _approve(address(this), address(uniswapV2Router), tokenAmount);

        // add the liquidity
        uniswapV2Router.addLiquidityETH{value: ethAmount}(
            address(this),
            tokenAmount,
            0, // slippage is unavoidable
            0, // slippage is unavoidable
            address(0), // send LP token to burn address
            block.timestamp
        );
    }

Looked into the uniswap doc but unsure what could be the consequences of this use

Upvotes: 0

Views: 921

Answers (1)

johnny 5
johnny 5

Reputation: 21005

There are tons of Risks to your approach,

  1. you're not protecting again slippage, which allows attackers

     uniswapV2Router.addLiquidityETH{value: ethAmount}(
         address(this),
         tokenAmount,
         0, // slippage is unavoidable
         0, // slippage is unavoidable
     //...
    
  2. You're not setting the minimum amount out during the swaps.

uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens(
        tokenAmount,
        0, 
        path,
        address(this),
        block.timestamp
    );

This means that if an attackers and mev bots can exploit, if they wrap your transactions they can flash loan, if they can't wrap them they can sandwich attack them.

Ideally you need a way to determine what the current market conditions are, whether thats leveraging chainlink, or having a trust provider update general quotes.

Upvotes: 0

Related Questions