Solidity – Checking If Smart Contract is Secure and Optimized

optimizationSecuritysolidity

how safe is this code and is it possible to optimize it

// SPDX-License-Identifier: AGPL

pragma solidity ^0.8.17;

interface IBEP20 {
    function transfer(address to, uint256 value) external returns (bool);
    function balanceOf(address who) external view returns (uint256);
}


contract TokenDistributerGBR {
    address[] public recipientsList;
    address public owner;
    IBEP20 public token;
    mapping(address => bool) public recipients;

    event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

    constructor(address _tokenAddress) {
        _tokenAddress = 0xA970cAE9Fa1D7Cca913b7C19DF45BF33d55384A9;
        owner = msg.sender;
        token = IBEP20(_tokenAddress);
    }

    modifier onlyOwner() {
        require(msg.sender == owner, "Only the contract owner can perform this action");
        _;
    }

    function transferOwnership(address newOwner) public onlyOwner {
        require(newOwner != address(0), "New owner cannot be the zero address");
        emit OwnershipTransferred(owner, newOwner);
        owner = newOwner;
    }

    function addRecipient(address recipient) public onlyOwner {
        recipients[recipient] = true;
    }

    function removeRecipient(address recipient) public onlyOwner {
        recipients[recipient] = false;
    }

    function recipientsCount() public view returns (uint256 count) {
        for (uint256 i = 0; i < recipientsList.length; i++) {
            if (recipients[recipientsList[i]]) {
                count++;
            }
        }
    }

    function distributeTokens() public onlyOwner {
    require(token.balanceOf(address(this)) >= 1000000 * recipientsList.length, "Insufficient balance to distribute tokens");

    for (uint256 i = 0; i < recipientsList.length; i++) {
        if (recipientsList[i] != address(0)) {
            require(token.transfer(recipientsList[i], 1000000), "Token transfer failed");
        }
    }
}
}

Best Answer

The code seems to be secure but here are a couple of things you can do to improve the overall efficiency of the smart contract

Remove the hardcoded _tokenAddress value from the constructor so it would look like this

constructor(address _tokenAddress) {
        owner = msg.sender;
        token = IBEP20(_tokenAddress);
    }

Add a recipientsCount variable to store the number of recipients instead of iterating over the entire recipientsList to count them. This can make the recipientsCount function more efficient.

uint256 public recipientsCount;

function distributeTokens() public onlyOwner {
        require(token.balanceOf(address(this)) >= 1000000 * recipientsCount, "Insufficient balance to distribute tokens");

        for (uint256 i = 0; i < recipientsList.length; i++) {
            if (recipients[recipientsList[i]]) {
                require(token.transfer(recipientsList[i], 1000000), "Token transfer failed");
            }
        }

Additionally, same variable will also be used in below 2 functions from point 3.

Modify the addRecipient and removeRecipient functions to check if the recipient is already added or removed before updating the recipients mapping and recipientsCount variable to avoid unnecessary gas costs. The updated code should look like this

function addRecipient(address recipient) public onlyOwner {
        if (!recipients[recipient]) {
            recipients[recipient] = true;
            recipientsCount++;
        }
    }

    function removeRecipient(address recipient) public onlyOwner {
        if (recipients[recipient]) {
            recipients[recipient] = false;
            recipientsCount--;
        }
    }

Related Solutions

Solidity – Handling Unimplemented Functions in Contracts

The problem is that totalSupply() implementation is missing. Seems like autogenerated getters do not count. So you need to explicitly create the function.

contract StandardToken is Token {
    uint256 _totalSupply;

    function totalSupply() constant returns (uint256 totalSupply) {
        totalSupply = _totalSupply;
    }
}

Solidity – How to Resolve Remix Compilation Error: Invalid Array Length, Expected Integer Literal or Constant Expression

I changed your smart contract implementation with this smart contract.

I put notes just so you'd know your errors.

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

// NOTE: Import IERC20 contract
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

// NOTE: You must to implement IERC20 for override ERC20 methods!
contract NewToken is IERC20 {

  string public constant name = "NewToken";
  string public constant symbol = "NTKN";
  uint8 public constant decimals = 18;
  uint256 totalSupply_ = 10000000000000000000000000; 

  mapping(address => uint256) public balances;
  mapping(address => mapping (address => uint256)) allowed;

  // NOTE: I removed the events because it already defined into IERC20
  
  constructor() { 
    balances[msg.sender] = totalSupply_;
  }

  function totalSupply() public override view returns (uint256) {
    return totalSupply_;
  }

  // NOTE: You must to change signature function adding address parameter
  function balanceOf(address _address) public override view returns(uint256) {
    return balances[_address];
  }

  function transfer(address to, uint256 value) public override returns (bool) {
      require(value <= balances[msg.sender]);
      balances[msg.sender] = balances[msg.sender]-value;
      balances[to] = balances[to]+value;
      // NOTE: The event is called  Transfer not 'transfer'
      emit Transfer(msg.sender, to, value);
      return true;
  }

  function approve(address spender, uint256 value) public override returns (bool) {
      allowed[msg.sender][spender] = value;
      emit Approval(msg.sender, spender, value);
      return true;
  }

  function allowance(address owner, address spender) public override view returns (uint) {
      return allowed[owner][spender];
  }

  function transferFrom(address from, address to, uint256 value) public override returns (bool) {
      require(value <= balances[from]);
      require(value <= allowed[from][msg.sender]);

      balances[from] = balances[from]-value;
      allowed[from][msg.sender] = allowed[from][msg.sender]+value;
      balances[to] = balances[to]+value;
      emit Transfer(from, to, value);
      return true;
  }
}

Best Answer

Related Solutions

Solidity – Handling Unimplemented Functions in Contracts

Solidity – How to Resolve Remix Compilation Error: Invalid Array Length, Expected Integer Literal or Constant Expression

Related Topic