GivingThanks

Root Cause and Impact

• Root Cause: Several functions are marked as public when they are not called internally within the contract. Using public exposes the function to both internal and external calls, whereas external restricts it to external calls only.

• Impact: Using public unnecessarily increases the contract's bytecode size and may slightly increase gas costs. It also deviates from Solidity best practices regarding function visibility.

Vulnerability Details

• Functions That Can Be Marked as external:

  • In CharityRegistry:

    function registerCharity(address charity) public {

    // ...

    }

    ​

    function verifyCharity(address charity) public {

    // ...

    }

    ​

    function isVerified(address charity) public view returns (bool) {

    // ...

    }

    ​

    function changeAdmin(address newAdmin) public {

    // ...

    }

  • In GivingThanks:

    function donate(address charity) public payable {

    // ...

    }

    ​

    function updateRegistry(address _registry) public {

    // ...

    }

• Issue: These functions are not called internally and can be marked as external to optimize gas usage.

Recommendations

• Change Visibility to external:

  • In CharityRegistry:

    function registerCharity(address charity) external {

    // ...

    }

    ​

    function verifyCharity(address charity) external {

    // ...

    }

    ​

    function isVerified(address charity) external view returns (bool) {

    // ...

    }

    ​

    function changeAdmin(address newAdmin) external {

    // ...

    }

  • In GivingThanks:

    function donate(address charity) external payable {

    // ...

    }

    ​

    function updateRegistry(address _registry) external {

    // ...

    }

• Benefits of Using external:

  • Reduces gas costs when the function is called externally.

  • Decreases the contract's bytecode size.

  • Clarifies the intended use of the function.

• Note: If a function might be called internally in the future or by inherited contracts, it should remain public.