Uncertainty regarding the implementation of the liquidity fee

[Nikol]

@barakman

You haven't provided the exact code of that function, so I'm having to rely on several different versions of it which you have posted throughout this thread.

function _transfer(address from, address to, uint256 amount) internal virtual override {
        uint256 marketingFee = (amount * 2) / 100;
        uint256 liquidityFee = (amount * 2) / 100;
        uint256 fees = marketingFee + liquidityFee;
        uint256 sendAmount = amount - fees;

        super._transfer(from, to, sendAmount);

        if (marketingFee > 0) {
            super._transfer(from, marketingWallet, marketingFee);
        }

        if (liquidityFee > 0) {
            super._transfer(from, address(this), liquidityFee);
        }

        uint256 accumulatedTokenThreshold = 5000000000 * 10**18; //For example,5000000000 tokens

        if (liquidityFee > 0 && from != uniswapV2Pair && balanceOf(address(this)) >= accumulatedTokenThreshold) {
            swapAccumulatedTokensForEth(balanceOf(address(this)));
        }

        uint256 minTimeInterval = 12 hours; // Napríklad 24 hodín
        if (liquidityFee > 0 && from != uniswapV2Pair && address(this).balance >= minEthBeforeLiquidity && (block.timestamp - lastLiquidityAddTime) >= minTimeInterval) {
            addLiquidity(liquidityFee, address(this).balance);
            lastLiquidityAddTime = block.timestamp;
        }
    }

    function swapAccumulatedTokensForEth(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(address(this), address(uniswapV2Router), tokenAmount);
        uniswapV2Router.addLiquidityETH{value: ethAmount}(
            address(this),
            tokenAmount,
            0,
            0,
            owner(),
            block.timestamp
        );
    }
}

I think the problem might be here:

function swapAccumulatedTokensForEth(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
    );
}

This condition is met when the contract reaches:

uint256 accumulatedTokenThreshold = 5000000000 * 10**18; // For example,5000000000 tokens
if (liquidityFee > 0 && from != uniswapV2Pair && balanceOf(address(this)) >= accumulatedTokenThreshold) {
    swapAccumulatedTokensForEth(balanceOf(address(this)));
}

Since this condition is not fulfilled, the addLiquidity function fails, which is what I suspect is occurring.

Do you have any specific suggestions on how I should resolve this issue? Do you think separating the swapping and liquidity addition processes from the basic _transfer function and executing them under the control of the administrator could be a suitable solution?

Thank you for your patience and help.

[barakman]

@Nikol

I think the problem might be here:

function swapAccumulatedTokensForEth(uint256 tokenAmount) private {

AGAIN - there is no indication of how and where you call this function!!!

Is it called from function _transfer?
If yes, then why isn't this a part of your question?
Why do you keep making your readers guess what's going on in your code?

@Nikol

This condition is met when the contract reaches:

uint256 accumulatedTokenThreshold = 5000000000 * 10**18; // For example,5000000000 tokens
if (liquidityFee > 0 && from != uniswapV2Pair && balanceOf(address(this)) >= accumulatedTokenThreshold) {
    swapAccumulatedTokensForEth(balanceOf(address(this)));
}

What condition?
And where exactly in your contract is that piece of code anyway?

@Nikol

Since this condition is not fulfilled, the addLiquidity function fails, which is what I suspect is occurring.

What addLiquidity function are you talking about?
No such function appears anywhere in your question!

[Nikol]

I have revised my previous post for clarity. Thank you for pointing out the need for more detailed information.

[barakman]

So what's this then:

@Nikol

addLiquidity(liquidityFee, address(this).balance);

???

Again, we are left to guess what some of your code actually does.
In this case - an internal function in your contract, called addLiquidity.

Anyways, have you stopped at any point to read this part in one of my previous comments:

@barakman

Inside function _transfer, you seem to be calling function addLiquidity in YOUR contract.

Again, you haven't provided the code of that function, but by name, I am inclined to guess that this function calls function addLiquidity on the Uniswap Router contract...?

If so, then what you get here is a kind of a recursive loop, starting as you add liquidity via the Uniswap web interface:

• Which calls function addLiquidity on the Uniswap Router contract
• Which calls function transferFrom on your contract
• Which calls function _transfer on your contract
• Which calls function addLiquidity on your contract
• Which calls function addLiquidity on the Uniswap Router contract

???

In other words, regardless of whether it is your swapAccumulatedTokensForEth function or your addLiquidity function which calls a function on the Uniswap Router contract, you still get to a point where that Uniswap Router contract function calls back your contract's _transfer function.

Please try to actually stop and think about it this time.

Your internal _transfer function should generally be very simple, handling token-transfer only, and not complex stuff like adding liquidity, etc.

[Nikol]

@Nikol

addLiquidity(liquidityFee, address(this).balance);

So, do I need to separate these functions from _transfer ? So that they are not called together. Is there a possibility that the accumulated tokens from the tax (2% tax for liquidity) will be automatically exchanged for ETH at a certain value and sent to the liquidity pool?

[barakman]

I don't know. Your question is way too messy by now, not focused on one single issue, and not containing a coherent piece of information (code, etc) required in order to be able to properly refer to the multiple different issues raised throughout this thread.

I suggest starting a new question, which based on everything that you've learned here so far, will be constructed as follows:

• A high-level description of the actual issue including the desired product requirement if relevant
• A description of the actual flow (set of transactions), which when executed, leads to the problem
• ALL the relevant code and ONLY the relevant code

[Nikol]

Hello Barakman

I would like to inform you about some key changes that I have recently made to my smart contract. These changes are aimed at improving functionality and security.
Changes to the Contract:

1. Separation of Liquidity Functions from _transfer Function: In an effort to keep the _transfer function as simple and clean as possible, I have separated the operations related to liquidity. This separation increases the clarity and security of the code.
2. Addition of New Functions for Liquidity Management: I have implemented new functions: manageLiquidity, canSwapTokensForEth, canAddLiquidity, and shouldPerformLiquidityFunctions. These functions ensure that swap operations and the addition of liquidity occur efficiently and only under predefined conditions.

• The manageLiquidity function coordinates the entire liquidity management process.
• canSwapTokensForEth decides whether it is appropriate to swap tokens for ETH.
• canAddLiquidity determines whether it is possible to add liquidity to the pool.
• shouldPerformLiquidityFunctions ensures that liquidity operations are triggered only in the right context.

3. Security and Optimization: This separation provides better protection against potential security threats and enhances the efficiency and clarity of the code.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

// Importing OpenZeppelin contracts for ERC20 token standard and its extensions.
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/ERC20Burnable.sol"; // Allows token burning.
import "@openzeppelin/contracts/security/ReentrancyGuard.sol"; // Prevents reentrancy attacks.
import "@openzeppelin/contracts/access/Ownable.sol"; // Provides ownership control functionalities.
import "@uniswap/v2-periphery/contracts/interfaces/IUniswapV2Router02.sol"; // Interface for Uniswap V2 Router.
import "@uniswap/v2-core/contracts/interfaces/IUniswapV2Factory.sol"; // Interface for Uniswap V2 Factory.

// CustomToken1 contract inheriting functionalities from ERC20, Burnable, ReentrancyGuard, and Ownable.
contract CustomToken1 is ERC20, ERC20Burnable, ReentrancyGuard, Ownable {
    address public marketingWallet; // Address for marketing wallet.
    address public uniswapV2RouterAddress; // Address of the Uniswap V2 Router.
    IUniswapV2Router02 public uniswapV2Router; // Instance of the Uniswap V2 Router.
    address public uniswapV2Pair; // Address of the Uniswap V2 Pair.

    uint256 public constant TOTAL_SUPPLY = 450000000000000 * 10**18; // Total token supply.
    uint256 public immutable minEthBeforeLiquidity = 0.05 ether; // Minimum ETH balance before adding liquidity.
    uint256 public lastLiquidityAddTime; // Timestamp of the last liquidity addition.
    uint256 public constant accumulatedTokenThreshold = 1000000000 * 10**18; // Predefined threshold for token swap.
    uint256 public constant minTimeInterval = 24 * 60 * 60; // Predefined time interval (24 hours).

    // Constructor to initialize the contract with specific parameters.
    constructor(
        address _marketingWallet,
        address _uniswapV2RouterAddress,
        address initialOwner
    ) ERC20("Testing4", "RRR") {
        marketingWallet = _marketingWallet;
        uniswapV2RouterAddress = _uniswapV2RouterAddress;
        uniswapV2Router = IUniswapV2Router02(uniswapV2RouterAddress);
        uniswapV2Pair = IUniswapV2Factory(uniswapV2Router.factory()).createPair(address(this), uniswapV2Router.WETH());
        _mint(initialOwner, TOTAL_SUPPLY);
    }

    // Overriding the _transfer function to include marketing and liquidity fees.
    function _transfer(address from, address to, uint256 amount) internal virtual override {
        uint256 marketingFee = (amount * 2) / 100;
        uint256 liquidityFee = (amount * 2) / 100;
        uint256 fees = marketingFee + liquidityFee;
        uint256 sendAmount = amount - fees;

        super._transfer(from, to, sendAmount);

        if (marketingFee > 0) {
            super._transfer(from, marketingWallet, marketingFee);
        }

        if (liquidityFee > 0) {
            super._transfer(from, address(this), liquidityFee);
        }

        // Executing liquidity management functions if criteria are met.
        if (shouldPerformLiquidityFunctions(from)) {
            try this.manageLiquidity() {} catch {}
        }
    }

    // Managing liquidity by swapping tokens for ETH and adding liquidity.
    function manageLiquidity() public {
        if (canSwapTokensForEth()) {
            swapAccumulatedTokensForEth(balanceOf(address(this)));
        }
        
        if (canAddLiquidity()) {
            addLiquidity(balanceOf(address(this)), address(this).balance);
        }
    }

    // Checking if the contract can swap tokens for ETH.
    function canSwapTokensForEth() private view returns (bool) {
        uint256 contractTokenBalance = balanceOf(address(this));
        return contractTokenBalance >= accumulatedTokenThreshold;
    }

    // Checking if the contract can add liquidity.
    function canAddLiquidity() private view returns (bool) {
        uint256 ethBalance = address(this).balance;
        return ethBalance >= minEthBeforeLiquidity && (block.timestamp - lastLiquidityAddTime) >= minTimeInterval;
    }

    // Determining if liquidity functions should be performed.
    function shouldPerformLiquidityFunctions(address from) private view returns (bool) {
        return from != uniswapV2Pair && from != address(uniswapV2Router);
    }

    // Function to swap accumulated tokens for ETH.
    function swapAccumulatedTokensForEth(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 to add liquidity to Uniswap.
    function addLiquidity(uint256 tokenAmount, uint256 ethAmount) private {
        _approve(address(this), address(uniswapV2Router), tokenAmount);
        uniswapV2Router.addLiquidityETH{value: ethAmount}(
            address(this),
            tokenAmount,
            0,
            0,
            owner(),
            block.timestamp
        );
    }
}

**The contract is currently in the testing phase. The token tax distribution is working correctly. I am particularly focused on verifying the functionality of swapping tokens for ETH. Everything looks promising so far, but I am still waiting for confirmation that everything is functioning as expected.

I am very grateful for all your support and advice. If you have any further suggestions or insights for improvement, I would be glad to hear them.

Best regards, Nikol

[barakman]

@Nikol

    function manageLiquidity() public {

This (a public function with no access-restriction) looks like a potential security breach.