GivingThanks
First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Bug Report: Code Design and Optimizations in CharityRegistry.sol

koinerdegen

Summary:
The contract CharityRegistry.sol contains multiple design issues that can be optimized for gas efficiency, readability, and clarity. Additionally, the code lacks proper Natspec documentation. The following optimizations and changes are recommended:

  • Use more efficient variable naming with a clear naming convention.

  • Reorganize functions for better clarity and code layout.

  • Introduce proper Natspec comments for functions to improve documentation and understanding of the contract's functionality.

Vulnerability Details:
No any Vulnerability. Only optimization for code design

Impact:
Low Impact. These issues don’t necessarily pose security risks or major vulnerabilities. However, optimizing the code design and adding Natspec comments significantly improve code readability, maintainability, and reduce potential future errors.

Tools Used:

  • Manual Code Review

Recommendations:

  1. Update Variable Naming:
    Rename admin, verifiedCharities, and registeredCharities to s_admin, s_verifiedCharities, and s_registeredCharities for consistency and readability.

  2. Reorganize Functions:
    Functions should be organized as follows:

    • External functions first.

    • Public functions next.

    • Internal and private functions next.

    • View and pure functions should be placed accordingly at the bottom.

  3. Add Natspec Documentation:
    Add proper Natspec comments to all functions to explain the purpose, parameters, and returns of each function.

  4. Check for Address(0):
    Consider adding checks for address(0) in functions where addresses are updated or used to avoid issues with sending Ether to the zero address.

Recommendations**:**

  1. Updated CharityRegistry.sol Contract:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
/**
* @title CharityRegistry Contract
* @dev This contract manages charity registration and verification.
* The contract allows only the admin to perform administrative actions, such as verifying charities and changing the admin.
*/
contract CharityRegistry {
// --- State Variables ---
address public immutable s_admin;
mapping(address => bool) public s_verifiedCharities;
mapping(address => bool) public s_registeredCharities;
​
// --- Modifiers ---
modifier onlyAdmin() {
require(msg.sender == s_admin, "Only admin can perform this action");
_;
}
​
modifier nonZeroAddress(address _addr) {
require(_addr != address(0), "Address cannot be zero");
_;
}
​
// --- Constructor ---
/**
* @dev Constructor sets the initial admin of the contract.
* @param _admin The address of the admin
*/
constructor(address _admin) nonZeroAddress(_admin) {
s_admin = _admin;
}
​
// --- External Functions ---
/**
* @dev Changes the admin of the contract to a new address.
* @param newAdmin The address of the new admin
*/
function changeAdmin(address newAdmin) external onlyAdmin nonZeroAddress(newAdmin) {
s_admin = newAdmin;
}
​
/**
* @dev Verifies a charity as legitimate.
* @param charity The address of the charity to verify
*/
function verifyCharity(address charity) external onlyAdmin nonZeroAddress(charity) {
require(s_registeredCharities[charity], "Charity not registered");
s_verifiedCharities[charity] = true;
}
​
/**
* @dev Registers a new charity. Only admin can register charities.
* @param charity The address of the charity to register
*/
function registerCharity(address charity) external onlyAdmin nonZeroAddress(charity) {
s_registeredCharities[charity] = true;
}
​
// --- Public View Functions ---
/**
* @dev Checks whether a charity is verified.
* @param charity The address of the charity to check
* @return True if the charity is verified, otherwise false
*/
function isVerified(address charity) public view returns (bool) {
return s_verifiedCharities[charity];
}
​
/**
* @dev Checks whether a charity is registered.
* @param charity The address of the charity to check
* @return True if the charity is registered, otherwise false
*/
function isRegistered(address charity) public view returns (bool) {
return s_registeredCharities[charity];
}
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.