Uniswap / v2-core

🦄 🦄 Core smart contracts of Uniswap V2
https://uniswap.org/docs
GNU General Public License v3.0
2.92k stars 3.13k forks source link

code=UNPREDICTABLE_GAS_LIMIT #119

Open aress31 opened 3 years ago

aress31 commented 3 years ago

I am in the process of implementing an ERC20 token with a basic add to liquidity feature, this is the relevant code snippet:

    // Track: https://forum.openzeppelin.com/t/updating-balance-on-inherited-contracts/8133
    function _beforeTokenTransfer(address sender, uint256 amount)
        internal
        returns (uint256)
    {
        uint256 updatedAmount = amount;

        if (_liquidityTax != 0) {
            uint256 liquidityFee = (amount * _liquidityTax) / (10**2);

            _balances[sender] -= liquidityFee;
            _balances[address(this)] += liquidityFee;

            bool overMinTokensRequiredBeforeSwap =
                _balances[address(this)] >= _minTokensRequiredBeforeSwap;

            if (
                overMinTokensRequiredBeforeSwap &&
                !_inSwapAndLiquify &&
                _isAutoSwapAndLiquify
            ) {
                uint256 halfContractBalance = _balances[address(this)] / 2;
                uint256 ethReceived = _swap(halfContractBalance);
                (uint256 amountETH, uint256 amountToken) =
                    _liquify(halfContractBalance, ethReceived);

                _totalLiquidityETH += amountETH;
                _totalLiquidity += amountToken;
            }

            updatedAmount -= liquidityFee;
        }

        return updatedAmount;
    }

    receive() external payable {}

    function _swap(uint256 tokensToSwap) private returns (uint256) {
        // contract's current BNB/ETH balance
        uint256 initialBalance = address(this).balance;

        // swap half of the tokens to ETH
        address[] memory path = new address[](2);
        path[0] = address(this);
        path[1] = _uniswapV2Router.WETH();

        _approve(address(this), address(_uniswapV2Router), tokensToSwap);

        // Swap tokens for ETH/BNB
        _uniswapV2Router.swapExactTokensForETHSupportingFeeOnTransferTokens(
            tokensToSwap,
            0,
            path,
            address(this), // this contract will receive the ETH that were swapped from the token
            block.timestamp
        );

        // Figure out the exact amount of tokens received from swapping
        // Check: https://github.com/Uniswap/uniswap-v2-periphery/issues/92
        uint256 ethReceived = address(this).balance - initialBalance;

        emit SwapExactTokensForETHSupportingFeeOnTransferTokens(
            tokensToSwap,
            ethReceived
        );

        return ethReceived;
    }

    function _liquify(uint256 tokensToAddToLiq, uint256 ethReceived)
        private
        returns (uint256, uint256)
    {
        // To cover all possible scenarios, msg.sender should have already given
        // the router an allowance of at least amountTokenDesired on token.
        _approve(address(this), address(_uniswapV2Router), ethReceived);

        (uint256 amountToken, uint256 amountETH, ) =
            _uniswapV2Router.addLiquidityETH{value: ethReceived}(
                address(this),
                tokensToAddToLiq, //amountTokenDesired
                0, // amountTokenMin
                0, // amountETHMin
                owner(),
                block.timestamp
            );

        emit AddLiquidityETH(amountToken, amountETH);

        return (amountETH, amountToken);
    }

When deploying this token to ropsten and after having manually added liquidity to the pool, I am unable to buy the token.

See:

image

Swap failed: cannot estimate gas; transaction may fail or may require manual gas limit (error={"code":-32603,"message":"execution reverted: UniswapV2: TRANSFER_FAILED","data":{"originalError":{"code":3,"data":"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000001a556e697377617056323a205452414e534645525f4641494c4544000000000000","message":"execution reverted: UniswapV2: TRANSFER_FAILED"}}}, method="estimateGas", transaction={"from":"0x8c5ABc25C0cfD9D62E469340fD7d812a742090AC","to":"0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D","value":{"type":"BigNumber","hex":"0x016cde"},"data":"0xfb3bdb410000000000000000000000000000000000000000000000056bc75e2d6310000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000008c5abc25c0cfd9d62e469340fd7d812a742090ac00000000000000000000000000000000000000000000000000000000609b07930000000000000000000000000000000000000000000000000000000000000002000000000000000000000000c778417e063141139fce010982780140aa0cd5ab00000000000000000000000050c39e87a915910520e8cc2cf594ae2cc547b5d6","accessList":null}, code=UNPREDICTABLE_GAS_LIMIT, version=providers/5.1.1)

The contract address is: https://ropsten.etherscan.io/address/0x50c39e87a915910520e8cc2cf594ae2cc547b5d6

Could anyone please assist with this?

Jorropo commented 3 years ago

This is clearly an issue with your code (as seen by the UniswapV2: TRANSFER_FAILED).

An rgrep for this yields one result :

$ rgrep "TRANSFER_FAILED" uniswap-v2-*
uniswap-v2-core/contracts/UniswapV2Pair.sol:        require(success && (data.length == 0 || abi.decode(data, (bool))), 'UniswapV2: TRANSFER_FAILED');

This is in the safeTransfer function :

https://github.com/Uniswap/uniswap-v2-core/blob/4dd59067c76dea4a0e8e4bfdda41877a6b16dedc/contracts/UniswapV2Pair.sol#L44-L47

This mean uniswap is attempting to call transfer but it's failing somehow (not enough balance, your token has a reentrency lock , ... who knows ?).

I have a few issues with this code tho :

If you want to debug this better I think you would need to fork uniswap and removing the bytes override in the safe transfers (so you could get a meaning full revert message) (or use something like the javascript VM and debug opcode by opcode the transaction). I think the best advice I can give is just to read uniswap V2's code, it is probably way simpler that you think, it will remove all questions you have about uniswap V2 (how to compute tokens after a swap, how to do a swap / liquidity add) (all the hard part of bullet proofing and maths has already been done, just reading it mostly everything makes sense).

aress31 commented 3 years ago

@Jorropo thanks for this complete answer. My first post litterally contains all the relavant code, not sure if the following code will make things clearer:

function _transfer(
        address sender,
        address recipient,
        uint256 amount
    ) internal override notNull(amount) {
        require(sender != address(0), "ERC20: transfer from the zero address");
        require(recipient != address(0), "ERC20: transfer to the zero address");
        require(
            sender != recipient,
            "_transfer: 'sender' cannot also be 'recipient'"
        );

        if (!_isExcludedFromPayingFees[sender]) {
            amount = _beforeTokenTransfer(sender, amount);
        }

        uint256 senderBalance = _balances[sender];
        require(
            senderBalance >= amount,
            "ERC20: transfer amount exceeds balance"
        );
        _balances[sender] = senderBalance - amount;
        _balances[recipient] += amount;

        emit Transfer(sender, recipient, amount);
    }

constructor(
        string memory name_,
        string memory symbol_,
        uint8 decimals_,
        uint256 totalSupply_,
        address router_,
        uint8 liquidityTax_
    ) ERC20(name_, symbol_) {
        _decimals = decimals_;
        _liquidityTax = liquidityTax_;
        _minTokensRequiredBeforeSwap = 10**6 * 10**_decimals;
        _uniswapV2Router = IUniswapV2Router02(router_);

        address pair =
            IUniswapV2Factory(_uniswapV2Router.factory()).getPair(
                _uniswapV2Router.WETH(),
                address(this)
            );

        // Pair not yet created
        if (pair == address(0)) {
            _uniswapV2Pair = IUniswapV2Factory(_uniswapV2Router.factory())
                .createPair(_uniswapV2Router.WETH(), address(this));
        } else {
            _uniswapV2Pair = pair;
        }

        setIsExcludedFromPayingFees(address(this), true);
        setIsExcludedFromPayingFees(owner(), true);

        _mint(_msgSender(), totalSupply_ * (10**decimals_));
    }

Its a really interesting suggestion that you made about using the pair directly rather than the router, however the pair doesn't export any swapping or liquifying function.

Also I vaguely heard about sandwich attacks and oracles but in a first time I would like to make this basic swap and liquify token works.

As you can see there is no lock and the balance should be sufficient for the swap.

Does this additional code speaks to you?

EDIT: I tried to troubleshot the code a bit by commenting out the liquify function and the issue comes indeed from the swap function... This is really weird since I meet all the requirements for the function to work!