Modifications on Safemoon contract, MultiSig and Time lock

Smart Contracts
, , ,
1

Hi,

I have tried to modify the Safemoon contract adding new functions and incorporating recommendations given by the CertiK audit team.

So, I am wondering if anyone can help me to understand the following things and overcome the issues?.

  1. Have I done modifications correctly (Listed on bottom).

  2. There are two "getOwner" functions on "Read Contract" after deployment. Can I remove one?.

  3. The function "getUnlockTime" does not reset to zero on "Read Contract" once lock time is exceeded.

  4. Is deliver function necessary and how can I correct it. (As per Certik audit - SSL-12)

  5. How can I identify redundant codes easily.

  6. Would you recommend introducing anything else to improve the contract and safety.

  7. How can I add MultiSig wallet and Time Lock features to this contract.

My experience in this field is not more than 3 months, therefore I appreciate any of your advice, link for a source, and correction.

Thank you very much in advance.

Relevant Links

I have done the following modifications to the source code.

  1. All ERC20, ETH related Names and Comments changed to BEP20, BNB

  2. Added the following function on IBEP20 interface.

  /**
     * @dev Returns the token decimals.
     */
    function decimals() external view returns (uint8);

    /**
     * @dev Returns the token symbol.
     */
    function symbol() external view returns (string memory);

    /**
     * @dev Returns the token name.
     */
    function name() external view returns (string memory);

    /**
     * @dev Returns the bep token owner.
     */
    function getOwner() external view returns (address);
  1. Added "function getTime" under Contract context.
    /**
     * @dev Returns the current time.
     */
        function getTime() public view returns (uint256) {
        return block.timestamp;
    }
  1. Replaced dead wallet from "0x0000000000000000000000000000000000000000" to "0x000000000000000000000000000000000000dEaD" and excluded it from fee.
    uint256 public _burnFee = 1;
    address public deadWallet = 0x000000000000000000000000000000000000dEaD;    <<<Here
    uint256 private _previousBurnFee = _burnFee;

        //exclude owner, this contract and burn wallet from fee
        _isExcludedFromFee[owner()] = true;
        _isExcludedFromFee[address(this)] = true;
        _isExcludedFromFee[deadWallet] = true; <<<Here
  1. Replaced liquidity receiver, owner() with address(this). (As per Certik audit - SSL-04) 
    function addLiquidity(uint256 tokenAmount, uint256 ethAmount) private {
        // approve token transfer to cover all possible scenarios
        _approve(address(this), address(pancakeswapV2Router), tokenAmount);

        // add the liquidity
        pancakeswapV2Router.addLiquidityETH{value: ethAmount}(
            address(this),
            tokenAmount,
            0, // slippage is unavoidable
            0, // slippage is unavoidable
            address(this),<<<Here
            block.timestamp
  1. Removed the following code from the contract.(As per Certik audit - SSL-02) 
!_isExcluded[sender] && !_isExcluded[recipient]

Full code:

    //this method is responsible for taking all fee, if takeFee is true
    function _tokenTransfer(address sender, address recipient, uint256 amount) private {
        if(_isExcludedFromFee[sender] || _isExcludedFromFee[recipient]){
            removeAllFee();
        }
        else{
            require(amount <= _maxTxAmount, "Transfer amount exceeds the maxTxAmount.");
        }
        
        //Calculate burn amount and charity amount
        uint256 burnAmt = amount.mul(_burnFee).div(100);
        uint256 charityAmt = amount.mul(_charityFee).div(100);

        if (_isExcluded[sender] && !_isExcluded[recipient]) {
            _transferFromExcluded(sender, recipient, (amount.sub(burnAmt).sub(charityAmt)));
        } else if (!_isExcluded[sender] && _isExcluded[recipient]) {
            _transferToExcluded(sender, recipient, (amount.sub(burnAmt).sub(charityAmt)));
        } else if (!_isExcluded[sender] && !_isExcluded[recipient]) {
            _transferStandard(sender, recipient, (amount.sub(burnAmt).sub(charityAmt)));
        } else if (_isExcluded[sender] && _isExcluded[recipient]) {
            _transferBothExcluded(sender, recipient, (amount.sub(burnAmt).sub(charityAmt)));
        } else {
            _transferStandard(sender, recipient, (amount.sub(burnAmt).sub(charityAmt)));
        }
  1. Added a withdraw function. (As per Certik audit - SSL-03) 
    function withdrawLockedETH(address payable recipient, uint256 amount) external onlyOwner() {
        require(amount <= address(this).balance, "BEP20: amount should not exceed the contract balance");
        recipient.transfer(amount);
  1. Added disableAllFees function.
    /**
     * @dev Call this function to disable all Fees during the Presale stage.
     */
    function disableAllFees() external onlyOwner() {
        _taxFee = 0;
        _previousTaxFee = 0;
        _liquidityFee = 0;
        _previousLiquidityFee = 0;
        _burnFee = 0;
        _previousBurnFee = _burnFee;
        _marketingFee = 0;
        _previousMarketingFee = 0;
        inSwapAndLiquify = false;
        emit SwapAndLiquifyEnabledUpdated(false);
    }
  1. Added enableAllFees function.
    /**
     * @dev Call this function to enable Fees after finalizing the Presale.
     */
    function enableAllFees() external onlyOwner() {
        _taxFee = 3;
        _previousTaxFee = _taxFee;
        _liquidityFee = 1;
        _previousLiquidityFee = _liquidityFee;
        _burnFee = 1;
        _previousBurnFee = _taxFee;
        _marketingFee = 1;
        _previousMarketingFee = _marketingFee;
        inSwapAndLiquify = true;
        emit SwapAndLiquifyEnabledUpdated(true);
    }
  1. Added enableAllFees function.
    /**
     * @dev Call this function to change burnFee to zero percent upon a certain amount of token is burned.
     */
    function stopAutoBurn() external onlyOwner() {
        _burnFee = 0;
        _previousBurnFee = _taxFee;
    }
  1. Made the following two function externally adjustable.
    /**
     * @dev Update the Router address if Pancakeswap upgrades to a newer version.
     */
    function setRouterAddress(address newRouter) external onlyOwner {
        IPancakeswapV2Router02 _newPancakeRouter = IPancakeswapV2Router02(newRouter);
        pancakeswapV2Pair = IPancakeswapV2Factory(_newPancakeRouter.factory()).createPair(address(this), _newPancakeRouter.WETH());
        pancakeswapV2Router = _newPancakeRouter;

    /**
     * @dev Update the amount of 'numTokensSellToAddToLiquidity'.
     */
    function SetNumTokensSellToAddToLiquidity(uint256 newAmount, uint256 decimal) external onlyOwner() {
        numTokensSellToAddToLiquidity = newAmount*10**decimal;
    }
  1. Added more comments on functions and changed some "public" functions to "external".