GivingThanks

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

Bug Report: Unused Ownable Import in GivingThanks.sol

Summary:
The Ownable contract from OpenZeppelin is imported in GivingThanks.sol, but it is not used anywhere in the contract. This import adds unnecessary complexity and increases the contract size, which could lead to higher gas costs and maintenance overhead. If Ownable is not needed, it should be removed to simplify the contract.

Vulnerability Details:
No Effect. Only Gas Report

Impact:

Low Impact: Since the Ownable contract is not being used, it does not affect the functionality or security of the contract directly. However, leaving unused imports in the contract increases the deployment size, making it inefficient and unnecessarily complex. This can result in higher deployment and interaction costs.

Tools Used:

  • Manual code review

Recommendations:
To improve the contract's efficiency and reduce unnecessary complexity, it is recommended to:

  1. Remove the Ownable import: If the Ownable functionality is not required, remove the import statement for Ownable to streamline the contract and reduce deployment costs.

  2. Use Ownable if needed: If there are plans to implement ownership-related functions (such as administrative control over certain functions), consider implementing Ownable properly.

Solution: Updated Code

1. If Ownable is not required:

Simply remove the import statement as follows:

-diff:
import "@openzeppelin/contracts/access/Ownable.sol";

2. If Ownable is required (for example, for future features such as administrative control):

If you plan to use Ownable for administrative functions (e.g., adding, removing, or verifying charities), you can implement it as follows:

function setRegistry(address _registry) public onlyOwner {
registry = CharityRegistry(_registry);
}

In this case, you would be able to use the onlyOwner modifier on functions that only the contract owner should have access to.

Conclusion

The unused import of Ownable in GivingThanks.sol should either be removed if not needed, or fully implemented if future administrative functions require it. Removing unnecessary imports reduces contract size and improves efficiency, while adding ownership functionality could offer more control over the contract’s operations.

Updates

Lead Judging Commences

n0kto Lead Judge 6 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.