2

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");
        }
    }
}
}

1 Answer 1

3

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

  1. Remove the hardcoded _tokenAddress value from the constructor so it would look like this
constructor(address _tokenAddress) {
        owner = msg.sender;
        token = IBEP20(_tokenAddress);
    }
  1. 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.

  1. 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--;
        }
    }
2
  • thanks for the good advice, but could you give an example of the code that implements point two from your answer? Apr 5, 2023 at 4:57
  • @LllCilician check updated answer! I missed that part earlier Apr 5, 2023 at 5:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.