Audit hyacinthaudits.xyz registration test

Audit TestErrorNFT.sol for registration test of hyacinthaudits.xyz platform.

Table of Contents

• Summary
  • Files Summary
  • Files Details
  • Issue Summary
• High Issues
  • H-1: Mint Limit Check Missing
  • H-2: Reentrancy Issues
• Medium Issues
  • M-1: Compile errors | No arguments passed to the base constructor
  • M-2: Compile errors | Line 30: The line totalSupply += amount; is outside of any function
  • M-3: Wrong Visibility of Functions
  • M-4: Redundant totalSupply Update
• Low Issues
  • L-1: SPDX license identifiers missing
  • L-2: Imports files method
  • L-3: Removed imported openzeppelin contract in new version
  • L-4: Naming Convention
  • L-5: Incorrect Return Statement in _baseURI

Summary

Files Summary

Key	Value
.sol Files	1
Total nSLOC	41

Files Details

Filepath	nSLOC
src/TestErrorNFT.sol	41
Total	41

Issue Summary

Category	No. of Issues
High	2
Medium	4
Low	5

High Issues

H-1: Mint Limit Check Missing

There is no check to enforce the maxMintPerUser limit in the mint or batchMint functions.

2 Found Instances

• Found in src/TestErrorNFT.sol Line: 18

      function mint(address to) public {
  +       require(userMintedCount[to] < maxMintPerUser, "Max mint per user exceeded");
  +       require(totalSupply() < maxSupply, "Max supply exceeded");
  
          _safeMint(to, _tokenIdCounter.current());
          _tokenIdCounter.increment();
          userMintedCount[to]++;
      }

• Found in src/TestErrorNFT.sol Line: 25

      function batchMint(address to, uint256 amount) public {
  +       require(userMintedCount[to] + amount <= maxMintPerUser, "Max mint per user exceeded");
  +       require(totalSupply() + amount <= maxSupply, "Max supply exceeded");
  
          for (uint256 i = 0; i < amount; i++) {
              mint(to);
          }
      }

H-2: Reentrancy Issues

The contract could benefit from using checks-effects-interactions pattern for safer minting processes.

1 Found Instance

• Found in src/TestErrorNFT.sol Line: 18

Medium Issues

M-1: Compile errors | No arguments passed to the base constructor

No arguments were passed to the base constructor. Specify the arguments.

1 Found Instance

• Found in src/TestErrorNFT.sol Line: 16

  TypeError: No arguments passed to the base constructor. Specify the arguments
  Note: Base constructor parameters:
  --> @openzeppelin/contracts/access/Ownable.sol:38:16:
  |
  38 |     constructor(address initialOwner) {
  |                ^^^^^^^^^^^^^^^^^^^^^^

  constructor() ERC721("ErrorNFT", "ENFT") Ownable(msg.sender) {}

M-2: Compile errors | Line 30: The line totalSupply += amount; is outside of any function

The line totalSupply += amount; should be inside the batchMint function.

1 Found Instance

• Found in src/TestErrorNFT.sol Line: 30

      function batchMint(address to, uint256 amount) public {
          for (uint256 i = 0; i < amount; i++) {
              mint(to);
          }
  +        totalSupply += amount;
      }
  -        totalSupply += amount;

M-3: Wrong Visibility of Functions

The batchMint and setUserMintLimit functions should ideally be external based on the intended use case.

2 Found Instances

• Found in src/TestErrorNFT.sol Line: 25

  function batchMint(address to, uint256 amount) external

• Found in src/TestErrorNFT.sol Line: 32

  function setUserMintLimit(uint256 newLimit) external onlyOwner

M-4: Redundant totalSupply Update

The totalSupply is incremented both in the mint function and in the batchMint function for each mint.

2 Found Instances

• Found in src/TestErrorNFT.sol Line: 22

• Found in src/TestErrorNFT.sol Line: 30

  Recommended: we can remove totalSupply variable and use a counter instead.

      function totalSupply() public view returns (uint256) {
          return _tokenIdCounter.current();
      }

Low Issues

L-1: SPDX license identifiers missing

SPDX license identifiers should be added to the top of contract files.

1 Found Instance

• Found in src/TestErrorNFT.sol Line: 0

  // SPDX-License-Identifier: MIT

L-2: Imports files method

Instead of Wildcard Import, consider using Named Import.

3 Found Instances

• Found in src/TestErrorNFT.sol Line: 3

  import {ERC721} from "@openzeppelin/contracts/token/ERC721/ERC721.sol";

• Found in src/TestErrorNFT.sol Line: 4

  import {Counters} from "@openzeppelin/contracts/utils/Counters.sol";

• Found in src/TestErrorNFT.sol Line: 5

  import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

L-3: Removed imported openzeppelin contract in new version

Counters.sol in version 5 has been removed.

1 Found Instance

• Found in src/TestErrorNFT.sol Line: 4

  Link to Issue

L-4: Naming Convention

For consistency and readability, you may want to keep a consistent naming convention for variables (e.g., maxMintPerUser could be maxMintPerAddress).

1 Found Instance

• Found in [email protected] Line: 11

L-5: Incorrect Return Statement in _baseURI

The _baseURI function should have a valid URI string in return and can be marked as pure.

1 Found Instance

• Found in [email protected] Line: 38

  function _baseURI() internal pure override returns (string memory) {
      return "ipfs://";
  }